From 079471ebebc898d5038ea8cde02eee815ef7caae Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Wed, 25 Oct 2017 19:13:24 +0200 Subject: [PATCH] update to stderr instead of stdout, update tests Signed-off-by: Riyaz Faizullabhoy --- cli/command/trust/key_generate.go | 5 +++-- cli/command/trust/key_load.go | 11 ++++------- cli/command/trust/signer_add.go | 4 ++-- cli/command/trust/signer_add_test.go | 14 ++++++-------- cli/command/trust/signer_remove.go | 7 ++++--- cli/command/trust/signer_remove_test.go | 8 ++++---- 6 files changed, 23 insertions(+), 26 deletions(-) diff --git a/cli/command/trust/key_generate.go b/cli/command/trust/key_generate.go index 23b3f121b9..913083be8f 100644 --- a/cli/command/trust/key_generate.go +++ b/cli/command/trust/key_generate.go @@ -16,6 +16,7 @@ import ( "github.com/docker/notary/trustmanager" "github.com/docker/notary/tuf/data" tufutils "github.com/docker/notary/tuf/utils" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -85,7 +86,7 @@ func validateAndGenerateKey(streams command.Streams, keyName string, workingDir pubPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore) if err != nil { fmt.Fprintf(streams.Out(), err.Error()) - return fmt.Errorf("Error generating key for: %s", keyName) + return errors.Wrapf(err, "failed to generate key for %s", keyName) } // Output the public key to a file in the CWD or specified dir @@ -124,7 +125,7 @@ func writePubKeyPEMToDir(pubPEM pem.Block, keyName, workingDir string) (string, pubFileName := strings.Join([]string{keyName, "pub"}, ".") pubFilePath := filepath.Join(workingDir, pubFileName) if err := ioutil.WriteFile(pubFilePath, pem.EncodeToMemory(&pubPEM), notary.PrivNoExecPerms); err != nil { - return "", fmt.Errorf("Error writing public key to location: %s", pubFilePath) + return "", errors.Wrapf(err, "failed to write public key to %s", pubFilePath) } return pubFilePath, nil } diff --git a/cli/command/trust/key_load.go b/cli/command/trust/key_load.go index e1a394b3a7..9dc77b9996 100644 --- a/cli/command/trust/key_load.go +++ b/cli/command/trust/key_load.go @@ -13,6 +13,7 @@ import ( "github.com/docker/notary/storage" "github.com/docker/notary/trustmanager" tufutils "github.com/docker/notary/tuf/utils" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -54,10 +55,10 @@ func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions passRet := trust.GetPassphraseRetriever(streams.In(), streams.Out()) keyBytes, err := getPrivKeyBytesFromPath(keyPath) if err != nil { - return fmt.Errorf("error reading key from %s: %s", keyPath, err) + return errors.Wrapf(err, "error reading key from %s", keyPath) } if err := loadPrivKeyBytesToStore(keyBytes, privKeyImporters, keyPath, options.keyName, passRet); err != nil { - return fmt.Errorf("error importing key from %s: %s", keyPath, err) + return errors.Wrapf(err, "error importing key from %s", keyPath) } fmt.Fprintf(streams.Out(), "Successfully imported key from %s\n", keyPath) return nil @@ -78,11 +79,7 @@ func getPrivKeyBytesFromPath(keyPath string) ([]byte, error) { } defer from.Close() - keyBytes, err := ioutil.ReadAll(from) - if err != nil { - return nil, err - } - return keyBytes, nil + return ioutil.ReadAll(from) } func loadPrivKeyBytesToStore(privKeyBytes []byte, privKeyImporters []trustmanager.Importer, keyPath, keyName string, passRet notary.PassRetriever) error { diff --git a/cli/command/trust/signer_add.go b/cli/command/trust/signer_add.go index b595e189c3..830fb9eda4 100644 --- a/cli/command/trust/signer_add.go +++ b/cli/command/trust/signer_add.go @@ -63,7 +63,7 @@ func addSigner(cli command.Cli, options signerAddOptions) error { var errImages []string for _, imageName := range options.images { if err := addSignerToImage(cli, signerName, imageName, options.keys.GetAll()); err != nil { - fmt.Fprintln(cli.Out(), err.Error()) + fmt.Fprintln(cli.Err(), err.Error()) errImages = append(errImages, imageName) } else { fmt.Fprintf(cli.Out(), "Successfully added signer: %s to %s\n", signerName, imageName) @@ -124,7 +124,7 @@ func ingestPublicKeys(pubKeyPaths []string) ([]data.PublicKey, error) { if os.IsNotExist(err) { return nil, fmt.Errorf("file for public key does not exist: %s", pubKeyPath) } - return nil, fmt.Errorf("unable to read public key from file: %s", pubKeyPath) + return nil, errors.Wrapf(err, "unable to read public key from file: %s", pubKeyPath) } // Parse PEM bytes into type PublicKey diff --git a/cli/command/trust/signer_add_test.go b/cli/command/trust/signer_add_test.go index 41880ef9b5..8b61c0c31d 100644 --- a/cli/command/trust/signer_add_test.go +++ b/cli/command/trust/signer_add_test.go @@ -82,10 +82,10 @@ func TestSignerAddCommandNoTargetsKey(t *testing.T) { assert.EqualError(t, cmd.Execute(), "Failed to add signer to: alpine, linuxkit/alpine") assert.Contains(t, cli.OutBuffer().String(), "Adding signer \"alice\" to alpine...") - assert.Contains(t, cli.OutBuffer().String(), "no valid public key found") + assert.Contains(t, cli.ErrBuffer().String(), "no valid public key found") assert.Contains(t, cli.OutBuffer().String(), "Adding signer \"alice\" to linuxkit/alpine...") - assert.Contains(t, cli.OutBuffer().String(), "no valid public key found") + assert.Contains(t, cli.ErrBuffer().String(), "no valid public key found") } func TestSignerAddCommandBadKeyPath(t *testing.T) { @@ -102,8 +102,8 @@ func TestSignerAddCommandBadKeyPath(t *testing.T) { cmd.SetOutput(ioutil.Discard) assert.EqualError(t, cmd.Execute(), "Failed to add signer to: alpine") - expectedError := "\nAdding signer \"alice\" to alpine...\nfile for public key does not exist: /path/to/key.pem" - assert.Contains(t, cli.OutBuffer().String(), expectedError) + expectedError := "file for public key does not exist: /path/to/key.pem" + assert.Contains(t, cli.ErrBuffer().String(), expectedError) } func TestSignerAddCommandInvalidRepoName(t *testing.T) { @@ -120,11 +120,9 @@ func TestSignerAddCommandInvalidRepoName(t *testing.T) { cmd.SetOutput(ioutil.Discard) assert.EqualError(t, cmd.Execute(), "Failed to add signer to: 870d292919d01a0af7e7f056271dc78792c05f55f49b9b9012b6d89725bd9abd") - expectedOutput := fmt.Sprintf("\nAdding signer \"alice\" to %s...\n"+ - "invalid repository name (%s), cannot specify 64-byte hexadecimal strings\n", - imageName, imageName) + expectedErr := fmt.Sprintf("invalid repository name (%s), cannot specify 64-byte hexadecimal strings\n", imageName) - assert.Equal(t, expectedOutput, cli.OutBuffer().String()) + assert.Equal(t, expectedErr, cli.ErrBuffer().String()) } func TestIngestPublicKeys(t *testing.T) { diff --git a/cli/command/trust/signer_remove.go b/cli/command/trust/signer_remove.go index 5d5dd873e3..0f13dad41f 100644 --- a/cli/command/trust/signer_remove.go +++ b/cli/command/trust/signer_remove.go @@ -12,6 +12,7 @@ import ( "github.com/docker/cli/cli/trust" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -42,7 +43,7 @@ func removeSigner(cli command.Cli, options signerRemoveOptions) error { var errImages []string for _, image := range options.images { if err := removeSingleSigner(cli, image, options.signer, options.forceYes); err != nil { - fmt.Fprintln(cli.Out(), err.Error()) + fmt.Fprintln(cli.Err(), err.Error()) errImages = append(errImages, image) } } @@ -93,7 +94,7 @@ func removeSingleSigner(cli command.Cli, imageName, signerName string, forceYes } delegationRoles, err := notaryRepo.GetDelegationRoles() if err != nil { - return fmt.Errorf("Error retrieving signers for %s", imageName) + return errors.Wrapf(err, "error retrieving signers for %s", imageName) } var role data.Role for _, delRole := range delegationRoles { @@ -121,7 +122,7 @@ func removeSingleSigner(cli command.Cli, imageName, signerName string, forceYes return nil } } else if err != nil { - fmt.Fprintln(cli.Out(), err.Error()) + return err } if err = notaryRepo.RemoveDelegationKeys(releasesRoleTUFName, role.KeyIDs); err != nil { return err diff --git a/cli/command/trust/signer_remove_test.go b/cli/command/trust/signer_remove_test.go index cde0c0b4ab..fea6c65731 100644 --- a/cli/command/trust/signer_remove_test.go +++ b/cli/command/trust/signer_remove_test.go @@ -42,7 +42,7 @@ func TestTrustSignerRemoveErrors(t *testing.T) { { name: "not-an-image", args: []string{"user", "notanimage"}, - expectedError: "Error retrieving signers for notanimage", + expectedError: "error retrieving signers for notanimage", }, { name: "sha-reference", @@ -62,7 +62,7 @@ func TestTrustSignerRemoveErrors(t *testing.T) { cmd.SetArgs(tc.args) cmd.SetOutput(ioutil.Discard) cmd.Execute() - assert.Contains(t, cli.OutBuffer().String(), tc.expectedError) + assert.Contains(t, cli.ErrBuffer().String(), tc.expectedError) } } @@ -83,8 +83,8 @@ func TestRemoveMultipleSigners(t *testing.T) { cli.SetNotaryClient(getLoadedNotaryRepository) err := removeSigner(cli, signerRemoveOptions{signer: "test", images: []string{"signed-repo", "signed-repo"}, forceYes: true}) assert.EqualError(t, err, "Error removing signer from: signed-repo, signed-repo") - assert.Contains(t, cli.OutBuffer().String(), - "\nRemoving signer \"test\" from signed-repo...\nNo signer test for image signed-repo\n\nRemoving signer \"test\" from signed-repo...\nNo signer test for image signed-repo") + assert.Contains(t, cli.ErrBuffer().String(), + "No signer test for image signed-repo") } func TestRemoveLastSignerWarning(t *testing.T) { cli := test.NewFakeCli(&fakeClient{})