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

feat(proxy-sigv4-backend): migrate to aws-sdk v3 for js and remove deprecations #10

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

alecjacobs5401
Copy link
Contributor

Description

Gives the @segment/backstage-plugin-sigv4-proxy-backend plugin some much needed love and attention:

  • fully leans into the new backend system for development servers
  • fully leans into the new backend system interfaces and removes all deprecations
  • migrates off of the aws-sdk V2 package in favor of the AWS SDK V3 for JavaScript packages.
  • retains use of aws4 for SigV4 signing of requests. (has no aws sdk dependencies)
  • replaces got implementation with node-fetch to satisfy https://backstage.io/docs/architecture-decisions/adrs-adr013/

Differences in this Approach

Credential Resolution

The way that credentials are resolved with @aws-sdk/credential-providers are slightly different now than before. The V3 package now has the concept of credential providers that return Promises that resolve to the necessary credentials. Credentials themselves are now a simpler type (essentially a Record). As such, it puts more of an onus on us to handle the refresh logic re-generate the credentials.

This also means that the contract for buildMiddleware had to change to return a promise. Before, credential resolution was synchronous, but now its async. This means that failure to assume credentials now will fail to register the route but let the app continue. We log a warning loudly about that, but its technically a difference in behavior since before a 401 or 5XX might have been returned instead of a 404.

I'm happy to adjust appropriately depending on what we feel is best.

Request Initiator

This PR swaps from got under the hood to node-fetch. As such, there are some differences in how Headers are handled since, apparently, every request library has a different idea of what headers can be.

aws4 requires the format of { [key: string]: string }, so we are normalizing values to that. I did some testing with API calls and multi value headers are typically comma-separated, so we're normalizing values to that.

I do not expect that to cause any upstream problems since the expected headers we're forwarding are so locked down anyways.

…precations

Signed-off-by: Alec Jacobs <charles.jacobs@segment.com>
@alecjacobs5401 alecjacobs5401 requested a review from a team as a code owner August 28, 2024 15:55
bhavanki
bhavanki previously approved these changes Aug 28, 2024
plugins/proxy-sigv4-backend/src/service/router.ts Outdated Show resolved Hide resolved
Signed-off-by: Alec Jacobs <charles.jacobs@segment.com>
@alecjacobs5401 alecjacobs5401 merged commit cac1d4a into main Aug 28, 2024
5 checks passed
@alecjacobs5401 alecjacobs5401 deleted the ajacobs/aws-sdk-v3 branch August 28, 2024 18:53
@alecjacobs5401
Copy link
Contributor Author

I'll aim to do a release this week to get this out.

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.

2 participants