feat: implement AWS ECR credentials loader in Kubernetes keychains by Nachiket-Roy · Pull Request #3864 · knative/func

Nachiket-Roy · 2026-05-28T20:31:39Z

Summary

This change adds programmatic AWS Elastic Container Registry (ECR) authentication support when resolving OCI registry keychains, aligning it with the existing GCP (Google Container Registry) and Azure (ACR) patterns. Previously, the ECR credentials loader was left unimplemented (GetECRCredentialLoader returned an empty slice), which prevented automatic, programmatic ECR credential resolution when pushing/pulling images in Kubernetes keychains workflows.

What changes were made?

Registry Matching (isECRRegistry): Added matching logic to detect if a registry hostname corresponds to an AWS ECR registry. This includes public.ecr.aws as well as private registry formats across various AWS partitions.
ECR Helper Integration (GetECRCredentialLoader): Leveraged the official amazon-ecr-credential-helper/ecr-login library programmatically, wrapped via authn.NewKeychainFromHelper().
Graceful Fallback: Silenced standard library logging with io.Discard and ensured that if no ambient credentials exist, creds.ErrCredentialsNotFound is returned, letting subsequent credential loaders in the chain attempt authentication.

Testing

Unit test was added (pkg/k8s/keychains_test.go)
Test has been verified locally

Closes : #3863

knative-prow · 2026-05-28T20:31:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Nachiket-Roy
Once this PR has been reviewed and has the lgtm label, please assign lkingland for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

linux-foundation-easycla · 2026-05-28T20:31:44Z

The committers listed above are authorized under a signed CLA.

✅ login: Nachiket-Roy / name: Nachiket Roy (9df4cbc)

knative-prow · 2026-05-28T20:31:48Z

Welcome @Nachiket-Roy! It looks like this is your first PR to knative/func 🎉

knative-prow · 2026-05-28T20:31:49Z

Hi @Nachiket-Roy. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copilot

Pull request overview

This PR adds programmatic AWS ECR authentication support to the Kubernetes keychain credential loader pipeline, aligning it with the existing GCR and ACR loaders so image push/pull can automatically resolve ECR credentials.

Changes:

Add ECR registry hostname detection (isECRRegistry) and wire an ECR credential loader using amazon-ecr-credential-helper via authn.NewKeychainFromHelper.
Return creds.ErrCredentialsNotFound when the loader should not apply (non‑ECR registries) to allow other loaders to proceed.
Add unit tests for ECR registry detection and basic loader behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
pkg/k8s/keychains.go	Implements ECR registry detection and an ECR credential loader using the AWS ECR credential helper.
pkg/k8s/keychains_test.go	Adds tests for ECR registry detection and ECR loader fallback behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

matejvasek · 2026-05-29T14:11:43Z

/ok-to-test

codecov · 2026-05-29T14:21:33Z

Codecov Report

❌ Patch coverage is 50.00000% with 56 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.71%. Comparing base (f6d9270) to head (0b649c4).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/k8s/keychains.go	50.00%	52 Missing and 4 partials ⚠️

❗ There is a different number of reports uploaded between BASE (f6d9270) and HEAD (0b649c4). Click for more details.

HEAD has 12 uploads less than BASE

Flag BASE (f6d9270) HEAD (0b649c4)

e2e 3 0

e2e-config-ci 1 0

e2e springboot 1 0

e2e node 1 0

e2e typescript 1 0

e2e go 1 0

e2e rust 1 0

e2e quarkus 1 0

e2e python 1 0

integration 1 0

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3864       +/-   ##
===========================================
- Coverage   53.41%   42.71%   -10.70%     
===========================================
  Files         200      200               
  Lines       23413    23524      +111     
===========================================
- Hits        12505    10049     -2456     
- Misses       9658    12581     +2923     
+ Partials     1250      894      -356

