diff --git a/README.md b/README.md index 4d302858..0649a7d3 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,17 @@ acr purge \ --ago 30d ``` +You can pair `--ago` with the `--untagged` flag to apply the same age threshold to manifest cleanup, ensuring that only manifests +older than the specified cutoff are removed: + +```sh +acr purge \ + --registry \ + --filter : \ + --ago 7d \ + --untagged +``` + The following table further explains the functionality of this flag. | Intention | Flag | @@ -152,7 +163,7 @@ The duration should be of the form \[integer\]d\[string\] where the first intege #### Untagged flag -To delete all the manifests that do not have any tags linked to them, the `--untagged` flag should be set. +To delete all the manifests that do not have any tags linked to them, the `--untagged` flag should be set. The manifest cleanup respects the same `--ago` cutoff that is used for tag deletions, so recently-untagged images that are newer than the configured age threshold are preserved. ```sh acr purge \ diff --git a/cmd/acr/annotate.go b/cmd/acr/annotate.go index b804f260..d934e74a 100644 --- a/cmd/acr/annotate.go +++ b/cmd/acr/annotate.go @@ -307,7 +307,7 @@ func annotateUntaggedManifests(ctx context.Context, // Contrary to getTagsToAnnotate, getManifests 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 annotated. - manifestsToAnnotate, err := repository.GetUntaggedManifests(ctx, poolSize, acrClient, repoName, true, nil, dryRun, includeLocked) + manifestsToAnnotate, err := repository.GetUntaggedManifests(ctx, poolSize, acrClient, repoName, true, nil, dryRun, includeLocked, nil) if err != nil { return -1, err } diff --git a/cmd/acr/purge.go b/cmd/acr/purge.go index a7b23a77..f94f98fb 100644 --- a/cmd/acr/purge.go +++ b/cmd/acr/purge.go @@ -179,7 +179,7 @@ func purge(ctx context.Context, singleDeletedManifestsCount := 0 // If the untagged flag is set then also manifests are deleted. if removeUtaggedManifests { - singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, manifestToTagsCountMap, dryRun, includeLocked) + singleDeletedManifestsCount, err = purgeDanglingManifests(ctx, acrClient, repoParallelism, loginURL, repoName, tagDeletionSince, manifestToTagsCountMap, dryRun, includeLocked) if err != nil { return deletedTagsCount, deletedManifestsCount, fmt.Errorf("failed to purge manifests: %w", err) } @@ -360,16 +360,21 @@ 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, manifestToTagsCountMap map[string]int, dryRun bool, includeLocked bool) (int, error) { +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) { 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, // then it should not be deleted. - manifestsToDelete, err := repository.GetUntaggedManifests(ctx, repoParallelism, acrClient, repoName, false, manifestToTagsCountMap, dryRun, includeLocked) + manifestsToDelete, err := repository.GetUntaggedManifests(ctx, repoParallelism, acrClient, repoName, false, manifestToTagsCountMap, dryRun, includeLocked, &timeToCompare) if err != nil { return -1, err } diff --git a/cmd/acr/purge_test.go b/cmd/acr/purge_test.go index 9ebc6763..8d26eeba 100644 --- a/cmd/acr/purge_test.go +++ b/cmd/acr/purge_test.go @@ -18,6 +18,8 @@ import ( "github.com/stretchr/testify/mock" ) +const defaultAgo = "0m" + // 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) { @@ -282,7 +284,7 @@ func TestPurgeManifests(t *testing.T) { assert := assert.New(t) mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(notFoundManifestResponse, errors.New("testRepo not found")).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(0, deletedTags, "Number of deleted elements should be 0") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -293,7 +295,7 @@ func TestPurgeManifests(t *testing.T) { assert := assert.New(t) mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(nil, errors.New("unauthorized")).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(-1, deletedTags, "Number of deleted elements should be -1") assert.NotEqual(nil, err, "Error should not be nil") mockClient.AssertExpectations(t) @@ -306,19 +308,66 @@ func TestPurgeManifests(t *testing.T) { mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(singleManifestV2WithTagsResult, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:2830cc0fcddc1bc2bd4aeab0ed5ee7087dab29a49e65151c77553e46a7ed5283").Return(EmptyListManifestsResult, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(0, deletedTags, "Number of deleted elements should be 0") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) }) + t.Run("SkipsManifestsNewerThanAgo", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + manifestList := &acr.Manifests{ + Registry: &testLoginURL, + ImageName: &testRepo, + ManifestsAttributes: &[]acr.ManifestAttributesBase{{ + LastUpdateTime: &lastUpdateTime, + ChangeableAttributes: &acr.ChangeableAttributes{DeleteEnabled: &deleteEnabled, WriteEnabled: &writeEnabled}, + Digest: &digest1, + MediaType: &dockerV2MediaType, + Tags: nil, + }}, + } + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(manifestList, nil).Once() + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", digest1).Return(EmptyListManifestsResult, nil).Once() + + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, "1h", nil, false, false) + assert.Equal(0, deletedTags, "Number of deleted elements should be 0") + assert.NoError(err) + mockClient.AssertExpectations(t) + }) + + t.Run("DeletesManifestsOlderThanAgo", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + manifestList := &acr.Manifests{ + Registry: &testLoginURL, + ImageName: &testRepo, + ManifestsAttributes: &[]acr.ManifestAttributesBase{{ + LastUpdateTime: &lastUpdateTime2DaysAgo, + ChangeableAttributes: &acr.ChangeableAttributes{DeleteEnabled: &deleteEnabled, WriteEnabled: &writeEnabled}, + Digest: &digest2, + MediaType: &dockerV2MediaType, + Tags: nil, + }}, + } + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(manifestList, nil).Once() + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", digest2).Return(EmptyListManifestsResult, nil).Once() + mockClient.On("DeleteManifest", mock.Anything, testRepo, digest2).Return(nil, nil).Once() + + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, "24h", nil, false, false) + assert.Equal(1, deletedTags, "Number of deleted elements should be 1") + assert.NoError(err) + mockClient.AssertExpectations(t) + }) + // If there is an error (different to a 404 error) getting the second set of manifests an error should be returned. t.Run("GetAcrManifestsErrorTest", func(t *testing.T) { assert := assert.New(t) mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(singleManifestV2WithTagsResult, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:2830cc0fcddc1bc2bd4aeab0ed5ee7087dab29a49e65151c77553e46a7ed5283").Return(nil, errors.New("error getting manifests")).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(-1, deletedTags, "Number of deleted elements should be -1") assert.NotEqual(nil, err, "Error should not be nil") mockClient.AssertExpectations(t) @@ -333,7 +382,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetManifest", mock.Anything, testRepo, "sha256:d88fb54ba4424dada7c928c6af332ed1c49065ad85eafefb6f26664695015119").Return(nil, errors.New("error getting manifest")).Once() // Despite the failure, the GetAcrManifests method may be called again before the failure happens mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:d88fb54ba4424dada7c928c6af332ed1c49065ad85eafefb6f26664695015119").Return(nil, nil).Maybe() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(-1, deletedTags, "Number of deleted elements should be -1") assert.NotEqual(nil, err, "Error not should be nil") mockClient.AssertExpectations(t) @@ -347,7 +396,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetManifest", mock.Anything, testRepo, "sha256:d88fb54ba4424dada7c928c6af332ed1c49065ad85eafefb6f26664695015119").Return([]byte("invalid manifest"), nil).Once() // Despite the failure, the GetAcrManifests method may be called again before the failure happens mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:d88fb54ba4424dada7c928c6af332ed1c49065ad85eafefb6f26664695015119").Return(nil, nil).Maybe() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(-1, deletedTags, "Number of deleted elements should be -1") assert.NotEqual(nil, err, "Error not should be nil") mockClient.AssertExpectations(t) @@ -364,7 +413,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(EmptyListManifestsResult, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:63532043b5af6247377a472ad075a42bde35689918de1cf7f807714997e0e683").Return(nil, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(nil, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(2, deletedTags, "Number of deleted elements should be 2") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -380,7 +429,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(EmptyListManifestsResult, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:63532043b5af6247377a472ad075a42bde35689918de1cf7f807714997e0e683").Return(nil, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(¬FoundResponse, errors.New("manifest not found")).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(2, deletedTags, "Number of deleted elements should be 2") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -395,7 +444,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(EmptyListManifestsResult, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:63532043b5af6247377a472ad075a42bde35689918de1cf7f807714997e0e683").Return(nil, errors.New("error deleting manifest")).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(nil, nil).Maybe() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(-1, deletedTags, "Number of deleted elements should be -1") assert.NotEqual(nil, err, "Error should not be nil") mockClient.AssertExpectations(t) @@ -411,7 +460,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(EmptyListManifestsResult, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:63532043b5af6247377a472ad075a42bde35689918de1cf7f807714997e0e683").Return(nil, nil).Maybe() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(nil, errors.New("error deleting manifest")).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(-1, deletedTags, "Number of deleted elements should be -1") assert.NotEqual(nil, err, "Error should not be nil") mockClient.AssertExpectations(t) @@ -428,7 +477,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:d88fb54ba4424dada7c928c6af332ed1c49065ad85eafefb6f26664695015119").Return(doubleManifestV2WithoutTagsResult, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(EmptyListManifestsResult, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(nil, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(1, deletedTags, "Number of deleted elements should be 1") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -446,7 +495,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:d88fb54ba4424dada7c928c6af332ed1c49065ad85eafefb6f26664695015119").Return(doubleOCIWithoutTagsResult, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(EmptyListManifestsResult, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha256:6305e31b9b0081d2532397a1e08823f843f329a7af2ac98cb1d7f0355a3e3696").Return(nil, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(1, deletedTags, "Number of deleted elements should be 1") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -459,7 +508,7 @@ func TestPurgeManifests(t *testing.T) { mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(deleteDisabledOneManifestResult, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", digest).Return(EmptyListManifestsResult, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(0, deletedTags, "Number of deleted elements should be 0") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -472,7 +521,7 @@ func TestPurgeManifests(t *testing.T) { mockClient := &mocks.AcrCLIClientInterface{} mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(writeDisabledOneManifestResult, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", digest).Return(EmptyListManifestsResult, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(0, deletedTags, "Number of deleted elements should be 0") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -485,7 +534,7 @@ func TestPurgeManifests(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(singleManifestWithSubjectWithoutTagResult, nil).Once() mockClient.On("GetManifest", mock.Anything, testRepo, "sha256:118811b833e6ca4f3c65559654ca6359410730e97c719f5090d0bfe4db0ab588").Return(manifestWithSubjectOCIArtificate, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:118811b833e6ca4f3c65559654ca6359410730e97c719f5090d0bfe4db0ab588").Return(EmptyListManifestsResult, nil).Once() - deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, false) + deletedTags, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, false) assert.Equal(0, deletedTags, "Number of deleted elements should be 0") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -1427,7 +1476,7 @@ func TestIncludeLockedFlag(t *testing.T) { return attrs.DeleteEnabled != nil && *attrs.DeleteEnabled && attrs.WriteEnabled != nil && *attrs.WriteEnabled })).Return(&deletedResponse, nil).Once() mockClient.On("DeleteManifest", mock.Anything, testRepo, digest).Return(&deletedResponse, nil).Once() - deletedManifests, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, false, true) + deletedManifests, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, false, true) assert.Equal(1, deletedManifests, "Number of deleted manifests should be 1") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) @@ -1492,7 +1541,7 @@ func TestDryRunWithIncludeLocked(t *testing.T) { mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(deleteDisabledDanglingManifest, nil).Once() mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", digest).Return(EmptyListManifestsResult, nil).Once() // No unlock or delete calls should be made in dry-run mode - deletedManifests, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, nil, true, true) + deletedManifests, err := purgeDanglingManifests(testCtx, mockClient, defaultPoolSize, testLoginURL, testRepo, defaultAgo, nil, true, true) assert.Equal(1, deletedManifests, "Number of manifests to be deleted should be 1") assert.Equal(nil, err, "Error should be nil") mockClient.AssertExpectations(t) diff --git a/cmd/repository/image_functions.go b/cmd/repository/image_functions.go index b0ee9f5a..a3259fa4 100644 --- a/cmd/repository/image_functions.go +++ b/cmd/repository/image_functions.go @@ -158,7 +158,7 @@ func GetLastTagFromResponse(resultTags *acr.RepositoryTagsType) string { // the manifest should also not have a tag and not have a subject manifest. // Param manifestToTagsCountMap is an optional map that can be used to pass the count of tags for each manifest that we know would be deleted if the command is exectued // under dryRun conditions. Its ignored if the dryRun flag is false. -func GetUntaggedManifests(ctx context.Context, poolSize int, acrClient api.AcrCLIClientInterface, repoName string, preserveAllOCIManifests bool, manifestToDeletedTagsCountMap map[string]int, dryRun bool, includeLocked bool) ([]acr.ManifestAttributesBase, error) { +func GetUntaggedManifests(ctx context.Context, poolSize int, acrClient api.AcrCLIClientInterface, repoName string, preserveAllOCIManifests bool, manifestToDeletedTagsCountMap map[string]int, dryRun bool, includeLocked bool, deleteCutoff *time.Time) ([]acr.ManifestAttributesBase, error) { lastManifestDigest := "" var manifestsToDelete []acr.ManifestAttributesBase resultManifests, err := acrClient.GetAcrManifests(ctx, repoName, "", lastManifestDigest) @@ -255,6 +255,22 @@ func GetUntaggedManifests(ctx context.Context, poolSize int, acrClient api.AcrCL } // _____MANIFEST IS UNTAGGED BUT MAY BE PROTECTED_____ + if deleteCutoff != nil { + if manifest.LastUpdateTime == nil { + fmt.Printf("Skipping manifest %s because the last update time is unavailable\n", *manifest.Digest) + continue + } + + lastUpdateTime, err := time.Parse(time.RFC3339Nano, *manifest.LastUpdateTime) + if err != nil { + return nil, err + } + + if lastUpdateTime.After(*deleteCutoff) { + continue + } + } + // TODO: #468 I am a little unclear as to why this was ever an option but respecting it for now. Its not used by the purge scenarios only for // the annotate command. if preserveAllOCIManifests { diff --git a/scripts/experimental/test-purge-all.sh b/scripts/experimental/test-purge-all.sh index 19ba96bf..31b617f7 100755 --- a/scripts/experimental/test-purge-all.sh +++ b/scripts/experimental/test-purge-all.sh @@ -190,6 +190,34 @@ create_test_image() { return 0 } +create_unique_test_image() { + local repo="$1" + local tag="$2" + local content="${3:-$tag}" + local tmpdir + tmpdir=$(mktemp -d) + + cat >"$tmpdir/Dockerfile" < /unique.txt +EOF + + if ! docker build -t "$REGISTRY/$repo:$tag" "$tmpdir" >/dev/null 2>&1; then + echo "Error: Failed to build unique image for $repo:$tag" >&2 + rm -rf "$tmpdir" + return 1 + fi + + rm -rf "$tmpdir" + + if ! docker push "$REGISTRY/$repo:$tag" >/dev/null 2>&1; then + echo "Error: Failed to push image $REGISTRY/$repo:$tag" >&2 + return 1 + fi + + return 0 +} + lock_image() { local repo="$1" local tag="$2" @@ -1497,6 +1525,22 @@ run_test_comp_age() { local output=$("$ACR_CLI" purge --registry "$REGISTRY" --filter "$age_repo:.*" --ago 1d --dry-run 2>&1) assert_contains "$output" "Number of tags to be deleted: 0" "New images should not be deleted with --ago 1d" + + local manifest_repo="${age_repo}-untagged" + create_unique_test_image "$manifest_repo" "keep" + create_unique_test_image "$manifest_repo" "temp" + + "$ACR_CLI" purge --registry "$REGISTRY" --filter "$manifest_repo:temp" --ago 0d >/dev/null 2>&1 + + local manifests_before=$(count_manifests "$manifest_repo") + + "$ACR_CLI" purge --registry "$REGISTRY" --filter "$manifest_repo:.*" --ago 1d --untagged >/dev/null 2>&1 + local manifests_after=$(count_manifests "$manifest_repo") + assert_equals "$manifests_before" "$manifests_after" "Recent untagged manifests should be preserved by --ago" + + "$ACR_CLI" purge --registry "$REGISTRY" --filter "$manifest_repo:.*" --ago 0d --untagged >/dev/null 2>&1 + local manifests_final=$(count_manifests "$manifest_repo") + assert_equals "$((manifests_before - 1))" "$manifests_final" "Untagged manifests meeting --ago cutoff should be deleted" } # Test suite runners