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.

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

Only include Action/Version params for v4 sigs when really required #13

Merged
merged 1 commit into from Mar 29, 2019

Conversation

Projects
None yet
2 participants
@pimterry
Copy link
Contributor

pimterry commented Mar 28, 2019

Currently this conversion assumes all v4 signatures include required Action and Version parameters. That's not correct: v4 APIs that don't use the query or ec2 protocols don't need or use these parameters.

To see this, compare the v2 docs with the v4 docs - v2 lists these parameters as required, but v4 only talks about the X-Amz-Security-Token query parameter, and no others (and we include that on every spec generated here already). There was a URL commented in here explaining this logic, but it linked to the IAM signing docs, and IAM uses the query protocol.

This PR stops including those parameters as default, and instead makes sure they're both included (adding Version) for the right protocols.

To check the results, take a look at:

  • appsync - the docs don't use Action or Version params, and now neither does the spec
  • autoscaling - the docs do require Action & Version params, and the spec now includes these more explicitly on every operation with the exact correct value.
@MikeRalphson
Copy link
Contributor

MikeRalphson left a comment

LGTM!

@MikeRalphson MikeRalphson merged commit 490a095 into APIs-guru:master Mar 29, 2019

@MikeRalphson

This comment has been minimized.

Copy link
Contributor

MikeRalphson commented Mar 29, 2019

API definitions updated and deploying now. Many thanks!

@pimterry pimterry deleted the pimterry:action-params branch Mar 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.