Skip to content

Conversation

@rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Jan 20, 2025

In a recent upgrade of the SDKv3, the INI credential provider has gotten a private copy of the STS Client, to make AssumeRole calls with. This means the STS Client used by the INI provider is now unmockable.

In #31702, we migrated the tests of the credentials chain mostly to using mocks, as opposed to what it was doing in the past: intercepting network traffic and emulating a fake STS Endpoint. These tests now start failing due to an SDK upgrade.

Fortunately, most of the old code was still there, so in this change I'm ripping out the STS Mocking and doing a couple minor changes; the tests now pass again.

This PR also upgrades the SDKv3 version at the same time, some other packages that needed to be upgraded along with this as well (@smithy/middleware-endpoint and cdk-assets which covers a new enum value for the S3 client).


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

In a recent upgrade of the SDKv3, the INI credential provider has gotten
a private copy of the STS Client, to make AssumeRole calls with. This
means the STS Client used by the INI provider is now unmockable.

In #31702, we migrated the tests of the credentials chain mostly to
using mocks, as opposed to what it was doing in the past: intercepting
network traffic and emulating a fake STS Endpoint. These tests now start
failing due to an SDK upgrade.

Fortunately, most of the old code was still there, so in this change
I'm ripping out the STS Mocking and doing a couple minor changes;
the tests now pass again.
@rix0rrr rix0rrr requested a review from a team as a code owner January 20, 2025 10:48
@aws-cdk-automation aws-cdk-automation requested a review from a team January 20, 2025 10:49
@github-actions github-actions bot added the p2 label Jan 20, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 20, 2025
Copy link
Contributor

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

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

Jeez louis

"@aws-sdk/credential-providers": "3.699.0",
"@aws-sdk/ec2-metadata-service": "3.699.0",
"@aws-sdk/lib-storage": "3.699.0",
"@aws-sdk/client-appsync": "^3.699.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not super important but I think our package.json normally corresponds with the lock file.

Suggested change
"@aws-sdk/client-appsync": "^3.699.0",
"@aws-sdk/client-appsync": "^3.730.0",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorta the point of having the distinction between a package.json and a lockfile?

I do not care about this 😉

@iliapolo iliapolo added the pr/do-not-merge This PR should not be merged at this time. label Jan 20, 2025
@iliapolo
Copy link
Contributor

@rix0rrr this also upgrades the SDK version, mention it in the PR title?

@rix0rrr rix0rrr removed the pr/do-not-merge This PR should not be merged at this time. label Jan 20, 2025
@mergify
Copy link
Contributor

mergify bot commented Jan 20, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@codecov
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.46%. Comparing base (e31924a) to head (8d130e5).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #33016      +/-   ##
==========================================
+ Coverage   81.44%   81.46%   +0.02%     
==========================================
  Files         225      225              
  Lines       13750    13750              
  Branches     2412     2412              
==========================================
+ Hits        11198    11202       +4     
+ Misses       2277     2273       -4     
  Partials      275      275              
Flag Coverage Δ
suite.unit 81.46% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
packages/aws-cdk 80.86% <ø> (+0.05%) ⬆️
packages/aws-cdk-lib/core 82.10% <ø> (ø)

@rix0rrr rix0rrr merged commit d4845ce into main Jan 20, 2025
9 of 13 checks passed
@rix0rrr rix0rrr deleted the huijbers/sdkv3-update-tets branch January 20, 2025 16:15
@github-actions
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2025
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: f6c4fa7
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

contribution/core This is a PR that came from AWS. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants