fix: restore AWS credential chain resolution for ECRAuthorizationToken generator by aditmeno · Pull Request #5082 · external-secrets/external-secrets

aditmeno · 2025-07-29T23:42:27Z

Problem Statement

Since ESO v0.18.0, the ECRAuthorizationToken generator fails when no explicit AWS credentials are specified, breaking environments that rely on the controller's authentication context.

Additionally, during testing, we discovered that SecretStore validation errors were not using the correct error reason, causing test failures.

Related Issues

Fixes #4935

Proposed Changes

1. Changes to `pkg/provider/aws/auth/auth.go`:

Replace manual AWS config construction with the SDK's default configuration loader to restore credential chain resolution.

Before:

config := aws.NewConfig()

After:

awscfg, err := config.LoadDefaultConfig(ctx)

Why this fixes the issue:

The previous approach created an empty configuration that bypassed AWS's credential resolution chain
The SDK's default loader automatically discovers credentials from multiple sources in the correct precedence order
This restoration allows the generator to inherit authentication from the ESO controller's execution context
Maintains compatibility with explicit credentials while fixing the implicit credential flow

2. Changes to `pkg/controllers/secretstore/common.go`:

Map ValidationResultError to ReasonInvalidProviderConfig to align with test expectations.

Change:

// Use ReasonInvalidProviderConfig for validation errors that indicate
// invalid configuration (like empty server URL)
reason := esapi.ReasonValidationFailed
if validationResult == esapi.ValidationResultError {
    reason = esapi.ReasonInvalidProviderConfig
}

Why this change is needed:

Tests expect ReasonInvalidProviderConfig when a SecretStore has invalid configuration
The Vault provider returns ValidationResultError for configuration issues like empty server URL
This mapping ensures users get the appropriate error reason for configuration problems

3. Test updates:

pkg/generator/ecr/ecr_test.go: Added context parameter to test cases
pkg/generator/sts/sts_test.go: Added context parameter to test cases to align with the new AWS config loader pattern

Testing

All SecretStore controller tests now pass
ECRAuthorizationToken generator works correctly with implicit credentials
Proper error reasons are reported for invalid provider configurations

Checklist

I have read the contribution guidelines
All commits are signed with git commit --signoff
My changes have reasonable test coverage
All tests pass with make test
I ensured my PR is ready for review with make reviewable

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

When validateStore fails, the controller was returning an error immediately which prevented the deferred status patch from executing. This caused test failures where status conditions were not being persisted. Changes: - Return with RequeueAfter instead of error when validation fails to allow status update to complete - Fix patch creation timing by capturing original state before modifications - Use ReasonInvalidProviderConfig for ValidationResultError to match test expectations This ensures status conditions are properly set and persisted even when store validation fails, providing better feedback to users about configuration issues. Signed-off-by: Aditya Menorahalli <aditya@canary.ws> Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

pkg/provider/aws/auth/auth.go

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

aditmeno · 2025-07-30T23:20:45Z

@Skarlso Could you review my PR?

