Skip to content

Conversation

@kubukoz
Copy link
Member

@kubukoz kubukoz commented Sep 27, 2024

This is more of a random attempt rather than a methodically applied implementation, but early testing has proven it to be successful.

Potentially closes #1568, #1532 (works on my machine but needs investigation into whether this is actually a more correct implementation). It would be amazing if there are protocol tests for this...

PR Checklist (not all items are relevant to all PRs)

  • Added unit-tests (for runtime code)
  • Added bootstrapped code + smoke tests (when the rendering logic is modified)
  • Added build-plugins integration tests (when reflection loading is required at codegen-time)
  • Added alloy compliance tests (when simpleRestJson protocol behaviour is expanded/updated)
  • Updated dynamic module to match generated-code behaviour
  • Added documentation
  • Updated changelog

@kubukoz kubukoz changed the title Quick attempt at resolving AWS signing problems (#1568, #1532) Quick attempt at resolving AWS signing problems Sep 27, 2024
val endpointPrefix = awsService.endpointPrefix.getOrElse(endpoint.id.name)
val endpointPrefix =
awsService.endpointPrefix
.orElse(awsService.arnNamespace.map(_.value))
Copy link
Member Author

Choose a reason for hiding this comment

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

question: should this be sigv4 rather than arnNamespace?

should we even be looking at endpointPrefix? sigv4 is supposed to match arnNamespace but smithy says it's a SHOULD (see documentation trait of the sigv4 trait members).

For what it's worth, both Sagemaker and Location work if sigv4 is used instead, in both locations of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

we should eventually just go through all services on https://docs.aws.amazon.com/general/latest/gr/aws-service-information.html one-by-one and validate that we compute the right urls...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's a bit misleading because of the endpointPrefix value name, but really we should be using the sigv4 value here.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok I'll change to sigv4 then, and maybe run through the services to see if it's present on all of them (and what the fallback should be).

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.

Sagemaker: invalid signature (Credential should be scoped to correct service)

2 participants