Flag	Coverage Δ
e2e	`?`
e2e go	`?`
e2e node	`?`
e2e python	`?`
e2e quarkus	`?`
e2e rust	`?`
e2e springboot	`?`
e2e typescript	`?`
e2e-config-ci	`?`
integration	`?`
unit macos-14	`42.27% <49.43%> (+0.04%)`	⬆️
unit macos-latest	`42.27% <49.43%> (+0.04%)`	⬆️
unit ubuntu-24.04-arm	`42.57% <50.00%> (+0.04%)`	⬆️
unit ubuntu-latest	`43.13% <49.43%> (+0.03%)`	⬆️
unit windows-latest	`42.34% <49.43%> (+0.04%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

matejvasek · 2026-06-01T09:22:46Z

+			if !isECRRegistry(registry) {
+				return oci.Credentials{}, creds.ErrCredentialsNotFound
+			}


Do we really need this pre-check? Won't keychain.Resolve(res) do the job on it's own?

To fail fast and same pattern is followed in GCP and ACR

…credential loader

feat: implement AWS ECR credentials loader in Kubernetes keychains

9df4cbc

knative-prow Bot requested review from dsimansk and jrangelramos May 28, 2026 20:31

knative-prow Bot added size/L 🤖 PR changes 100-499 lines, ignoring generated files. needs-ok-to-test 🤖 Needs an org member to approve testing labels May 28, 2026

gauron99 requested review from Copilot and removed request for dsimansk and jrangelramos May 29, 2026 06:08

Copilot started reviewing on behalf of gauron99 May 29, 2026 06:08 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread pkg/k8s/keychains.go

Comment thread pkg/k8s/keychains_test.go

Comment thread pkg/k8s/keychains_test.go

Nachiket-Roy marked this pull request as draft May 29, 2026 07:03

knative-prow Bot added the do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. label May 29, 2026

Added spoofing suffix check test to prevent regression

1a81770

Nachiket-Roy marked this pull request as ready for review May 29, 2026 11:18

knative-prow Bot removed the do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. label May 29, 2026

knative-prow Bot requested review from dsimansk and jrangelramos May 29, 2026 11:18

Nachiket-Roy requested a review from Copilot May 29, 2026 11:19

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread pkg/k8s/keychains_test.go Outdated

Comment thread pkg/k8s/keychains.go Outdated

Comment thread pkg/k8s/keychains.go Outdated

Comment thread pkg/k8s/keychains.go

Comment thread pkg/k8s/keychains.go

Nachiket-Roy marked this pull request as draft May 29, 2026 11:24

knative-prow Bot added the do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. label May 29, 2026

Added error translation for ambient cred

9dd09ea

Nachiket-Roy marked this pull request as ready for review May 29, 2026 11:30

knative-prow Bot removed the do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. label May 29, 2026

Nachiket-Roy requested a review from Copilot May 29, 2026 11:30

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread pkg/k8s/keychains.go Outdated

Comment thread pkg/k8s/keychains_test.go Outdated

Added helper for ErrCredentialsNotFound

aa24833

Nachiket-Roy requested a review from Copilot May 29, 2026 11:34

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread pkg/k8s/keychains.go

Comment thread pkg/k8s/keychains.go

Comment thread pkg/k8s/keychains.go

improved isAWSCredentialsNotFound()

7c519a3

knative-prow Bot added ok-to-test 🤖 Non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test 🤖 Needs an org member to approve testing labels May 29, 2026

Update go.mod dependencies

753b9b6

matejvasek reviewed Jun 1, 2026

View reviewed changes

Nachiket-Roy added 3 commits June 1, 2026 13:20

add Timeout for ecr loader

bb5bd2a

resolve the KEDA pipeline test timeout failure by optimizing the ECR …

0b649c4

…credential loader

Merge branch 'knative:main' into feat/ecr-cred-loader

4652b0e

Conversation

Nachiket-Roy commented May 28, 2026

Summary

What changes were made?

Testing

Uh oh!

knative-prow Bot commented May 28, 2026

Uh oh!

linux-foundation-easycla Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knative-prow Bot commented May 28, 2026

Uh oh!

knative-prow Bot commented May 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matejvasek commented May 29, 2026

Uh oh!

codecov Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

matejvasek Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Nachiket-Roy Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

linux-foundation-easycla Bot commented May 28, 2026 •

edited

Loading

codecov Bot commented May 29, 2026 •

edited

Loading