From 2b3361cc1ad45c14a413a62b5dbfd3b8a8608357 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 8 Jun 2018 09:21:36 -0700 Subject: [PATCH] Move the successful removal print Signed-off-by: Nassim 'Nass' Eddequiouaq --- cli/command/trust/signer_remove.go | 8 +++++--- cli/command/trust/signer_remove_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cli/command/trust/signer_remove.go b/cli/command/trust/signer_remove.go index f945a8e111..5bd586c536 100644 --- a/cli/command/trust/signer_remove.go +++ b/cli/command/trust/signer_remove.go @@ -43,11 +43,9 @@ func removeSigner(cli command.Cli, options signerRemoveOptions) error { var errRepos []string for _, repo := range options.repos { fmt.Fprintf(cli.Out(), "Removing signer \"%s\" from %s...\n", options.signer, repo) - if didRemove, err := removeSingleSigner(cli, repo, options.signer, options.forceYes); err != nil { + if _, err := removeSingleSigner(cli, repo, options.signer, options.forceYes); err != nil { fmt.Fprintln(cli.Err(), err.Error()+"\n") errRepos = append(errRepos, repo) - } else if didRemove { - fmt.Fprintf(cli.Out(), "Successfully removed %s from %s\n\n", options.signer, repo) } } if len(errRepos) > 0 { @@ -78,6 +76,8 @@ func isLastSignerForReleases(roleWithSig data.Role, allRoles []client.RoleWithSi return counter < releasesRoleWithSigs.Threshold, nil } +// removeSingleSigner returns whether the signer has been removed during this operation and an error +// Note: the signer not being removed doesn't necessarily raise an error (eg. User saying "No" to the confirmation prompt) func removeSingleSigner(cli command.Cli, repoName, signerName string, forceYes bool) (bool, error) { ctx := context.Background() imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, nil, image.AuthResolver(cli), repoName) @@ -136,5 +136,7 @@ func removeSingleSigner(cli command.Cli, repoName, signerName string, forceYes b return false, err } + fmt.Fprintf(cli.Out(), "Successfully removed %s from %s\n\n", signerName, repoName) + return true, nil } diff --git a/cli/command/trust/signer_remove_test.go b/cli/command/trust/signer_remove_test.go index 27a622bd5b..ac81c1a676 100644 --- a/cli/command/trust/signer_remove_test.go +++ b/cli/command/trust/signer_remove_test.go @@ -71,13 +71,13 @@ func TestTrustSignerRemoveErrors(t *testing.T) { func TestRemoveSingleSigner(t *testing.T) { cli := test.NewFakeCli(&fakeClient{}) cli.SetNotaryClient(notaryfake.GetLoadedNotaryRepository) - didRemove, err := removeSingleSigner(cli, "signed-repo", "test", true) + removed, err := removeSingleSigner(cli, "signed-repo", "test", true) assert.Error(t, err, "No signer test for repository signed-repo") - assert.Equal(t, didRemove, false, "No signer should be removed") + assert.Equal(t, removed, false, "No signer should be removed") - _, err = removeSingleSigner(cli, "signed-repo", "releases", true) + removed, err = removeSingleSigner(cli, "signed-repo", "releases", true) assert.Error(t, err, "releases is a reserved keyword and cannot be removed") - assert.Equal(t, didRemove, false, "No signer should be removed") + assert.Equal(t, removed, false, "No signer should be removed") } func TestRemoveMultipleSigners(t *testing.T) {