Skip to content

feat: improve AWS credential errors - #918

Merged
cchu-ld merged 1 commit into
mainfrom
cchu/aws-devops-1-regions-creds
Oct 20, 2026
Merged

cchu-ld merged 1 commit into
mainfrom
cchu/aws-devops-1-regions-creds

Conversation

@cchu-ld

@cchu-ld cchu-ld commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

None.

Describe the solution you've provided

  • Problem: The credential error added the full AWS SDK cause. If no source had credentials, the cause was a long error from the EC2 metadata service, with retry counts and 169.254.169.254. That text did not help the user.
    • Solution: The error does not show the cause from the EC2 metadata service. Another cause, for example from a credential_process, still shows, because it tells what failed. errors.Unwrap still returns the cause.
  • Problem: The credential error told every profile to run aws sso login, also a profile that does not use SSO.
    • Solution: Only a profile whose SSO token is expired or not in the cache gets that advice. SSOTokenInvalid finds that case in each form that the SDK returns it: ssocreds.InvalidTokenError for a legacy sso_start_url profile, a wrapped fs.ErrNotExist for an sso_session profile with no cached token, and the text refresh cached SSO token failed for an sso_session token that expired. The SDK has no error type for the last case. Tests call the real SDK with real config files. Other SSO errors, for example a removed role assignment, show their cause, because a login does not fix them.
  • Problem: Without --profile or AWS_PROFILE, the error ignored the profile that the SDK used. A default profile with SSO got no SSO advice. A default profile with a broken credential_process got no AWS profile selected, and the cause was lost.
    • Solution: SelectedProfile reads AWS_PROFILE, then AWS_DEFAULT_PROFILE, then uses default, as the SDK does. The error then gives the advice or the cause for that profile.
  • Problem: The error listed every configured profile. With many profiles, the message was very long.
    • Solution: The error lists at most 5 profile names, then the count of the others, and points to aws configure list-profiles.

Describe alternatives you've considered

None.

Additional context

Testing: go test ./... and golangci-lint pass on this branch.

🤖 Generated with Claude Code


Note

Overview
Improves AWS credential failure messages when NewClients cannot resolve credentials, without changing how errors.Is(err, ErrNoCredentials) works.

User-facing text no longer appends EC2 IMDS noise (169.254.169.254, retries); the SDK cause stays on errors.Unwrap. Non-IMDS causes (e.g. credential_process) still surface when they explain the failure.

Messages are scenario-specific: SSO profiles get aws sso login only when SSOTokenInvalid detects a missing/expired token (new sso.go helpers parse config for sso_session / sso_start_url and mirror SDK error shapes). Non-SSO empty profiles get “has no AWS credentials”; profile selection follows --profile, AWS_PROFILE, then AWS_DEFAULT_PROFILE, then default. When nothing is selected, at most five profile names are listed plus a pointer to aws configure list-profiles.

Adds broad unit/integration-style tests in credentials_test.go and sso_test.go; existing NewClients credential tests are updated accordingly.

Reviewed by Cursor Bugbot for commit 35494a8. Bugbot is set up for automated code reviews on this repo. Configure here.

@cchu-ld

cchu-ld commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e45e486. Configure here.

@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-1-regions-creds branch 4 times, most recently from f26e330 to 35494a8 Compare October 20, 2026 18:36
@cchu-ld cchu-ld changed the title feat: add AWS regions, send the API version, and improve credential errors feat: improve AWS credential errors Oct 20, 2026
@cchu-ld
cchu-ld removed this pull request from stack #924 October 20, 2026 18:38
@cchu-ld
cchu-ld changed the base branch from main to cchu/aws-devops-0-regions-api October 20, 2026 18:38
@cchu-ld
cchu-ld added this pull request to stack #926 October 20, 2026 18:38
@cchu-ld
cchu-ld marked this pull request as ready for review October 20, 2026 18:43
@cchu-ld
cchu-ld requested a review from a team as a code owner October 20, 2026 18:43
@cchu-ld
cchu-ld requested review from a team, Andrewjeska and ld-ahartmann and removed request for a team October 20, 2026 18:43
Base automatically changed from cchu/aws-devops-0-regions-api to main October 20, 2026 19:02
The credential error is short. If the SSO token of the profile is
expired or not in the cache, the error gives the aws sso login advice.
SSOTokenInvalid finds this for sso_session profiles, which aws
configure sso writes, and for legacy sso_start_url profiles. Tests use
the real SDK with real config files.
Any other cause shows in the error, also for an SSO profile, except the
cause from the EC2 metadata service. That cause only says that no other
source had credentials. The error lists at most 5 profile names.
errors.Unwrap still returns the cause.

Without --profile, the profile comes from AWS_PROFILE, then
AWS_DEFAULT_PROFILE, then the default profile, as in the SDK.

sso.go adds SelectedProfile, SSOProfile, HasSSOConfig, and
SSOTokenInvalid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cchu-ld
cchu-ld force-pushed the cchu/aws-devops-1-regions-creds branch from 35494a8 to 8bfaa17 Compare October 20, 2026 19:02
@cchu-ld
cchu-ld merged commit ba627f2 into main Oct 20, 2026
9 checks passed
@cchu-ld
cchu-ld deleted the cchu/aws-devops-1-regions-creds branch October 20, 2026 19:09
@Andrewjeska

Copy link
Copy Markdown
Contributor

Post-merge thought for @cchu-ld — not a blocker, just something I noticed while reading through the stack:

HasSSOConfig in the new sso.go adds a second ad-hoc INI parser alongside knownProfiles in awsdevops.go. The AWS SDK already exposes config.LoadSharedConfigProfile, which returns a SharedConfig with SSOSession and SSOStartURL populated using the SDK's own parsing rules. Any reason not to use it, or at least share a parser with knownProfiles? Keeping two custom INI readers in the same package feels like a drift risk if AWS adds another SSO config shape (e.g. a new section type).

via LD Research 🤖

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