fix(cli): region specified in `~/.aws/credentials` is ignored #32133

iliapolo · 2024-11-14T11:49:08Z

We just didn't consider the shared credentials file as returned by the SDK when loading configuration.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts

codecov · 2024-11-14T12:00:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.19%. Comparing base (6b0b1e1) to head (7663a56).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #32133      +/-   ##
==========================================
+ Coverage   77.17%   77.19%   +0.02%     
==========================================
  Files         105      105              
  Lines        7162     7164       +2     
  Branches     1311     1311              
==========================================
+ Hits         5527     5530       +3     
+ Misses       1455     1454       -1     
  Partials      180      180

Flag	Coverage Δ
suite.unit	`77.19% <100.00%> (+0.02%)`	⬆️

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

Components	Coverage Δ
packages/aws-cdk	`77.19% <100.00%> (+0.02%)`	⬆️

iliapolo · 2024-11-14T13:18:41Z

Exemption from integ because unit tests cover the offending code.

rix0rrr · 2024-11-14T13:57:44Z

AWS CLI does:

If --profile: credentials profile > config profile > no region
If not --profile: credentials default > config default > no region

No region behaves as follows:

Global services work (IAM, S3, Route53)
Regional services fail with an error, saying you must specify a region

Traditionally, CDK has defaulted to us-east-1 in this case. I'm like 90% sure we used to do that to copy the AWS CLI, I wonder if they changed their behavior when migrating CLIv1 -> CLIv2

Also CDK traditionally had the behavior that if the profile doesn't have a region, we will take the region from the [default] profile, but apparently the CLI doesn't do that.

EDIT: Ohh, AWS CLIv2 now has a magic aws-global region, that they default to.

iliapolo · 2024-11-14T14:06:23Z

draft until we sort our credentials priority

packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts

…32134) In Node.js, if you assign `undefined` to an environment variable, that variable ends up having the string `"undefined"`. If we are using IAM user credentials, `AWS_SESSION_TOKEN` should not be set, but because we were not handling this edge case, it was getting assigned an invalid value: ``` Welcome to Node.js v22.9.0. Type ".help" for more information. > process.env.AWS_SESSION_TOKEN || process.env.AMAZON_SESSION_TOKEN undefined > process.env.AWS_SESSION_TOKEN = process.env.AWS_SESSION_TOKEN || process.env.AMAZON_SESSION_TOKEN undefined > process.env.AWS_SESSION_TOKEN 'undefined' ``` Closes #32120. - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

### Reason for this change We don't need to use the third party `source-map-support` package anymore to achieve the same result. ### Description of changes We use `process.setSourceMapsEnabled(true)` instead. However unlike the previous package, this command needs to be run _before_ we import any other files, otherwise it won't work. We therefore move it into the executable. ### Description of how you validated changes Manual verification ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws-cdk-automation · 2024-11-14T17:38:03Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

mergify · 2024-11-14T20:21:22Z

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).

aws-cdk-automation · 2024-11-14T20:51:06Z

AWS CodeBuild CI Report

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

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

mergify · 2024-11-14T20:51:23Z

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).

github-actions · 2024-11-14T20:51:36Z

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.

mid work

8e68a1f

aws-cdk-automation requested a review from a team November 14, 2024 11:49

github-actions bot added bug This issue is a bug. p2 labels Nov 14, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label Nov 14, 2024

iliapolo had a problem deploying to test-pipeline November 14, 2024 11:50 — with GitHub Actions Failure

aws-cdk-automation previously requested changes Nov 14, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Nov 14, 2024

iliapolo commented Nov 14, 2024

View reviewed changes

packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts Outdated Show resolved Hide resolved

unit tests

32116ba

iliapolo had a problem deploying to test-pipeline November 14, 2024 12:45 — with GitHub Actions Failure

iliapolo marked this pull request as ready for review November 14, 2024 12:46

iliapolo added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Nov 14, 2024

mid work

ea4bf96

iliapolo had a problem deploying to test-pipeline November 14, 2024 13:31 — with GitHub Actions Failure

iliapolo marked this pull request as draft November 14, 2024 14:06

iliapolo added 3 commits November 14, 2024 17:21

change priorities

125e822

mid work

4919bf8

Merge branch 'main' into epolon/cli-region-from-credentials-file

b472ae0

iliapolo marked this pull request as ready for review November 14, 2024 15:27

iliapolo requested a review from rix0rrr November 14, 2024 15:27

iliapolo had a problem deploying to test-pipeline November 14, 2024 15:27 — with GitHub Actions Failure

rix0rrr reviewed Nov 14, 2024

View reviewed changes

packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts Show resolved Hide resolved

rix0rrr approved these changes Nov 14, 2024

View reviewed changes

otaviomacedo and others added 2 commits November 14, 2024 19:16

iliapolo had a problem deploying to test-pipeline November 14, 2024 17:20 — with GitHub Actions Failure

Merge branch 'main' into epolon/cli-region-from-credentials-file

23ed98a

iliapolo had a problem deploying to test-pipeline November 14, 2024 17:23 — with GitHub Actions Failure

fix tests

aa9fb79

iliapolo temporarily deployed to test-pipeline November 14, 2024 17:36 — with GitHub Actions Inactive

iliapolo added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Nov 14, 2024

Merge branch 'main' into epolon/cli-region-from-credentials-file

b3c4571

aws-cdk-automation removed the pr/needs-cli-test-run This PR needs CLI tests run against it. label Nov 14, 2024

iliapolo had a problem deploying to test-pipeline November 14, 2024 19:53 — with GitHub Actions Failure

Merge branch 'main' into epolon/cli-region-from-credentials-file

7663a56

mergify bot had a problem deploying to test-pipeline November 14, 2024 20:22 Failure

mergify bot merged commit d1b3c81 into main Nov 14, 2024
16 of 17 checks passed

mergify bot deleted the epolon/cli-region-from-credentials-file branch November 14, 2024 20:51

github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): region specified in `~/.aws/credentials` is ignored #32133

fix(cli): region specified in `~/.aws/credentials` is ignored #32133

iliapolo commented Nov 14, 2024 •

edited

Loading

aws-cdk-automation left a comment

codecov bot commented Nov 14, 2024 •

edited

Loading

iliapolo commented Nov 14, 2024

rix0rrr commented Nov 14, 2024 •

edited

Loading

iliapolo commented Nov 14, 2024

aws-cdk-automation commented Nov 14, 2024

mergify bot commented Nov 14, 2024

aws-cdk-automation commented Nov 14, 2024

mergify bot commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

fix(cli): region specified in ~/.aws/credentials is ignored #32133

fix(cli): region specified in ~/.aws/credentials is ignored #32133

Conversation

iliapolo commented Nov 14, 2024 • edited Loading

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 14, 2024 • edited Loading

Codecov Report

iliapolo commented Nov 14, 2024

rix0rrr commented Nov 14, 2024 • edited Loading

iliapolo commented Nov 14, 2024

aws-cdk-automation commented Nov 14, 2024

mergify bot commented Nov 14, 2024

aws-cdk-automation commented Nov 14, 2024

AWS CodeBuild CI Report

mergify bot commented Nov 14, 2024

github-actions bot commented Nov 14, 2024

fix(cli): region specified in `~/.aws/credentials` is ignored #32133

fix(cli): region specified in `~/.aws/credentials` is ignored #32133

iliapolo commented Nov 14, 2024 •

edited

Loading

codecov bot commented Nov 14, 2024 •

edited

Loading

rix0rrr commented Nov 14, 2024 •

edited

Loading