ACR Login: Use Azure SDK library to get the credential and token by dbalseiro · Pull Request #3398 · knative/func

dbalseiro · 2026-01-26T18:57:07Z

Changes

Add Azure SDK for Go as a dependency
Use Azure SDK for GO to get credentials and token to log in to [Azure Container Registry[(https://learn.microsoft.com/en-us/cli/azure/msal-based-azure-cli?view=azure-cli-latest)

/kind bug

Release Note

`func` no longer relies on Azure CLI’s deprecated `accessToken.json` file to authenticate with Azure Container Registry. Authentication now uses Azure’s supported credential mechanisms (e.g. Azure CLI, environment credentials, managed identity). No action is required for most users.

linux-foundation-easycla · 2026-01-26T18:57:15Z

The committers listed above are authorized under a signed CLA.

✅ login: dbalseiro / name: Diego Balseiro (615673b, 6609dd8, 8ebe95c, 9aa595b)

knative-prow · 2026-01-26T18:57:17Z

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

knative-prow · 2026-01-26T18:57:18Z

Hi @dbalseiro. 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.

lkingland · 2026-02-09T07:43:05Z

/ok-to-test

Thanks for the PR!

codecov · 2026-02-17T13:05:50Z

Codecov Report

❌ Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.54%. Comparing base (94711b0) to head (8ebe95c).
⚠️ Report is 32 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/k8s/keychains.go	0.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3398      +/-   ##
==========================================
+ Coverage   54.50%   54.54%   +0.03%     
==========================================
  Files         173      179       +6     
  Lines       19694    20260     +566     
==========================================
+ Hits        10735    11051     +316     
- Misses       7836     8037     +201     
- Partials     1123     1172      +49

Flag	Coverage Δ
e2e	`38.43% <0.00%> (+0.48%)`	⬆️
e2e go	`32.64% <0.00%> (-0.98%)`	⬇️
e2e node	`28.55% <0.00%> (-0.75%)`	⬇️
e2e python	`32.15% <0.00%> (-0.98%)`	⬇️
e2e quarkus	`28.67% <0.00%> (-0.77%)`	⬇️
e2e rust	`28.11% <0.00%> (-0.72%)`	⬇️
e2e springboot	`26.54% <0.00%> (-0.72%)`	⬇️
e2e typescript	`28.66% <0.00%> (-0.75%)`	⬇️
integration	`17.53% <0.00%> (+0.96%)`	⬆️
unit macos-14	`42.25% <0.00%> (-1.05%)`	⬇️
unit macos-latest	`42.25% <0.00%> (-1.05%)`	⬇️
unit ubuntu-24.04-arm	`42.56% <0.00%> (-1.06%)`	⬇️
unit ubuntu-latest	`43.14% <0.00%> (-1.08%)`	⬇️
unit windows-latest	`42.27% <0.00%> (-1.05%)`	⬇️

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.

lkingland

Looks good! I have one minor improvement worth at least exploring the feasability of (plumbing through a context object) and one concern worth double-checking (that there's no other code depending on creds.ErrCredentialsNotFound being returned).

/approve
/hold pending possible changes per the above

lkingland · 2026-02-17T13:02:24Z

pkg/k8s/keychains.go

-					}, nil
-				}
+			scope := "https://containerregistry.azure.net/.default"
+			token, err := azCredentials.GetToken(context.Background(), policy.TokenRequestOptions{


We might want to plumb through a context object to GetACRCredentialLoader in case this thing can lock up or need canceling.

giving it a try :)

lkingland · 2026-02-17T13:06:18Z

pkg/k8s/keychains.go

+			if err != nil {
+				return oci.Credentials{}, fmt.Errorf("failed to get azure access token: %w", err)
 			}
-			return oci.Credentials{}, creds.ErrCredentialsNotFound


I notice that creds.ErrCredentialsNotFound is no longer returned. This sentinel value may be depended upon by other systems to fall-through (despite using error types as flow-control being an antipattern; it's life). It might be a good idea to double-check that there's no other code depending on exactly that error type to ensure we're not breaking any "error fallback" systems if one of the other generic errors is returned instead.

Yes, we are still returning ErrCredentialsNotFound when the registry's domain is not azure.io. And if I'm not mistaken, returning ErrCredentialsNotFound in this line made sense at the time, because it was trying to fetch the token from a JSON file. But now we try to generate the token using the SDK. I think if the SDK fails, we should return a plain error. What do you think?

knative-prow · 2026-02-17T13:09:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dbalseiro, lkingland

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [lkingland]

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

knative-prow · 2026-03-14T02:23:07Z

New changes are detected. LGTM label has been removed.

dbalseiro

@lkingland Thanks for the review! Sorry it took me so long to do it, but I addressed your comments. Let me know what you think.

dbalseiro · 2026-03-13T22:27:31Z

pkg/k8s/keychains.go

+			if err != nil {
+				return oci.Credentials{}, fmt.Errorf("failed to get azure access token: %w", err)
 			}
-			return oci.Credentials{}, creds.ErrCredentialsNotFound


Yes, we are still returning ErrCredentialsNotFound when the registry's domain is not azure.io. And if I'm not mistaken, returning ErrCredentialsNotFound in this line made sense at the time, because it was trying to fetch the token from a JSON file. But now we try to generate the token using the SDK. I think if the SDK fails, we should return a plain error. What do you think?

dbalseiro · 2026-03-14T02:23:32Z

pkg/k8s/keychains.go

-					}, nil
-				}
+			scope := "https://containerregistry.azure.net/.default"
+			token, err := azCredentials.GetToken(context.Background(), policy.TokenRequestOptions{


giving it a try :)

ACR Login: Use Azure SDK library to get the credential and token

615673b

knative-prow bot added do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. kind/bug Bugs labels Jan 26, 2026

knative-prow bot requested review from dsimansk and jrangelramos January 26, 2026 18:57

knative-prow bot added needs-ok-to-test 🤖 Needs an org member to approve testing size/XS 🤖 PR changes 0-9 lines, ignoring generated files. labels Jan 26, 2026

Add Azure SDK and implement ACR Login

9aa595b

knative-prow bot added size/M 🤖 PR changes 30-99 lines, ignoring generated files. and removed size/XS 🤖 PR changes 0-9 lines, ignoring generated files. labels Feb 3, 2026

dbalseiro marked this pull request as ready for review February 3, 2026 22:11

knative-prow bot removed the do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. label Feb 3, 2026

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 Feb 9, 2026

dbalseiro mentioned this pull request Feb 10, 2026

Use Azure SDK library to get the credential and token stackbuilders/func#2

Closed

run update-codegen

8ebe95c

lkingland approved these changes Feb 17, 2026

View reviewed changes

knative-prow bot added the do-not-merge/hold 🤖 PR should not merge because someone has issued a /hold command. label Feb 17, 2026

knative-prow bot assigned lkingland Feb 17, 2026

knative-prow bot added the lgtm 🤖 PR is ready to be merged. label Feb 17, 2026

knative-prow bot added the approved 🤖 PR has been approved by an approver from all required OWNERS files. label Feb 17, 2026

Unify credential loader interface with context support; add ACR tests

6609dd8

knative-prow bot removed the lgtm 🤖 PR is ready to be merged. label Mar 14, 2026

knative-prow bot added size/L 🤖 PR changes 100-499 lines, ignoring generated files. and removed size/M 🤖 PR changes 30-99 lines, ignoring generated files. labels Mar 14, 2026

dbalseiro commented Mar 14, 2026

View reviewed changes

Conversation

dbalseiro commented Jan 26, 2026

Changes

Uh oh!

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

Uh oh!

Uh oh!

knative-prow bot commented Jan 26, 2026

Uh oh!

knative-prow bot commented Jan 26, 2026

Uh oh!

lkingland commented Feb 9, 2026

Uh oh!

codecov bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lkingland left a comment

Choose a reason for hiding this comment

Uh oh!

lkingland Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

dbalseiro Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

lkingland Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

dbalseiro Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

knative-prow bot commented Feb 17, 2026

Uh oh!

knative-prow bot commented Mar 14, 2026

Uh oh!

dbalseiro left a comment

Choose a reason for hiding this comment

Uh oh!

dbalseiro Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

dbalseiro Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

linux-foundation-easycla bot commented Jan 26, 2026 •

edited

Loading

codecov bot commented Feb 17, 2026 •

edited

Loading