T keithyao/batch purge Adds ABAC support for purge command with batched token requests by keithy1012 · Pull Request #577 · Azure/acr-cli

keithy1012 · 2026-02-05T21:56:01Z

Purpose of the PR

Implements batch processing logic for the purge command.
ABAC-enabled registries grant permissions at the repository level, not registry-wide. The current auth flow requests a wildcard scope, which fails for ABAC registries because they cannot grant blanket access to all repositories.
Solution: Detects ABAC registries by checking for aad_identity in the refresh token. Batch token requests for ABAC registries: instead of one wildcard token, request tokens scoped to batches of 10 repositories at a time. Non-ABAC registries continue using the existing wildcard flow.

Fixes #

estebanreyl · 2026-02-06T00:12:46Z

cmd/acr/purge.go

+
+			// Request a token that covers all repositories in this batch
+			// The token scope will include: repository:repo1:pull repository:repo1:delete ... for each repo
+			if err := acrClient.RefreshTokenForAbac(ctx, batch); err != nil {


The token is only refreshed once at the start of the requests. This seems like it could be problematic as I presume these tokens time out. You should have some strategy to rotate this as time goes on.

Implemented

Now the token is refreshed per batch. In practice we want to generally avoid doing the refreshes upfront and just refresh as needed on the token acquisition portion. So for example if you look at the getManifest calls you will see we call refreshAcrCLIClientToken first on those. What you could do is modify that function to refresh the token you have if its necessary dynamically at that stage.

Implemented

cmd/acr/purge.go

internal/api/acrsdk.go

cmd/acr/purge.go

lizMSFT

You could follow https://github.com/Azure/acr-cli/tree/main?tab=readme-ov-file#installation to build the binary or check https://github.com/Azure/acr-cli/blob/main/scripts/experimental/README-purge-testing.md#1-test-purge-allsh-consolidated-test-suite and test the changes locally first.

cmd/acr/purge.go

internal/api/acrsdk.go

cmd/acr/purge.go

estebanreyl

Looking good overall. I would recommend adding some test to the cli experimental tests as well. Just some minor things left to address

estebanreyl · 2026-02-18T21:53:48Z

cmd/acr/purge.go

+
+			// Request a token that covers all repositories in this batch
+			// The token scope will include: repository:repo1:pull repository:repo1:delete ... for each repo
+			if err := acrClient.RefreshTokenForAbac(ctx, batch); err != nil {


Now the token is refreshed per batch. In practice we want to generally avoid doing the refreshes upfront and just refresh as needed on the token acquisition portion. So for example if you look at the getManifest calls you will see we call refreshAcrCLIClientToken first on those. What you could do is modify that function to refresh the token you have if its necessary dynamically at that stage.

cmd/acr/purge.go

estebanreyl · 2026-02-18T21:54:56Z

cmd/acr/purge.go

+		// Process all repositories in this batch
+		for j, repoName := range batch {
+			// For ABAC registries, check if token expired and refresh for remaining repos in batch
+			if acrClient.IsAbac() && acrClient.IsTokenExpired() {


The token can fail at any moment, for example let's say there are a lot of images inside of a particular repository that need to be scanned. We could have the token expire mid iteration and thus that repo listing and exploration would fail. My suggestion would be to give the acquirer the ability to refresh itself, so for example I see that you are not storing the token anwyhere in this part of the code, instead its maintained inside of acrClient,

I'm thinking about updating acrsdk.go to handle single repo token refresh:

// In acrsdk.go - modify refreshAcrCLIClientToken func refreshAcrCLIClientToken(ctx context.Context, c *AcrCLIClient, repoName string) error { var scope string if c.isAbac && repoName != "" { scope = fmt.Sprintf("repository:%s:pull,delete,metadata_read,metadata_write", repoName) } else { scope = "repository:*:*" } // ... refresh logic }

This allows SDK methods like GetAcrTags() to auto-refresh with single repo scope when the token expires mid-operation. But we should still keep the batch-level token refresh logic in purge.go for efficiency. The batch refresh at the start of each batch reduces the number of token requests when processing multiple repositories. And to coordinate between these two, we could add a flag (e.g., NeedsBatchRefresh()) that gets set to true when a single repo refresh occurs. Then in purge.go, after processing each repository, we can check this flag and proactively refresh for the remaining repositories in the batch if needed. What do you think?

Would this approach require 2 token refreshes (one for the current repoName, one for the rest of the batch)? Also, I think this would increase complexity for state management since we require another flag like NeedsBatchRefresh. I think the current implementation is less complex and accomplishes the same task. Does that make sense?

lizMSFT · 2026-02-19T05:19:41Z

internal/api/acrsdk.go

+
 // GetAcrTags list the tags of a repository with their attributes.
 func (c *AcrCLIClient) GetAcrTags(ctx context.Context, repoName string, orderBy string, last string) (*acrapi.RepositoryTagsType, error) {
 	if c.isExpired() {


This block also needs to be updated. Otherwise, when GetAcrTags detects that the token has expired, it will obtain an RBAC access token instead. This also applies to DeleteAcrTag

Additional potential ABAC expiry edge case:

refreshAcrCLIClientToken() seems to request wildcard scope (repository:*:*), and this expiry path seems to be called by SDK methods like GetAcrTags/DeleteAcrTag/GetAcrManifests.

Even with ABAC batch refresh in purge(), if expiry is hit during these SDK operations, it seems like this path could still fall back to wildcard refresh. On ABAC registries, wildcard refresh fallback attempts will likely fail.

Perhaps, you could make the SDK expiry refresh ABAC-aware (repo-scoped) as well? A targeted test in internal/api/acrsdk_test.go for this expiry path will likely help prevent regressions.

Implemented!

acr/acr

acr-cli

cmd/acr/purge.go

johnsonshi · 2026-02-19T08:06:18Z

internal/api/acrsdk.go

+
 // GetAcrTags list the tags of a repository with their attributes.
 func (c *AcrCLIClient) GetAcrTags(ctx context.Context, repoName string, orderBy string, last string) (*acrapi.RepositoryTagsType, error) {
 	if c.isExpired() {


Additional potential ABAC expiry edge case:

refreshAcrCLIClientToken() seems to request wildcard scope (repository:*:*), and this expiry path seems to be called by SDK methods like GetAcrTags/DeleteAcrTag/GetAcrManifests.

Even with ABAC batch refresh in purge(), if expiry is hit during these SDK operations, it seems like this path could still fall back to wildcard refresh. On ABAC registries, wildcard refresh fallback attempts will likely fail.

Perhaps, you could make the SDK expiry refresh ABAC-aware (repo-scoped) as well? A targeted test in internal/api/acrsdk_test.go for this expiry path will likely help prevent regressions.

lizMSFT

nit: it would also be helpful if you can follow https://github.com/Azure/acr-cli/blob/main/scripts/experimental/README-purge-testing.md#performance-testing to do some performance testing and include the result in the PR description

lizMSFT · 2026-02-20T21:11:02Z

.gitignore

 coverage.txt
 *.test
 *.out
+


Could you keep this change in your local env?

Implemented

…ng repo without proper roles

estebanreyl

I validated its working. Left a couple of minor comments but once those are addressed I think we can move forward.

estebanreyl · 2026-03-03T23:23:09Z

README.md

    --include-locked
 ```

+#### ABAC batch size (environment variable)


Can you add some information on using ABAC registries in general. Maybe note what permissions are necessary + what happens if partial access is available

implemented

.gitignore

estebanreyl · 2026-03-03T23:25:20Z

scripts/experimental/test-purge-all.sh

    assert_equals "$initial_manifests" "$final_manifests" "Manifest should remain"
 }

 run_test_comp_age() {


What are you trying to validate with this change? I dont think this script change is necessarily super relevant to the abac scenarios.

lizMSFT · 2026-03-04T23:30:23Z

cmd/acr/purge.go

+
+	// Collect all repository names into a slice for batching
+	repos := make([]string, 0, len(tagFilters))
+	for repoName := range tagFilters {


nit: Go map iteration is non-deterministic, causing unstable batching and test results. Consider sorting repos before batching

lizMSFT · 2026-03-04T23:42:17Z

internal/api/acrsdk.go

+		if repo == "catalog" {
+			scopeParts = append(scopeParts, "registry:catalog:*")
+		} else {
+			scopeParts = append(scopeParts, fmt.Sprintf("repository:%s:pull,delete,metadata_read,metadata_write", repo))


nit: Both RefreshTokenForAbac and refreshAcrCLIClientToken functions build ABAC scopes with near-identical logic (the catalog sentinel handling and the repository:%s:pull,delete,metadata_read,metadata_write format string). This is duplicated and could drift.
Consider extracting a shared helper

estebanreyl

LGTM. Will wait for the license agreement check and merge once thats done :)

keithy1012 · 2026-03-07T03:15:54Z

@keithy1012 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

Keith Yao and others added 4 commits February 5, 2026 06:03

Implemented Per-Repository Token Refresh method

02e4cf1

Added new testing

69c5fbb

Implemented option 2 batch processing for purge

e666217

Merge branch 'main' into t-keithyao/batch-purge

d6779aa

estebanreyl reviewed Feb 6, 2026

View reviewed changes

Refactored code

6b6b106

wangxiaoxuan273 reviewed Feb 6, 2026

View reviewed changes

cmd/acr/purge.go Outdated Show resolved Hide resolved

wangxiaoxuan273 reviewed Feb 6, 2026

View reviewed changes

cmd/acr/purge.go Outdated Show resolved Hide resolved

lizMSFT reviewed Feb 6, 2026

View reviewed changes

cmd/acr/purge.go Outdated Show resolved Hide resolved

Keith Yao added 2 commits February 6, 2026 18:30

Added token refresh check

7db0aea

Merge origin/main and fix conflicts

eb5ea3a

lizMSFT reviewed Feb 6, 2026

View reviewed changes

cmd/acr/purge.go Outdated Show resolved Hide resolved

cmd/acr/purge.go Outdated Show resolved Hide resolved

lizMSFT reviewed Feb 6, 2026

View reviewed changes

internal/api/acrsdk.go Outdated Show resolved Hide resolved

lizMSFT reviewed Feb 6, 2026

View reviewed changes

internal/api/acrsdk.go Outdated Show resolved Hide resolved

northtyphoon reviewed Feb 9, 2026

View reviewed changes

internal/api/acrsdk.go Show resolved Hide resolved

cmd/acr/purge.go Outdated Show resolved Hide resolved

cmd/acr/purge.go Outdated Show resolved Hide resolved

Keith Yao added 4 commits February 9, 2026 22:37

Made config for abacBatchSize

b857681

Check abac from token before constructing acrcliclient

8ad2ac8

Changed scope string

f9f0762

env var

6aa0c97

estebanreyl reviewed Feb 18, 2026

View reviewed changes

Merge branch 'Azure:main' into t-keithyao/batch-purge

b57476b

lizMSFT reviewed Feb 19, 2026

View reviewed changes

johnsonshi reviewed Feb 19, 2026

View reviewed changes

Keith Yao added 3 commits February 19, 2026 18:28

Removed binarie

73922fb

Added verbose flag and testing

84703be

Added test for refreshToken

00bb47b

lizMSFT reviewed Feb 20, 2026

View reviewed changes

Edited gitignore

c581d0d

FeynmanZhou linked an issue Feb 23, 2026 that may be closed by this pull request

acr purge command: support ABAC-enabled registries if identity has full data plane permissions #501

Open

Performance testing

1bea364

Keith Yao added 3 commits March 2, 2026 21:35

Fixed bugs

1866382

Added clearer logging for partial repo permissions, stops after hitti…

7b607d6

…ng repo without proper roles

Fixed logging twice bug

57e9acf

balcsida mentioned this pull request Mar 3, 2026

Fix: Support ABAC-enabled registries #504

Closed

6 tasks

keithy1012 marked this pull request as ready for review March 3, 2026 21:05

keithy1012 requested review from gildardogmsft and sajayantony as code owners March 3, 2026 21:05

estebanreyl reviewed Mar 3, 2026

View reviewed changes

Keith Yao added 2 commits March 4, 2026 00:20

Updated readme and gitignore

961bdda

edited test file

2df3959

lizMSFT reviewed Mar 4, 2026

View reviewed changes

Keith Yao added 3 commits March 4, 2026 23:53

Removed setCurrentRepositories dead code

ec6d621

Sort repos lexicographically

d95e5ce

Abstract functionality into buildAbacScope()

9521565

keithy1012 force-pushed the t-keithyao/batch-purge branch from 9521565 to 2df3959 Compare March 5, 2026 00:21

Added test for buildAbacScope

72f3c5f

estebanreyl approved these changes Mar 7, 2026

View reviewed changes

Conversation

keithy1012 commented Feb 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lizMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

estebanreyl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lizMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebanreyl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebanreyl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keithy1012 commented Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

estebanreyl left a comment •

edited

Loading