Merge pull request #14125 from mfojtik/signature-follow-up

Merged by openshift-bot

pkg/cmd/admin/admin.go

@@ -95,7 +95,7 @@ func NewCommandAdmin(name, fullName string, in io.Reader, out io.Writer, errout
 					migratestorage.NewCmdMigrateAPIStorage("storage", fullName+" "+migrate.MigrateRecommendedName+" storage", f, in, out, errout),
 				),
 				top.NewCommandTop(top.TopRecommendedName, fullName+" "+top.TopRecommendedName, f, out, errout),
-				image.NewCmdVerifyImageSignature("verify-image-signature", fullName, f, out, errout),
+				image.NewCmdVerifyImageSignature(name, fullName+" "+image.VerifyRecommendedName, f, out, errout),
 			},
 		},
 		{

pkg/cmd/admin/image/verify-signature.go

@@ -57,8 +57,7 @@ var (
 			--expected-identity=registry.local:5000/foo/bar:v1 --save
 
 	# Remove all signature verifications from the image
-	%[1]s sha256:c841e9b64e4579bd56c794bdd7c36e1c257110fd2404bebbb8b613e4935228c4 \
-			--expected-identity=registry.local:5000/foo/bar:v1 --remove-all
+	%[1]s sha256:c841e9b64e4579bd56c794bdd7c36e1c257110fd2404bebbb8b613e4935228c4 --remove-all
 	`)
 )
 
@@ -79,6 +78,10 @@ type VerifyImageSignatureOptions struct {
 	ErrOut io.Writer
 }
 
+const (
+	VerifyRecommendedName = "verify-image-signature"
+)
+
 func NewCmdVerifyImageSignature(name, fullName string, f *clientcmd.Factory, out, errOut io.Writer) *cobra.Command {
 	opts := &VerifyImageSignatureOptions{
 		ErrOut:       errOut,
@@ -90,10 +93,10 @@ func NewCmdVerifyImageSignature(name, fullName string, f *clientcmd.Factory, out
 		PublicKeyFilename: filepath.Join(os.Getenv("GNUPGHOME"), "pubring.gpg"),
 	}
 	cmd := &cobra.Command{
-		Use:     fmt.Sprintf("%s IMAGE --expected-identity=EXPECTED_IDENTITY [--save]", name),
+		Use:     fmt.Sprintf("%s IMAGE --expected-identity=EXPECTED_IDENTITY [--save]", VerifyRecommendedName),
 		Short:   "Verify the image identity contained in the image signature",
 		Long:    verifyImageSignatureLongDesc,
-		Example: fmt.Sprintf(verifyImageSignatureExample, name),
+		Example: fmt.Sprintf(verifyImageSignatureExample, fullName),
 		Run: func(cmd *cobra.Command, args []string) {
 			kcmdutil.CheckErr(opts.Validate())
 			kcmdutil.CheckErr(opts.Complete(f, cmd, args, out))
@@ -109,11 +112,16 @@ func NewCmdVerifyImageSignature(name, fullName string, f *clientcmd.Factory, out
 }
 
 func (o *VerifyImageSignatureOptions) Validate() error {
-	if len(o.ExpectedIdentity) == 0 {
-		return errors.New("the --expected-identity is required")
+	if !o.RemoveAll {
+		if len(o.ExpectedIdentity) == 0 {
+			return errors.New("the --expected-identity is required")
+		}
+		if _, err := imageapi.ParseDockerImageReference(o.ExpectedIdentity); err != nil {
+			return errors.New("the --expected-identity must be valid image reference")
+		}
 	}
-	if _, err := imageapi.ParseDockerImageReference(o.ExpectedIdentity); err != nil {
-		return errors.New("the --expected-identity must be valid image reference")
+	if o.RemoveAll && len(o.ExpectedIdentity) > 0 {
+		return errors.New("the --expected-identity cannot be used when removing all verifications")
 	}
 	return nil
 }
@@ -173,17 +181,16 @@ func (o VerifyImageSignatureOptions) Run() error {
 	}
 	defer pc.Destroy()
 
-	for i, s := range img.Signatures {
-		if o.RemoveAll {
-			o.clearSignatureVerificationStatus(&img.Signatures[i])
-			continue
-		}
+	if o.RemoveAll {
+		img.Signatures = []imageapi.ImageSignature{}
+	}
 
+	for i, s := range img.Signatures {
 		// Verify the signature against the policy
 		signedBy, err := o.verifySignature(pc, img, s.Content)
 		if err != nil {
-			fmt.Fprintf(o.ErrOut, "error: %s: %v\n", o.InputImage, err)
-			o.clearSignatureVerificationStatus(&img.Signatures[i])
+			fmt.Fprintf(o.ErrOut, "error verifying signature %s for image %s (verification status will be removed): %v\n", img.Signatures[i].Name, o.InputImage, err)
+			img.Signatures[i] = imageapi.ImageSignature{}
 			continue
 		}
 		fmt.Fprintf(o.Out, "image %q identity is now confirmed (signed by GPG key %q)\n", o.InputImage, signedBy)
@@ -212,7 +219,7 @@ func (o VerifyImageSignatureOptions) Run() error {
 		img.Signatures[i].IssuedBy.CommonName = signedBy
 	}
 
-	if o.Save {
+	if o.Save || o.RemoveAll {
 		_, err := o.Client.Images().Update(img)
 		return err
 	}
@@ -253,14 +260,6 @@ func (o *VerifyImageSignatureOptions) verifySignature(pc *signature.PolicyContex
 	}
 }
 
-// clearSignatureVerificationStatus removes the current image signature from the Image object by
-// erasing all signature fields that were previously set (when image signature was
-// previously verified).
-func (o *VerifyImageSignatureOptions) clearSignatureVerificationStatus(s *imageapi.ImageSignature) {
-	s.Conditions = []imageapi.SignatureCondition{}
-	s.IssuedBy = nil
-}
-
 // dummyDockerTransport is containers/image/docker.Transport, except that it only provides identity information.
 var dummyDockerTransport = dockerTransport{}