sonarqubecloud · 2025-07-30T23:44:17Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [external-secrets](https://github.com/external-secrets/external-secrets) | minor | `0.18.2` -> `0.19.0` | --- ### Release Notes <details> <summary>external-secrets/external-secrets (external-secrets)</summary> ### [`v0.19.0`](https://github.com/external-secrets/external-secrets/releases/tag/v0.19.0) [Compare Source](external-secrets/external-secrets@v0.18.2...v0.19.0) #### **BREAKING CHANGE** 🔴 🔴 BREAKING CHANGE 🔴 🔴 Please note that this a breaking change because our CRDs are now too big. Meaning a simple kubectl apply or Argo's default client side apply WILL NOT WORK! You have to add `--server-side` to kubectl apply and in argo add: ```yaml spec: project: default syncPolicy: automated: prune: true selfHeal: true syncOptions: - CreateNamespace=true - ServerSideApply=true ``` for it to correctly install the CRDs. Thank you. Image: `ghcr.io/external-secrets/external-secrets:v0.19.0` Image: `ghcr.io/external-secrets/external-secrets:v0.19.0-ubi` Image: `ghcr.io/external-secrets/external-secrets:v0.19.0-ubi-boringssl` #### What's Changed - chore: release helm chart for v0.18.2 by [@Skarlso](https://github.com/Skarlso) in external-secrets/external-secrets#4985 - chore(deps): bump golang from `ee7ff13` to `10f549d` in /e2e by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#4997 - chore(deps): bump golang from `68932fa` to `68932fa` by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5000 - chore(deps): bump mkdocs-material from 9.6.14 to 9.6.15 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#4998 - chore(deps): bump anchore/sbom-action from 0.20.1 to 0.20.2 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5001 - chore(deps): bump github/codeql-action from 3.29.1 to 3.29.2 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5003 - chore(deps): bump aquasecurity/trivy-action from 0.31.0 to 0.32.0 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5002 - fix: do not turn original value into string on value scope by [@Skarlso](https://github.com/Skarlso) in external-secrets/external-secrets#5011 - fix: add uuid in edit and view clusterroles by [@sylvainOL](https://github.com/sylvainOL) in external-secrets/external-secrets#5017 - chore: update dependencies by [@eso-service-account-app](https://github.com/eso-service-account-app)\[bot] in external-secrets/external-secrets#4999 - fix: template data should not be the secret Data itself by [@gusfcarvalho](https://github.com/gusfcarvalho) in external-secrets/external-secrets#5023 - Fix: Return appropriate error in ValidateStore by [@prakash-218](https://github.com/prakash-218) in external-secrets/external-secrets#5019 - feat(helm): allow to set init containers by [@rclsilver](https://github.com/rclsilver) in external-secrets/external-secrets#4745 - chore(deps): bump certifi from 2025.6.15 to 2025.7.14 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5032 - Fix: Remove root/buildinfo from ubi build files by [@bainsy88](https://github.com/bainsy88) in external-secrets/external-secrets#5037 - chore(deps): bump ubi8/ubi from `19eae3d` to `c0b0729` by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5033 - chore(deps): bump golang from 1.24.4-bookworm to 1.24.5-bookworm in /e2e by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5029 - chore(deps): bump golang from 1.24.4 to 1.24.5 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5034 - chore: update dependencies by [@eso-service-account-app](https://github.com/eso-service-account-app)\[bot] in external-secrets/external-secrets#5031 - Add Red Hat OpenShift in Adopters by [@KeenonLee](https://github.com/KeenonLee) in external-secrets/external-secrets#5039 - fix: remove authentication option with JWT token from STSSessionToken generator by [@Skarlso](https://github.com/Skarlso) in external-secrets/external-secrets#5026 - fix: add validation constraints to ExternalSecretRewrite by [@Aakkash-Suresh](https://github.com/Aakkash-Suresh) in external-secrets/external-secrets#5006 - fix: stability support matrix by [@gusfcarvalho](https://github.com/gusfcarvalho) in external-secrets/external-secrets#5043 - docs(decoding-strategy): clarify base64 auto-detection limitations by [@orymate](https://github.com/orymate) in external-secrets/external-secrets#5004 - feat(infisical): auth methods by [@DanielHougaard](https://github.com/DanielHougaard) in external-secrets/external-secrets#5040 - chore(deps): bump alpine from 3.22.0 to 3.22.1 in /e2e by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5046 - chore(aws): parameterstore unit tests improvement by [@ivankatliarchuk](https://github.com/ivankatliarchuk) in external-secrets/external-secrets#4986 - fix(helm): grafana dashboard: fix heatmaps to actually be heatmaps, not time series by [@desaintmartin](https://github.com/desaintmartin) in external-secrets/external-secrets#5069 - chore(deps): bump sigstore/cosign-installer from 3.9.1 to 3.9.2 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5047 - chore(deps): bump step-security/harden-runner from 2.12.2 to 2.13.0 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5048 - chore(deps): bump golang from `ddf5200` to `daae04e` by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5049 - chore(deps): bump alpine from `8a1f59f` to `4bcff63` by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5051 - chore(deps): bump alpine from `8a1f59f` to `4bcff63` in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5052 - chore(deps): bump mkdocs-material from 9.6.15 to 9.6.16 in /hack/api-docs by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5077 - Add SelfSubjectAccessReview as a fallback for failing SelfSubjectRulesReview by [@alvin-rw](https://github.com/alvin-rw) in external-secrets/external-secrets#5025 - chore(deps): bump golang from `69adc37` to `ef8c5c7` in /e2e by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5076 - chore(deps): bump ubi8/ubi from `c0b0729` to `785d38c` by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5075 - chore(deps): bump github/codeql-action from 3.29.2 to 3.29.4 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5072 - chore(deps): bump anchore/sbom-action from 0.20.2 to 0.20.4 by [@dependabot](https://github.com/dependabot)\[bot] in external-secrets/external-secrets#5073 - SSHKey generator by [@dex4er](https://github.com/dex4er) in external-secrets/external-secrets#5083 - fix: restore AWS credential chain resolution for ECRAuthorizationToken generator by [@aditmeno](https://github.com/aditmeno) in external-secrets/external-secrets#5082 - fix(helm): grafana dashboard: add widget for sum of not ready secrets by [@desaintmartin](https://github.com/desaintmartin) in external-secrets/external-secrets#5086 - feat(aws): secretsmanager to update/patch/delete tags by [@ivankatliarchuk](https://github.com/ivankatliarchuk) in external-secrets/external-secrets#4984 - fix: update the e2e test with the new store status value by [@Skarlso](https://github.com/Skarlso) in external-secrets/external-secrets#5089 - fix: correct usage of if in dlc and update for server side apply by [@Skarlso](https://github.com/Skarlso) in external-secrets/external-secrets#5092 #### New Contributors - [@sylvainOL](https://github.com/sylvainOL) made their first contribution in external-secrets/external-secrets#5017 - [@prakash-218](https://github.com/prakash-218) made their first contribution in external-secrets/external-secrets#5019 - [@rclsilver](https://github.com/rclsilver) made their first contribution in external-secrets/external-secrets#4745 - [@bainsy88](https://github.com/bainsy88) made their first contribution in external-secrets/external-secrets#5037 - [@KeenonLee](https://github.com/KeenonLee) made their first contribution in external-secrets/external-secrets#5039 - [@orymate](https://github.com/orymate) made their first contribution in external-secrets/external-secrets#5004 - [@desaintmartin](https://github.com/desaintmartin) made their first contribution in external-secrets/external-secrets#5069 - [@alvin-rw](https://github.com/alvin-rw) made their first contribution in external-secrets/external-secrets#5025 - [@dex4er](https://github.com/dex4er) made their first contribution in external-secrets/external-secrets#5083 - [@aditmeno](https://github.com/aditmeno) made their first contribution in external-secrets/external-secrets#5082 **Full Changelog**: external-secrets/external-secrets@v0.18.2...v0.19.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/1114 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>

aditmeno requested a review from a team as a code owner July 29, 2025 23:42

aditmeno requested a review from moolen July 29, 2025 23:42

aditmeno marked this pull request as draft July 29, 2025 23:42

aditmeno force-pushed the main branch from 8d04c71 to 2b62d53 Compare July 30, 2025 02:01

aditmeno changed the title ~~Fix ECRAuthorizationToken not work in 0.18.0~~ Fix ECRAuthorizationToken not working in 0.18.0 Jul 30, 2025

aditmeno changed the title ~~Fix ECRAuthorizationToken not working in 0.18.0~~ fix(aws): restore implicit credential support for ECRAuthorizationToken generator Jul 30, 2025

aditmeno changed the title ~~fix(aws): restore implicit credential support for ECRAuthorizationToken generator~~ fix: restore AWS credential chain resolution for ECRAuthorizationToken generator Jul 30, 2025

aditmeno marked this pull request as ready for review July 30, 2025 02:11

aditmeno force-pushed the main branch 2 times, most recently from 1744aca to a539a7d Compare July 30, 2025 02:14

restore AWS default credential chain

cc55c0c

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

aditmeno force-pushed the main branch from a539a7d to b7f0efa Compare July 30, 2025 02:17

aditmeno force-pushed the main branch from b7f0efa to 2eabd63 Compare July 30, 2025 02:17

add tests for auth

6908236

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

Skarlso reviewed Jul 30, 2025

View reviewed changes

pkg/provider/aws/auth/auth.go Outdated Show resolved Hide resolved

aditmeno and others added 3 commits July 30, 2025 10:04

add error handling for config.LoadDefaultConfig(ctx)

182cf82

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

Merge branch 'main' into main

9977d34

remove whitspace in tests ctx.Background()

6da95dd

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

aditmeno force-pushed the main branch from a0354b0 to 6da95dd Compare July 30, 2025 08:22

Merge branch 'main' into main

ce89411

aditmeno requested a review from Skarlso July 30, 2025 08:27

Merge branch 'main' into main

35e437b

Skarlso approved these changes Jul 31, 2025

View reviewed changes

Skarlso merged commit 7acab69 into external-secrets:main Jul 31, 2025
20 checks passed

josemaia mentioned this pull request Jul 31, 2025

Allow ECRAuthorizationToken usage with controller credentials #5007

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: restore AWS credential chain resolution for ECRAuthorizationToken generator#5082

fix: restore AWS credential chain resolution for ECRAuthorizationToken generator#5082
Skarlso merged 8 commits intoexternal-secrets:mainfrom
aditmeno:main

aditmeno commented Jul 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

aditmeno commented Jul 30, 2025

Uh oh!

sonarqubecloud bot commented Jul 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aditmeno commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem Statement

Related Issues

Proposed Changes

1. Changes to pkg/provider/aws/auth/auth.go:

2. Changes to pkg/controllers/secretstore/common.go:

3. Test updates:

Testing

Checklist

Uh oh!

Uh oh!

aditmeno commented Jul 30, 2025

Uh oh!

sonarqubecloud bot commented Jul 30, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aditmeno commented Jul 29, 2025 •

edited

Loading

1. Changes to `pkg/provider/aws/auth/auth.go`:

2. Changes to `pkg/controllers/secretstore/common.go`: