Skip to content

Conversation

@Velfi
Copy link
Contributor

@Velfi Velfi commented Jan 19, 2022

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

jdisanti and others added 29 commits January 11, 2022 11:47
Co-authored-by: Zelda Hessler <zhessler@amazon.com>
* Allow the canary runner to log metrics

* Add instructions on adding a new canary

* Add note about deployment to readme
* add: links to related types for fluent builders

* update: CHANGELOG.next.toml

* update: generate fancy documented lists

* refactor: put docs on client builder methods instead of fluent builder
rename: generateShapeMemberList -> generateShapeMemberDocs
refactor: move generateShapeMemberList to FluentClientDecorator.kt
update: appease spellchecker

* remove: extra line in docs

* remove: unnecessary snake case conversion
…None` (#1050)

We were instead providing empty collections e.g. `Some(Vec::new())`,
which makes protocol tests fail.

This commit also passes Smithy member names through the symbol provider
to obtain safe variable names.

This commit also adds some plumbing to the server protocol test case
generation to fix broken request test definitions in the
`awslabs/smithy` project.
* Add Support for SSO

This commit adds support for the SSO credential provider, which enables the aws-config to support using SSO when specified in `~/.aws/config`.

* Rename & add test of configuration failure

* Add SSO to the smoke test list

* CR improvements

- Improve error messages
- zeroize token
- add track_caller to improve test failure error messages

* Apply suggestions from code review

Co-authored-by: John DiSanti <jdisanti@amazon.com>

* Update changelogs

Co-authored-by: John DiSanti <jdisanti@amazon.com>
* Assorted canary improvements

1. Add a timeout for individual canaries
2. Add an Ec2 canary.
3. Add functionality to enable/disable canaries based on SDK version

* Refactor page size
… when routing (#1029)

In #996 [0], we realized that we were ignoring empty URI path segments
when doing label extraction. It turns out we are also ignoring them when
doing routing, as the TypeScript sSDK currently does [1], from which the
initial implementation was copied.

However, a discussion with the Smithy team in smithy-lang/smithy#1024 [2]
revealed that we _must not_ ignore empty URI path segments when routing
or doing label extraction, since empty strings (`""`) should be assigned
to the labels in those segments.

This commit fixes the behavior so that we don't ignore empty path
segments _when doing routing_. #996 will take care of fixing the behavior
when doing label extraction.

[0]: smithy-lang/smithy-rs#996 (comment)
[1]: https://github.com/awslabs/smithy-typescript/blob/d263078b81485a6a2013d243639c0c680343ff47/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.ts#L78
[2]: smithy-lang/smithy#1024
…lict disambiguation (#1036)

This commit implements basic URI pattern conflict disambiguation when
routing incoming requests to service operations.

It does this by introducing a measure of how "important" a `RequestSpec`
is. The more specific a `RequestSpec` is, the higher it ranks in
importance. Specificity is measured by the number of segments plus the
number of query string literals in its URI pattern, so
`/{Bucket}/{Key}?query` is more specific than `/{Bucket}/{Key}`, which
is more specific than `/{Bucket}`, which is more specific than `/`.

Why do we need this?

Note that:

1. the Smithy spec does not define how servers should route incoming
   requests in the case of pattern conflicts; and
2. the Smithy spec even outright rejects conflicting patterns that can
   be easily disambiguated e.g. `/{a}` and `/{label}/b` cannot coexist.

We can't to anything about (2) since the Smithy CLI will refuse to build
a model with those kind of conflicts. However, the Smithy CLI does allow
_other_ conflicting patterns to coexist, e.g. `/` and `/{label}`. We
therefore have to take a stance on (1), since if we route arbitrarily we
render basic usage impossible (see issue #1009).

So this ranking of routes implements some basic pattern conflict
disambiguation with some common sense. It's also the same behavior that
the TypeScript sSDK is implementing [0].

This commit also:

* makes the `future` module private,
* hides documentation for the `request_spec` module; and
* makes some fields from some structs in the `request_spec` module
  private and adds constructors.

Closes #1009.

[1]: https://github.com/awslabs/smithy-typescript/blob/d263078b81485a6a2013d243639c0c680343ff47/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.ts#L59.
…ict avoidance test (#1070)

The previous commit introduced a failing test because CI doesn't have
Rust tests as a blocking step and auto-merge was enabled.

With the two operations:

1. `ListBuckets` with URI pattern `/`; and
2. `ListObjects` with URI pattern `/{bucket}`,

the test wants to route the request `/` to the first operation. However,
in #1029 we started allowing empty path segments. Since `ListBuckets`
has more weight than `ListObjects`, the request matches against it,
binding `""` to the `bucket` label.
* fix: silence warnings in Lambda environment

* fix: implement code review changes

* fix: implement unit test changes

* fix: implement code review changes
* Use `pretty_assertions` in protocol tests

We do this by pulling in `pretty_assertions` into the `codegenScope` so
that rendered code that uses `assert_eq!` _explicitly_ invokes the macro
from `pretty_assertions`.
* update: client input links to point to fluent builder

* refactor: fluent-builder-related string formatting

* add: more docs

* fix: two bugs I introduced
…equests (#1060)

This commit adds support for the `httpPayload` Smithy trait when applied
to operation input members (request deserialization).

The code to deserialize HTTP-bound data from HTTP responses in
`ResponseBindingGenerator.kt` has been moved into a common class,
`HttpBindingGenerator.kt`, since it's useful for both clients and
servers in deserializing data from HTTP requests and responses,
respectively.
…t or server (#1081)

For `httpRequestTest`s and `httpResponseTest`s applied to operations,
we're only generating the tests that apply to servers or clients.

This commit does the same when `httpResponseTest` is applied to errors.
…1083)

* Fix paginator bug where `None` was returned immediately

The escape hatch added by aws-sdk-rust#391 did not properly handle the case where the first response was `None` and _not_ the empty string. This diff:
- Checks for emptiness for both maps and strings
- Fixes the check so that an initial `None` input does not cause an incorrect paginator error

* Update changelog

* Apply suggestions from code review

Co-authored-by: Zelda Hessler <zhessler@amazon.com>

* rustfmt

Co-authored-by: Zelda Hessler <zhessler@amazon.com>
…y string data (#1058)

We are currently deserializing the query string into `Vec<(&str,
&str)>`. `serde_urlencoded` panics if the input string slice contains
escaped data, since in that case it needs to allocate a new `String` to
unescape the input string slice's contents.

Instead of deserializing to `Vec<(String, String)>`, we can instead use
`Cow<'a, str>` so that deserialization only allocates when strictly
required.

Reference: serde-rs/serde#1413 (comment)
* release v0.35.0

* update: aws models
@Velfi Velfi requested a review from a team as a code owner January 19, 2022 17:53
@rcoh
Copy link
Contributor

rcoh commented Jan 19, 2022

we should remove the readme version update—we need to hold off until the crates are actually published

* Simplify Canary Lambda CI

* Remove `Cargo.toml` check and make the script recursive
Copy link
Contributor

@rcoh rcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff audited by @rcoh. We should add a note to the README that releases may be delayed

@Velfi Velfi merged commit 56c7ee7 into main Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants