From 0119a615b4b63398f490c5c7dbb988fb09c2a15f Mon Sep 17 00:00:00 2001 From: esrey Date: Fri, 19 Dec 2025 09:20:56 -0800 Subject: [PATCH 1/3] Add validation for purge ago values that overflow --- cmd/acr/purge.go | 38 ++++++++++++++++++++++++++++++++++++-- cmd/acr/purge_test.go | 7 +++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/cmd/acr/purge.go b/cmd/acr/purge.go index fee3337f..8b008032 100644 --- a/cmd/acr/purge.go +++ b/cmd/acr/purge.go @@ -65,6 +65,7 @@ var ( // Default settings for regexp2 const ( defaultRegexpMatchTimeoutSeconds int64 = 60 + maxAgoDurationYears int = 150 // Maximum duration in years for --ago flag to prevent overflow ) // purgeParameters defines the parameters that the purge command uses (including the registry name, username and password). @@ -145,7 +146,7 @@ func newPurgeCmd(rootParams *rootParameters) *cobra.Command { cmd.Flags().BoolVar(&purgeParams.untagged, "untagged", false, "If the untagged flag is set all the manifests that do not have any tags associated to them will be also purged, except if they belong to a manifest list that contains at least one tag") cmd.Flags().BoolVar(&purgeParams.dryRun, "dry-run", false, "If the dry-run flag is set no manifest or tag will be deleted, the output would be the same as if they were deleted") cmd.Flags().BoolVar(&purgeParams.includeLocked, "include-locked", false, "If the include-locked flag is set, locked manifests and tags (where deleteEnabled or writeEnabled is false) will be unlocked before deletion") - cmd.Flags().StringVar(&purgeParams.ago, "ago", "", "The tags and untagged manifests that were last updated before this duration will be deleted, the format is [number]d[string] where the first number represents an amount of days and the string is in a Go duration format (e.g. 2d3h6m selects images older than 2 days, 3 hours and 6 minutes)") + cmd.Flags().StringVar(&purgeParams.ago, "ago", "", "The tags and untagged manifests that were last updated before this duration will be deleted, the format is [number]d[string] where the first number represents an amount of days and the string is in a Go duration format (e.g. 2d3h6m selects images older than 2 days, 3 hours and 6 minutes). Maximum duration is capped at 150 years to prevent overflow") cmd.Flags().IntVar(&purgeParams.keep, "keep", 0, "Number of latest to-be-deleted tags to keep, use this when you want to keep at least x number of latest tags that could be deleted meeting all other filter criteria") cmd.Flags().StringArrayVarP(&purgeParams.filters, "filter", "f", nil, "Specify the repository and a regular expression filter for the tag name, if a tag matches the filter and is older than the duration specified in ago it will be deleted. Note: If backtracking is used in the regexp it's possible for the expression to run into an infinite loop. The default timeout is set to 1 minute for evaluation of any filter expression. Use the '--filter-timeout-seconds' option to set a different value.") cmd.Flags().StringArrayVarP(&purgeParams.configs, "config", "c", nil, "Authentication config paths (e.g. C://Users/docker/config.json)") @@ -274,14 +275,47 @@ func parseDuration(ago string) (time.Duration, error) { return time.Duration(0), err } } + // Cap at maxAgoDurationYears to prevent overflow + const maxDays = maxAgoDurationYears * 365 + originalDays := days + capped := false + if days > maxDays { + days = maxDays + capped = true + fmt.Printf("Warning: ago value exceeds maximum duration of %d years, capping to %d years\n", maxAgoDurationYears, maxAgoDurationYears) + } // The number of days gets converted to hours. duration := time.Duration(days) * 24 * time.Hour if len(durationString) > 0 { agoDuration, err := time.ParseDuration(durationString) if err != nil { - return time.Duration(0), err + // Check if it's an overflow error from time.ParseDuration + if strings.Contains(err.Error(), "invalid duration") || strings.Contains(err.Error(), "overflow") { + // If days were already capped, just use that and ignore the overflow portion + if capped { + return (-1 * duration), nil + } + // Cap at max duration and continue + agoDuration = time.Duration(maxDays) * 24 * time.Hour + fmt.Printf("Warning: ago value exceeds maximum duration of %d years, capping to %d years\n", maxAgoDurationYears, maxAgoDurationYears) + } else { + return time.Duration(0), err + } + } + // Cap the additional duration to prevent overflow when adding + maxDuration := time.Duration(maxDays) * 24 * time.Hour + if agoDuration > maxDuration { + agoDuration = maxDuration + if originalDays <= maxDays && !capped { + // Only print warning if we haven't already printed one for days + fmt.Printf("Warning: ago value exceeds maximum duration of %d years, capping to %d years\n", maxAgoDurationYears, maxAgoDurationYears) + } } + // Make sure the combined duration doesn't exceed max duration = duration + agoDuration + if duration > maxDuration { + duration = maxDuration + } } return (-1 * duration), nil } diff --git a/cmd/acr/purge_test.go b/cmd/acr/purge_test.go index 8d26eeba..8025974e 100644 --- a/cmd/acr/purge_test.go +++ b/cmd/acr/purge_test.go @@ -932,8 +932,15 @@ func TestParseDuration(t *testing.T) { {"1d1h3m", -25*time.Hour - 3*time.Minute, nil}, {"3d", -3 * 24 * time.Hour, nil}, {"", 0, io.EOF}, + {"999999d", -1 * time.Duration(150*365) * 24 * time.Hour, nil}, // Capped at 150 years + {"9999999d", -1 * time.Duration(150*365) * 24 * time.Hour, nil}, // Capped at 150 years + {"999999999h", -1 * time.Duration(150*365) * 24 * time.Hour, nil}, // Capped at 150 years + {"9999999d999999999h", -1 * time.Duration(150*365) * 24 * time.Hour, nil}, // Capped at 150 years + {"0m", 0 * time.Minute, nil}, + {"-1d", 24 * time.Hour, nil}, // Negative durations pretty much just mean anything can be cleaned up {"15p", 0, errors.New("time: unknown unit \"p\" in duration \"15p\"")}, {"15", 0 * time.Minute, errors.New("time: missing unit in duration \"15\"")}, + {"1d1h3m", -25*time.Hour - 3*time.Minute, nil}, } assert := assert.New(t) for _, table := range tables { From 7323d5f8fd7c36b0679c112714a54ffa730d0cc9 Mon Sep 17 00:00:00 2001 From: esrey Date: Fri, 19 Dec 2025 09:36:17 -0800 Subject: [PATCH 2/3] Move ago parsing to only happen once --- cmd/acr/purge.go | 24 ++++----- cmd/acr/purge_test.go | 117 ++++++++++++++++++++++-------------------- 2 files changed, 72 insertions(+), 69 deletions(-) diff --git a/cmd/acr/purge.go b/cmd/acr/purge.go index 8b008032..c34c51ae 100644 --- a/cmd/acr/purge.go +++ b/cmd/acr/purge.go @@ -171,16 +171,22 @@ func purge(ctx context.Context, dryRun bool, includeLocked bool) (deletedTagsCount int, deletedManifestsCount int, err error) { + // Parse the duration once instead of for every repository + agoDuration, err := parseDuration(tagDeletionSince) + if err != nil { + return 0, 0, err + } + // In order to print a summary of the deleted tags/manifests the counters get updated everytime a repo is purged. for repoName, tagRegex := range tagFilters { - singleDeletedTagsCount, manifestToTagsCountMap, err := purgeTags(ctx, acrClient, repoParallelism, loginURL, repoName, tagDeletionSince, tagRegex, tagsToKeep, filterTimeout, dryRun, includeLocked) + singleDeletedTagsCount, manifestToTagsCountMap, err := purgeTags(ctx, acrClient, repoParallelism, loginURL, repoName, agoDuration, tagRegex, tagsToKeep, filterTimeout, dryRun, includeLocked) if err != nil { return deletedTagsCount, deletedManifestsCount, fmt.Errorf("failed to purge tags: %w", err) } singleDeletedManifestsCount := 0 // If the untagged flag is set then also manifests are deleted. if removeUtaggedManifests { - singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, tagDeletionSince, manifestToTagsCountMap, dryRun, includeLocked) + singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, agoDuration, manifestToTagsCountMap, dryRun, includeLocked) if err != nil { return deletedTagsCount, deletedManifestsCount, fmt.Errorf("failed to purge manifests: %w", err) } @@ -194,18 +200,14 @@ func purge(ctx context.Context, } -// purgeTags deletes all tags that are older than the ago value and that match the tagFilter string. -func purgeTags(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, ago string, tagFilter string, keep int, regexpMatchTimeoutSeconds int64, dryRun bool, includeLocked bool) (int, map[string]int, error) { +// purgeTags deletes all tags that are older than the agoDuration value and that match the tagFilter string. +func purgeTags(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, agoDuration time.Duration, tagFilter string, keep int, regexpMatchTimeoutSeconds int64, dryRun bool, includeLocked bool) (int, map[string]int, error) { if dryRun { fmt.Printf("Would delete tags for repository: %s\n", repoName) } else { fmt.Printf("Deleting tags for repository: %s\n", repoName) } manifestToTagsCountMap := make(map[string]int) // This map is used to keep track of how many tags would have been deleted per manifest. - agoDuration, err := parseDuration(ago) - if err != nil { - return -1, manifestToTagsCountMap, err - } timeToCompare := time.Now().UTC() // Since the parseDuration function returns a negative duration, it is added to the current duration in order to be able to easily compare // with the LastUpdatedTime attribute a tag has. @@ -394,16 +396,12 @@ func getTagsToDelete(ctx context.Context, // purgeDanglingManifests deletes all manifests that do not have any tags associated with them. // except the ones that are referenced by a multiarch manifest or that have subject. -func purgeDanglingManifests(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, tagDeletionSince string, manifestToTagsCountMap map[string]int, dryRun bool, includeLocked bool) (int, error) { +func purgeDanglingManifests(ctx context.Context, acrClient api.AcrCLIClientInterface, repoParallelism int, loginURL string, repoName string, agoDuration time.Duration, manifestToTagsCountMap map[string]int, dryRun bool, includeLocked bool) (int, error) { if dryRun { fmt.Printf("Would delete manifests for repository: %s\n", repoName) } else { fmt.Printf("Deleting manifests for repository: %s\n", repoName) } - agoDuration, err := parseDuration(tagDeletionSince) - if err != nil { - return -1, err - } timeToCompare := time.Now().UTC().Add(agoDuration) // Contrary to getTagsToDelete, getManifestsToDelete gets all the Manifests at once, this was done because if there is a manifest that has no // tag but is referenced by a multiarch manifest that has tags then it should not be deleted. Or if a manifest has no tag, but it has subject, diff --git a/cmd/acr/purge_test.go b/cmd/acr/purge_test.go index 8025974e..ce33aa85 100644 --- a/cmd/acr/purge_test.go +++ b/cmd/acr/purge_test.go @@ -20,6 +20,21 @@ import ( const defaultAgo = "0m" +var defaultAgoDuration time.Duration + +func init() { + defaultAgoDuration = mustParseDuration(defaultAgo) +} + +// Helper function to parse ago string for tests +func mustParseDuration(ago string) time.Duration { + d, err := parseDuration(ago) + if err != nil { + panic(err) + } + return d +} + // TestPurgeTags contains all the tests regarding the purgeTags method which is called when the --dry-run flag is // not set. func TestPurgeTags(t *testing.T) { @@ -28,7 +43,7 @@ func TestPurgeTags(t *testing.T) { mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrTags", mock.Anything, testRepo, "timedesc", "").Return(TagWithLocal, nil).Once() mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1-c-local.test").Return(&deletedResponse, nil).Once() - deletedTags, _, err := purgeTags(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, "0m", ".*-?local[.].+", 0, 60, false, false) + deletedTags, _, err := purgeTags(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgoDuration, ".*-?local[.].+", 0, 60, false, false) assert.Equal(1, deletedTags, "Number of deleted elements should be 1") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -41,7 +56,7 @@ func TestPurgeTags(t *testing.T) { mockClient.On("GetAcrTags", mock.Anything, testRepo, "timedesc", "").Return(FourTagsWithRepoFilterMatch, nil).Once() mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1-c").Return(&deletedResponse, nil).Once() mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1-b").Return(&deletedResponse, nil).Once() - deletedTags, _, err := purgeTags(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, "0m", "v1(?!-a)", 0, 60, false, false) + deletedTags, _, err := purgeTags(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgoDuration, "v1(?!-a)", 0, 60, false, false) assert.Equal(2, deletedTags, "Number of deleted elements should be 2") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -53,7 +68,7 @@ func TestPurgeTags(t *testing.T) { mockClient.On("GetAcrTags", mock.Anything, testRepo, "timedesc", "").Return(FourTagsWithRepoFilterMatch, nil).Once() mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1-c").Return(&deletedResponse, nil).Once() mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1-b").Return(&deletedResponse, nil).Once() - deletedTags, _, err := purgeTags(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, "0m", "v1-*[abc]+(? Date: Fri, 19 Dec 2025 09:48:06 -0800 Subject: [PATCH 3/3] update docs --- README.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 684db54e..83803324 100644 --- a/README.md +++ b/README.md @@ -121,14 +121,17 @@ The filter flag is used to specify the repository and a regex filter, if a tag i Examples of filters -| Intention | Flag | -|----------------------------------------------------------------------------------|---------------------------------------| -| Untag all tags that begin with hello in app repository | --filter `"app:^hello.*"` | -| Untag tags that end with world in app repository | --filter `"app:\w*world\b"` | -| Untag tags that include hello-world in their name in app repository | --filter `"app:hello-world"` | -| Untag all tags that are older than the duration in repositories ending in /cache | --filter `".*/cache:.*"` | -| Untag all tags that are older than the duration in app repository | --filter `"app:.*"` | -| Untag all tags that are older than the duration in all repositories | --filter `".*:.*"` | +| Intention | Flag | +|-------------------------------------------------------------------------------------|---------------------------------------| +| Untag all tags that begin with hello in app repository | --filter `"app:^hello.*"` | +| Untag tags that end with world in app repository | --filter `"app:\w*world\b"` | +| Untag tags that include hello-world in their name in app repository | --filter `"app:hello-world"` | +| Untag all tags in repositories ending in /cache | --filter `".*/cache:.*"` | +| Untag all tags in app repository | --filter `"app:.*"` | +| Untag all tags in all repositories | --filter `".*:.*"` | +| Clean only untagged manifests in all repos (with --untagged) | --filter `".*:^$"` | +| Clean only untagged manifests in app repo (with --untagged) | --filter `"app:^$"` | + #### Ago flag @@ -160,7 +163,7 @@ The following table further explains the functionality of this flag. | To delete all images that were last modified before 10 minutes ago | --ago 10m | | To delete all images that were last modified before 1 hour and 15 minutes ago | --ago 1h15m | -The duration should be of the form \[integer\]d\[string\] where the first integer specifies the number of days and the string is in a go style duration (can be omitted) +The duration should be of the form \[integer\]d\[string\] where the first integer specifies the number of days and the string is in a go style duration (can be omitted). The maximimum ago duration is set to 150 years but that will effectively clean nothing up as no images should be that old. ### Optional purge flags