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

-					}, 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

+			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-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

+			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

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


giving it a try :)

lkingland · 2026-04-21T05:06:11Z

/ok-to-test

dbalseiro · 2026-05-19T18:25:18Z

/retest

dbalseiro · 2026-05-19T18:56:37Z

@lkingland rebased and retested :)
Thank you!

dbalseiro · 2026-06-08T21:32:38Z

Hello everyone! any news on this PR?

knative-prow · 2026-06-15T19:53:02Z

[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-06-15T19:53:03Z

[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

dbalseiro · 2026-06-15T19:54:56Z

Hey @lkingland, I rebased this again, but I closed the PR by mistake and then reopened it 😓. Could you please take a look again and see if this is ready to merge -- sorry about the troubles

gauron99 · 2026-08-21T22:17:37Z

/retest

gauron99 · 2026-08-21T22:21:46Z

@dbalseiro the branch is only a test name tweak (ErrCredentialsNotFound → … error). The Azure SDK ACR change from the description is gone — likely dropped on rebase.

I assume that was accidental — are you still working on this?

dbalseiro · 2026-08-28T19:39:57Z

@lkingland hey! yes this is still on my radar :) I can take care of this over the weekend

dbalseiro · 2026-09-07T19:25:21Z

/retest

dbalseiro · 2026-09-07T22:06:17Z

@gauron99 you were right I had force-pushed a bad rebase :( Now everything is how it's supposed to be

We are using a version of this in our project and I'd greatly appreciate if it can go upstream. Thanks for your time!

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

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

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 lgtm 🤖 PR is ready to be merged. approved 🤖 PR has been approved by an approver from all required OWNERS files. labels Feb 17, 2026

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

dbalseiro requested a review from lkingland March 18, 2026 21:47

knative-prow-robot added the needs-rebase Cannot be merged due to conflicts with HEAD. label May 16, 2026

dbalseiro force-pushed the acr-login-with-sdk branch 2 times, most recently from f8eee8f to 36255ef Compare May 19, 2026 18:23

knative-prow-robot removed the needs-rebase Cannot be merged due to conflicts with HEAD. label May 19, 2026

knative-prow-robot added the needs-rebase Cannot be merged due to conflicts with HEAD. label Jun 11, 2026

dbalseiro force-pushed the acr-login-with-sdk branch from 36255ef to ff6fe08 Compare June 15, 2026 19:26

dbalseiro closed this Jun 15, 2026

dbalseiro force-pushed the acr-login-with-sdk branch from ff6fe08 to 9249593 Compare June 15, 2026 19:45

knative-prow Bot added size/XS 🤖 PR changes 0-9 lines, ignoring generated files. and removed approved 🤖 PR has been approved by an approver from all required OWNERS files. size/L 🤖 PR changes 100-499 lines, ignoring generated files. labels Jun 15, 2026

dbalseiro reopened this Jun 15, 2026

knative-prow Bot added the approved 🤖 PR has been approved by an approver from all required OWNERS files. label Jun 15, 2026

knative-prow-robot removed the needs-rebase Cannot be merged due to conflicts with HEAD. label Jun 15, 2026

dbalseiro force-pushed the acr-login-with-sdk branch from d1b384c to 3ea4af3 Compare September 7, 2026 19:17

knative-prow Bot added size/L 🤖 PR changes 100-499 lines, ignoring generated files. and removed size/XS 🤖 PR changes 0-9 lines, ignoring generated files. labels Sep 7, 2026

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

721088b

dbalseiro force-pushed the acr-login-with-sdk branch from 3ea4af3 to 721088b Compare September 7, 2026 19:23

Uh oh!

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 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!

lkingland commented Apr 21, 2026

Uh oh!

dbalseiro commented May 19, 2026

Uh oh!

dbalseiro commented May 19, 2026

Uh oh!

dbalseiro commented Jun 8, 2026

Uh oh!

knative-prow Bot commented Jun 15, 2026

Uh oh!

knative-prow Bot commented Jun 15, 2026

Uh oh!

dbalseiro commented Jun 15, 2026

Uh oh!

gauron99 commented Aug 21, 2026

Uh oh!

gauron99 commented Aug 21, 2026

Uh oh!

dbalseiro commented Aug 28, 2026

Uh oh!

dbalseiro commented Sep 7, 2026

Uh oh!

dbalseiro commented Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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

edited

Loading

codecov Bot commented Feb 17, 2026 •

edited

Loading