Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add round-trip convertibility of resource schemas #2445

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Add round-trip convertibility of resource schemas #2445

merged 2 commits into from
Nov 13, 2024

Conversation

haydenbaker
Copy link
Contributor

Background

  • Before these changes, converting a node to resource-schema is lossy
  • These changes include the necessary changes to keep round-trips between node and resource-schema lossless
  • Property was adapted to incorporate its separate "properties" as extensions to the schema**
    • **only when these "properties" (insertionOrder, dependencies) is set to a non-default
  • Unknown properties are added as extensions to the Schema

Testing

  • Unit testing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@haydenbaker haydenbaker marked this pull request as ready for review November 7, 2024 22:01
@haydenbaker haydenbaker requested a review from a team as a code owner November 7, 2024 22:01
@haydenbaker haydenbaker force-pushed the haydenbaker/cfn-resource-schema-conversions branch 3 times, most recently from 6ec3454 to 47437e3 Compare November 13, 2024 16:11
@haydenbaker haydenbaker force-pushed the haydenbaker/cfn-resource-schema-conversions branch from 47437e3 to e0ad918 Compare November 13, 2024 16:13
@kstich kstich dismissed JordonPhillips’s stale review November 13, 2024 17:36

Fix validated by other reviewer

@haydenbaker haydenbaker merged commit ccf6e6a into main Nov 13, 2024
14 checks passed
yasmewad pushed a commit to yasmewad/smithy that referenced this pull request Nov 13, 2024
@haydenbaker @yasmewad
Add round-trip convertibility of resource schemas (smithy-lang#2445)
fcaf7dc
MYoung25 pushed a commit to MYoung25/smithy that referenced this pull request Nov 29, 2024
@haydenbaker @MYoung25
Add round-trip convertibility of resource schemas (smithy-lang#2445)
61bf9b4
MYoung25 added a commit to MYoung25/smithy that referenced this pull request Feb 26, 2025
@MYoung25 @kstich
@landonxjames @mtdowling @rhernandez35 @milesziemer @hpmellema @haydenbaker @yasmewad @JordonPhillips @jbelkins
New Smithy Landing Page (#2)
f86301d 
* landing page scaffolding

* add base shadcn components and font

* add simple navigation, primarily to test exporting the html snippet

* enable static site generation for landing page and nav bar snippet

* add card with gradient border

* add smithy color palette

* add storybook and stories

* add story for top navigation

* add svg spider connections

* work on making svg connections easier to use

* better svg implementations

* updated svgs, ran prettier, proof of concept of core design elements

* add service example and full diagram

* add i18n support

* add support for an svg wheel

* support i18n in storybook

* add simple test checking that the app can build

* fix gradient collision

* add section and pop gradient ui elements

* example outline based on prelim design

* mlp

* update doc build process for landing page

* add favicons and clean up public

* switch to arial

* implement a11y and i18n improvements

* add non-cookie tracking scripts

* add logo attributions

* fix svg bug with scroll position

* Bump version to 1.52.1

* Lower UnboundTestOperation to WARNING

Consumers of this package using the `includeServices` transform may not
also be running `removeUnusedShapes`, leaving behind operations that then
trip this validation and cause an error. Lowering this to a warning still
emits a message but doesn't block consumers until we find a better strategy
for running this validation at Smithy-only build time.

* Improve member target diff checks for maps

This commit updates the ChangedMemberTarget diff evaluator to properly
check changes to map keys and values the same way it checks changes to
list members.

* Update NullAndEmptyHeadersServer tests (smithy-lang#2433)

The test now align with the similar tests for clients. They expect empty
headers to be serialized as "" and null headers to not be serialized

* Return Builders instead of SmithyBuilders

This commit updates all instances where a generic SmithyBuilder<> was
returned to return the specific implementing Builder. This means users
of these methods can easily use the Builder's functionality without
using a cast.

* Use lowercase headers for prefix-header test

HTTP headers are case-insensitive so using casing in prefix header
tests adds unnecessary challenges when implementing rest-xml and
rest-json protocol tests. Many HTTP implementations lowercase header
names to make them case-insensitive (e.g., Go), and it's even a
requirement of h2 (https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2).

This commit updates httpPrefixHeaders tests to use lowercase header
names to avoid any kind of header casing issues when implementing
these tests.

* Add delimiter and reusable to arn trait

This commit adds two fields to the aws.api#arn trait:

* resourceDelimiter - used to define the delimiter for resource segments of
absolute ARN templates.
* reusable - indicates whether an ARN may be reused for different instances
of a resource.

* Fix for tagsProperty in CFN schema creation

This value is expected to be a JSON pointer to a property, not just the
name of the property.

See: https://github.com/aws-cloudformation/cloudformation-cli/blob/master/src/rpdk/core/data/schema/provider.definition.schema.v1.json#L193-L197

* Use the correct CBOR type for blobs in RPCv2 CBOR protocol tests

* Warn for service missing tagEnabled

* Inject tagging operation permissions for CFN

This commit updates the CFN resource schema generation to fill in the
permissions field of the tagging configuration for resources. The AwsTagIndex
is used to find the APIs and their relevant required actions to invoke.

This also refactors the Tagging updates into a single Mapper so the updates
aren't strewn about the general CfnConverter.

* Defer scrubTraitDefinitions for JsonSchema

* Add config for inline JSON Schema maps

This commit adds the useInlineMaps setting to allow users to configure
JSON Schema conversion to inline converted map shapes instead of creating
references.

* Add validator for xmlFlattened + xmlName (smithy-lang#2439)

Adds a validator for the xmlFlattened trait that checks if the member's
target is a list that has a member with xmlName, and that xmlName
doesn't match the name of the xmlFlattened member. xmlFlattened causes
the xmlName of the list member to be ignored. This is called out in our
docs:
https://smithy.io/2.0/spec/protocol-traits.html#flattened-list-serialization,
but we were missing a validator for it. It is a warning because you might
mean to do this, i.e. if the list is used in multiple places.

I also added some suppressions to our protocol tests that are testing
the behavior for models that would trip up this validator.

* Downgrade noisy log statement in CreateDedicatedInputAndOutput transformer (smithy-lang#2451)

* Infer defined condition key service

This commit relaxes the pattern on the @defineConditionKeys trait's keys
to enable inferring the service to be the service's arnNamespace.

* Add pagination flattening transform (smithy-lang#2454)

Adds a transform that flattens service-level pagination info into pagination traits on operations.

* Fix broken README link

Fixes smithy-lang#2456

* Fix typo in event stream content-type

* Add transforms to remove deprecated shapes (smithy-lang#2452)

Adds two transforms to smithy-model and directed codegen that allow users to filter out deprecated shapes by version and/or date.

* Expand usage of title trait (smithy-lang#2461)

Expands the usage of the `@title` trait to allow application to any non-member shape.

* Add round-trip convertibility of resource schemas (smithy-lang#2445)

* Converting existing gradle DSL from groovy to kotlin. (smithy-lang#2453)

* Converting existing gradle DSL from groovy to kotlin.

** Why is this needed ? **
This is the first step in moving to a more consistent and composable experience for gradle configuration in the smithy repository. Writing it in kotlin to make it more expressive as compared to groovy. This commit just converts the existing gradle build from groovy to kotlin.

** Next steps: **
Once this change is merged, few other PRs will follow to introduce nice features such as `buildSrc` plugins to enhance configurability of how each sub project builds in this package. It will introduce flexibility in things such as allowing each subproject to compile with different JVM toolchain, and much more.

* Converting extensions to extras. Other changes addressing comments.

* Update IDL serializer to write metadata to a separate file to avoid duplication of data

* Add transform to mark required idempotency tokens client optional (smithy-lang#2466)

Add transform to make required `@idempotencyToken` members `@clientOptional` so that they can be left empty and injected.



---------

Co-authored-by: Kevin Stich <kevin@kstich.com>
Co-authored-by: Michael Dowling <michael@mtdowling.com>

* Update blob defaults for protocol tests (smithy-lang#2467)

Updates blob defaults in protocol tests to use valid Base64 strings. Also updates the associated default documentation to make this requirement clear.

* Bump version to 1.53.0 (smithy-lang#2465)

* Add a tool to create and render changelog entries

Writing changelog entries is not super fun. Writing them after the
fact, especially when you didn't write the original feature, is even
less fun. And merge conflicts because of synchronizing a changelog
file are SUPER not-fun.

This change solves those problems. It introduces a changelog tool
similar to what many of the SDK teams use (and indeed cribs some code
from them).

> Why?

Using distinct files for changelog entries solves a major problem:
merge conflicts. These are horribly annoying to deal with during
development, especially on a file that is essentially documentation
and semantically has no reason to ever conflict. This problem is
less bad for Smithy itself than it is for SDKs that have daily
releases, but it's a problem for us too with our relatively large
team.

> What's different?

This differs from what other SDKs do in a few ways. Firstly, and
most notably, we aren't deriving a version number from these. We
could! It would not be hard! But I elected not to do that in this
PR since it's not as big a problem for us.

We also have a habit of linking the PRs for all of our changes in
the changelog. This is actually kinda annoying because it creates
a bit of a chicken-egg scenario. This is why I added the github
action to automatically add it. If the action is annoying we can
always get rid of it and just stop relying on the links. But that
would be a shame.

Mechanically there's also a change in that I used modern python to
write this, and updated any borrowed code to also be modern python.

> Why not use jmeslog?

jmeslog is a standalone tool derived from the python team's
changelog tool (which started it all). It is essentially exactly what
they do and want, but there's a few things that prevent us from using
it. Notably, we need the current date and pr links. jmeslog doesn't
have that and doesn't currently allow for easy extension in that way.

It also has things we don't necessarily want, like a category. That's
really mostly used in the sdks to indicate changes made for a
particular service. With hundreds of services, that's an issue worthy
of calling out. But smithy itself doesn't need it.

But also, it's a dependency, and it has dependencies. I'd rather not
have dependencies if I can avoid it. And what we need is ultimately
not that complex. And doing it ourselves gives us some flexibility.

> Why not derive changelogs from commits?

The audience for changelogs and the audience for commit messages are
two separate (though sometimes intersecting) audiences. Commits may
contain conext about technical junk that only mattters to people
working on the code itself.

There is also not necessarily a 1-1 relationship between a changelog
entry and a commit. Large features are often broken up into logical
commits that make reviewing and code diving easier, but aren't
important to the changelog. You might also have commits that make
changes unimportant to the changelog audience, such as formatting
changes or changes to CI configurations.

Commits are also, critically, ***immutable***. Did you make a typo
in your commit? Now it's preserved forever in your changelog. With
JSON files, you can just change them. Those changes are now part of
the commit record!

Also, on a more subjective level, have you seen repos that have
semantic commits? They're ugly. Having more than half your commits
starting with `chore` just makes it look like your project is in
maintenance mode and/or like you hate your job. And if you're
following best practices for commit messages, that's eating into
your 50 character limit for the title.

> Any unfortunate bits?

There's no line wrapping, so you better believe the changelog is
gonna have some long lines. It doesn't really matter though, because
it'll all be natural when you're seeing it fully rendered. The line
boundary thing these days mostly helps with readability when you're
editing a file, but you will never manually edit the file.

> What's next?

Provided this goes through, the next step will be to backfill all
the current changelog entries. That'll certainly be a task.

A github action to create the version bump pr would be sweet. We'd
need to add the ability to derive a version number from the feature
list, which isn't hard. Then the script could just aslo perform the
version bump by writing that one file. Then it could be just a button
press away in github. Maybe as a fully automated release?

* Add GitHub action to validate staged changelogs

This adds a GitHub action that checks for staged changelogs. If they
aren't present, it posts a reminder comment to add them. It does
*not* fail the build because there are valid cases where a changelog
is not desired.

If there are changelog entries present, it ensures that they have a
pull request associated with them. If they don't, it posts a pr
comment with a change suggestion that adds it in. It still does not
fail the build because maybe there's a reason not to add it.

* Revert "Add GitHub action to validate staged changelogs" (smithy-lang#2475)

This reverts commit 50fa20e.

* fix: Convert blob default values to Base64 (smithy-lang#2474)

Convert blob default values to Base64 in awsJson1_0 & restJson1 protocol tests.

* Update smithy logo on readme and documentation site (smithy-lang#2478)

* Update readme to use non-favicon (smithy-lang#2479)

* css tweaks

* updated section wording

* updates from review

* remove unnecessary code

---------

Co-authored-by: Kevin Stich <kevin@kstich.com>
Co-authored-by: Landon James <lnj@amazon.com>
Co-authored-by: Michael Dowling <mtdowling@gmail.com>
Co-authored-by: Richard Hernandez <riher@amazon.com>
Co-authored-by: Miles Ziemer <45497130+milesziemer@users.noreply.github.com>
Co-authored-by: Hunter Mellema <124718352+hpmellema@users.noreply.github.com>
Co-authored-by: Hayden Baker <haydenbaker98@gmail.com>
Co-authored-by: Yash Mewada <128434488+yasmewad@users.noreply.github.com>
Co-authored-by: Hunter Mellema <hpm@amazon.com>
Co-authored-by: Michael Dowling <michael@mtdowling.com>
Co-authored-by: Jordon Phillips <JordonPhillips@users.noreply.github.com>
Co-authored-by: Josh Elkins <jbelkins@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kstich kstich kstich approved these changes

@JordonPhillips JordonPhillips Awaiting requested review from JordonPhillips

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@haydenbaker @JordonPhillips @kstich