diff --git a/cmd/acr/manifest.go b/cmd/acr/manifest.go index 05480327..5a3d2813 100644 --- a/cmd/acr/manifest.go +++ b/cmd/acr/manifest.go @@ -65,12 +65,18 @@ func newManifestListCmd(manifestParams *manifestParameters) *cobra.Command { return err } loginURL := api.LoginURL(registryName) + ctx := context.Background() // An acrClient is created to make the http requests to the registry. acrClient, err := api.GetAcrCLIClientWithAuth(loginURL, manifestParams.username, manifestParams.password, manifestParams.configs) if err != nil { return err } - ctx := context.Background() + // For ABAC registries, scope the token to the target repository. + if acrClient.IsAbac() { + if err := acrClient.RefreshTokenForAbac(ctx, []string{manifestParams.repoName}); err != nil { + return err + } + } err = listManifests(ctx, acrClient, loginURL, manifestParams.repoName) if err != nil { return err @@ -122,11 +128,17 @@ func newManifestDeleteCmd(manifestParams *manifestParameters) *cobra.Command { return err } loginURL := api.LoginURL(registryName) + ctx := context.Background() acrClient, err := api.GetAcrCLIClientWithAuth(loginURL, manifestParams.username, manifestParams.password, manifestParams.configs) if err != nil { return err } - ctx := context.Background() + // For ABAC registries, scope the token to the target repository. + if acrClient.IsAbac() { + if err := acrClient.RefreshTokenForAbac(ctx, []string{manifestParams.repoName}); err != nil { + return err + } + } err = deleteManifests(ctx, acrClient, loginURL, manifestParams.repoName, args) if err != nil { return err diff --git a/cmd/acr/manifest_test.go b/cmd/acr/manifest_test.go index a21a8369..b63e06aa 100644 --- a/cmd/acr/manifest_test.go +++ b/cmd/acr/manifest_test.go @@ -9,6 +9,7 @@ import ( "github.com/Azure/acr-cli/cmd/mocks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestListManifests(t *testing.T) { @@ -67,3 +68,110 @@ func TestDeleteManifests(t *testing.T) { mockClient.AssertExpectations(t) }) } + +// TestListManifestsAbac tests the manifest list ABAC code path: after client creation, +// if IsAbac() returns true, RefreshTokenForAbac must be called before listing manifests. +func TestListManifestsAbac(t *testing.T) { + t.Run("AbacEnabledListManifestsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(nil).Once() + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(singleManifestV2WithTagsResult, nil).Once() + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:2830cc0fcddc1bc2bd4aeab0ed5ee7087dab29a49e65151c77553e46a7ed5283").Return(EmptyListManifestsResult, nil).Once() + // Simulate the ABAC code path from the manifest list command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + err := listManifests(testCtx, mockClient, testLoginURL, testRepo) + assert.Equal(nil, err, "Error should be nil") + mockClient.AssertExpectations(t) + }) + + t.Run("AbacRefreshFailureListManifestsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(errors.New("failed to refresh token for ABAC repositories")).Once() + // Simulate the ABAC code path from the manifest list command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.NotEqual(nil, err, "RefreshTokenForAbac should return an error") + } + // GetAcrManifests should NOT be called since ABAC refresh failed + mockClient.AssertExpectations(t) + }) + + t.Run("NonAbacListManifestsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(false) + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "").Return(singleManifestV2WithTagsResult, nil).Once() + mockClient.On("GetAcrManifests", mock.Anything, testRepo, "", "sha256:2830cc0fcddc1bc2bd4aeab0ed5ee7087dab29a49e65151c77553e46a7ed5283").Return(EmptyListManifestsResult, nil).Once() + // Simulate the ABAC code path from the manifest list command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + err := listManifests(testCtx, mockClient, testLoginURL, testRepo) + assert.Equal(nil, err, "Error should be nil") + // RefreshTokenForAbac should NOT have been called + mockClient.AssertNotCalled(t, "RefreshTokenForAbac", mock.Anything, mock.Anything) + mockClient.AssertExpectations(t) + }) +} + +// TestDeleteManifestsAbac tests the manifest delete ABAC code path: after client creation, +// if IsAbac() returns true, RefreshTokenForAbac must be called before deleting manifests. +func TestDeleteManifestsAbac(t *testing.T) { + args := []string{"sha:123", "sha:124"} + + t.Run("AbacEnabledDeleteManifestsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(nil).Once() + mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha:123").Return(&deletedResponse, nil).Once() + mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha:124").Return(&deletedResponse, nil).Once() + // Simulate the ABAC code path from the manifest delete command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + err := deleteManifests(testCtx, mockClient, testLoginURL, testRepo, args) + assert.Equal(nil, err, "Error should be nil") + mockClient.AssertExpectations(t) + }) + + t.Run("AbacRefreshFailureDeleteManifestsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(errors.New("failed to refresh token for ABAC repositories")).Once() + // Simulate the ABAC code path from the manifest delete command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.NotEqual(nil, err, "RefreshTokenForAbac should return an error") + } + // DeleteManifest should NOT be called since ABAC refresh failed + mockClient.AssertExpectations(t) + }) + + t.Run("NonAbacDeleteManifestsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(false) + mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha:123").Return(&deletedResponse, nil).Once() + mockClient.On("DeleteManifest", mock.Anything, testRepo, "sha:124").Return(&deletedResponse, nil).Once() + // Simulate the ABAC code path from the manifest delete command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + err := deleteManifests(testCtx, mockClient, testLoginURL, testRepo, args) + assert.Equal(nil, err, "Error should be nil") + mockClient.AssertNotCalled(t, "RefreshTokenForAbac", mock.Anything, mock.Anything) + mockClient.AssertExpectations(t) + }) +} diff --git a/cmd/acr/tag.go b/cmd/acr/tag.go index ba31fda3..93e6b6a5 100644 --- a/cmd/acr/tag.go +++ b/cmd/acr/tag.go @@ -65,12 +65,18 @@ func newTagListCmd(tagParams *tagParameters) *cobra.Command { return err } loginURL := api.LoginURL(registryName) + ctx := context.Background() // An acrClient is created to make the http requests to the registry. acrClient, err := api.GetAcrCLIClientWithAuth(loginURL, tagParams.username, tagParams.password, tagParams.configs) if err != nil { return err } - ctx := context.Background() + // For ABAC registries, scope the token to the target repository. + if acrClient.IsAbac() { + if err := acrClient.RefreshTokenForAbac(ctx, []string{tagParams.repoName}); err != nil { + return err + } + } tagList, err := tag.ListTags(ctx, acrClient, tagParams.repoName) if err != nil { return err @@ -99,11 +105,17 @@ func newTagDeleteCmd(tagParams *tagParameters) *cobra.Command { return err } loginURL := api.LoginURL(registryName) + ctx := context.Background() acrClient, err := api.GetAcrCLIClientWithAuth(loginURL, tagParams.username, tagParams.password, tagParams.configs) if err != nil { return err } - ctx := context.Background() + // For ABAC registries, scope the token to the target repository. + if acrClient.IsAbac() { + if err := acrClient.RefreshTokenForAbac(ctx, []string{tagParams.repoName}); err != nil { + return err + } + } err = tag.DeleteTags(ctx, acrClient, loginURL, tagParams.repoName, args) if err != nil { return err diff --git a/cmd/acr/tag_test.go b/cmd/acr/tag_test.go index dea1874e..7de04247 100644 --- a/cmd/acr/tag_test.go +++ b/cmd/acr/tag_test.go @@ -4,9 +4,13 @@ package main import ( + "errors" "testing" + "github.com/Azure/acr-cli/cmd/mocks" + "github.com/Azure/acr-cli/internal/tag" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestNewTagCmd(t *testing.T) { @@ -34,3 +38,112 @@ func TestNewTagDeleteCmd(t *testing.T) { assert.Equal(t, "delete", cmd.Use) assert.Equal(t, newTagDeleteCmdLongMessage, cmd.Long) } + +// TestListTagsAbac tests the tag list ABAC code path: after client creation, +// if IsAbac() returns true, RefreshTokenForAbac must be called before listing tags. +func TestListTagsAbac(t *testing.T) { + t.Run("AbacEnabledListTagsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(nil).Once() + mockClient.On("GetAcrTags", mock.Anything, testRepo, "", "").Return(OneTagResult, nil).Once() + mockClient.On("GetAcrTags", mock.Anything, testRepo, "", "latest").Return(EmptyListTagsResult, nil).Once() + // Simulate the ABAC code path from the tag list command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + tagList, err := tag.ListTags(testCtx, mockClient, testRepo) + assert.Equal(nil, err, "Error should be nil") + assert.Equal(1, len(tagList), "Tag list should have 1 tag") + mockClient.AssertExpectations(t) + }) + + t.Run("AbacRefreshFailureListTagsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(errors.New("failed to refresh token for ABAC repositories")).Once() + // Simulate the ABAC code path from the tag list command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.NotEqual(nil, err, "RefreshTokenForAbac should return an error") + } + // GetAcrTags should NOT be called since ABAC refresh failed + mockClient.AssertExpectations(t) + }) + + t.Run("NonAbacListTagsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(false) + mockClient.On("GetAcrTags", mock.Anything, testRepo, "", "").Return(OneTagResult, nil).Once() + mockClient.On("GetAcrTags", mock.Anything, testRepo, "", "latest").Return(EmptyListTagsResult, nil).Once() + // Simulate the ABAC code path from the tag list command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + tagList, err := tag.ListTags(testCtx, mockClient, testRepo) + assert.Equal(nil, err, "Error should be nil") + assert.Equal(1, len(tagList), "Tag list should have 1 tag") + // RefreshTokenForAbac should NOT have been called + mockClient.AssertNotCalled(t, "RefreshTokenForAbac", mock.Anything, mock.Anything) + mockClient.AssertExpectations(t) + }) +} + +// TestDeleteTagsAbac tests the tag delete ABAC code path: after client creation, +// if IsAbac() returns true, RefreshTokenForAbac must be called before deleting tags. +func TestDeleteTagsAbac(t *testing.T) { + args := []string{"latest", "v1"} + + t.Run("AbacEnabledDeleteTagsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(nil).Once() + mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "latest").Return(&deletedResponse, nil).Once() + mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1").Return(&deletedResponse, nil).Once() + // Simulate the ABAC code path from the tag delete command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + err := tag.DeleteTags(testCtx, mockClient, testLoginURL, testRepo, args) + assert.Equal(nil, err, "Error should be nil") + mockClient.AssertExpectations(t) + }) + + t.Run("AbacRefreshFailureDeleteTagsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(true) + mockClient.On("RefreshTokenForAbac", mock.Anything, []string{testRepo}).Return(errors.New("failed to refresh token for ABAC repositories")).Once() + // Simulate the ABAC code path from the tag delete command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.NotEqual(nil, err, "RefreshTokenForAbac should return an error") + } + // DeleteAcrTag should NOT be called since ABAC refresh failed + mockClient.AssertExpectations(t) + }) + + t.Run("NonAbacDeleteTagsTest", func(t *testing.T) { + assert := assert.New(t) + mockClient := &mocks.AcrCLIClientInterface{} + mockClient.On("IsAbac").Return(false) + mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "latest").Return(&deletedResponse, nil).Once() + mockClient.On("DeleteAcrTag", mock.Anything, testRepo, "v1").Return(&deletedResponse, nil).Once() + // Simulate the ABAC code path from the tag delete command + if mockClient.IsAbac() { + err := mockClient.RefreshTokenForAbac(testCtx, []string{testRepo}) + assert.Equal(nil, err, "RefreshTokenForAbac should not return an error") + } + err := tag.DeleteTags(testCtx, mockClient, testLoginURL, testRepo, args) + assert.Equal(nil, err, "Error should be nil") + mockClient.AssertNotCalled(t, "RefreshTokenForAbac", mock.Anything, mock.Anything) + mockClient.AssertExpectations(t) + }) +} diff --git a/internal/tag/tag.go b/internal/tag/tag.go index c1876d7b..1325372c 100644 --- a/internal/tag/tag.go +++ b/internal/tag/tag.go @@ -32,7 +32,9 @@ func ListTags(ctx context.Context, acrClient api.AcrCLIClientInterface, repoName } var tagList []acr.TagAttributesBase - tagList = append(tagList, *resultTags.TagsAttributes...) + if resultTags.TagsAttributes != nil { + tagList = append(tagList, *resultTags.TagsAttributes...) + } // A for loop is used because the GetAcrTags method returns by default only 100 tags and their attributes. for resultTags != nil && resultTags.TagsAttributes != nil {