From 449147a60c48afeb12828fb3f3807446251d7992 Mon Sep 17 00:00:00 2001
From: Dmitry S <dsavints@gmail.com>
Date: Tue, 6 Feb 2024 18:27:51 +0100
Subject: [PATCH] address PR feedback

* simplify switch statements (remove unnecessary brackets)
* reword help text for '--ca-roots' and '--ca-intermediates'
flags for clarity

Signed-off-by: Dmitry S <dsavints@gmail.com>
---
 cmd/cosign/cli/options/certificate.go |  4 +-
 cmd/cosign/cli/verify/verify.go       | 76 ++++++++++++---------------
 2 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/cmd/cosign/cli/options/certificate.go b/cmd/cosign/cli/options/certificate.go
index d2b7c0b38c53..98e6b2d5f8b3 100644
--- a/cmd/cosign/cli/options/certificate.go
+++ b/cmd/cosign/cli/options/certificate.go
@@ -79,7 +79,9 @@ func (o *CertVerifyOptions) AddFlags(cmd *cobra.Command) {
 	// -- Cert extensions end --
 	cmd.Flags().StringVar(&o.CAIntermediates, "ca-intermediates", "",
 		"path to a file of intermediate CA certificates in PEM format which will be needed "+
-			"when building the certificate chains for the signing certificate. Conflicts with --certificate-chain.")
+			"when building the certificate chains for the signing certificate. "+
+			"The flag is optional, when used must be used together with --ca-roots, conflicts "+
+			"with --certificate-chain.")
 	_ = cmd.Flags().SetAnnotation("ca-intermediates", cobra.BashCompFilenameExt, []string{"cert"})
 	cmd.Flags().StringVar(&o.CARoots, "ca-roots", "",
 		"path to a bundle file of CA certificates in PEM format which will be needed "+
diff --git a/cmd/cosign/cli/verify/verify.go b/cmd/cosign/cli/verify/verify.go
index 4c90e9c882b5..5596b7c9bee4 100644
--- a/cmd/cosign/cli/verify/verify.go
+++ b/cmd/cosign/cli/verify/verify.go
@@ -181,57 +181,51 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) {
 	if keylessVerification(c.KeyRef, c.Sk) {
 		switch {
 		case c.CertChain != "":
-			{
-				chain, err := loadCertChainFromFileOrURL(c.CertChain)
-				if err != nil {
-					return err
-				}
-				co.RootCerts = x509.NewCertPool()
-				co.RootCerts.AddCert(chain[len(chain)-1])
-				if len(chain) > 1 {
-					co.IntermediateCerts = x509.NewCertPool()
-					for _, cert := range chain[:len(chain)-1] {
-						co.IntermediateCerts.AddCert(cert)
-					}
+			chain, err := loadCertChainFromFileOrURL(c.CertChain)
+			if err != nil {
+				return err
+			}
+			co.RootCerts = x509.NewCertPool()
+			co.RootCerts.AddCert(chain[len(chain)-1])
+			if len(chain) > 1 {
+				co.IntermediateCerts = x509.NewCertPool()
+				for _, cert := range chain[:len(chain)-1] {
+					co.IntermediateCerts.AddCert(cert)
 				}
 			}
 		case c.CARoots != "":
-			{
-				caRoots, err := loadCertChainFromFileOrURL(c.CARoots)
+			caRoots, err := loadCertChainFromFileOrURL(c.CARoots)
+			if err != nil {
+				return err
+			}
+			co.RootCerts = x509.NewCertPool()
+			if len(caRoots) > 0 {
+				for _, cert := range caRoots {
+					co.RootCerts.AddCert(cert)
+				}
+			}
+			if c.CAIntermediates != "" {
+				caIntermediates, err := loadCertChainFromFileOrURL(c.CAIntermediates)
 				if err != nil {
 					return err
 				}
-				co.RootCerts = x509.NewCertPool()
-				if len(caRoots) > 0 {
-					for _, cert := range caRoots {
-						co.RootCerts.AddCert(cert)
-					}
-				}
-				if c.CAIntermediates != "" {
-					caIntermediates, err := loadCertChainFromFileOrURL(c.CAIntermediates)
-					if err != nil {
-						return err
-					}
-					if len(caIntermediates) > 0 {
-						co.IntermediateCerts = x509.NewCertPool()
-						for _, cert := range caIntermediates {
-							co.IntermediateCerts.AddCert(cert)
-						}
+				if len(caIntermediates) > 0 {
+					co.IntermediateCerts = x509.NewCertPool()
+					for _, cert := range caIntermediates {
+						co.IntermediateCerts.AddCert(cert)
 					}
 				}
 			}
 		default:
-			{
-				// This performs an online fetch of the Fulcio roots. This is needed
-				// for verifying keyless certificates (both online and offline).
-				co.RootCerts, err = fulcio.GetRoots()
-				if err != nil {
-					return fmt.Errorf("getting Fulcio roots: %w", err)
-				}
-				co.IntermediateCerts, err = fulcio.GetIntermediates()
-				if err != nil {
-					return fmt.Errorf("getting Fulcio intermediates: %w", err)
-				}
+			// This performs an online fetch of the Fulcio roots from a TUF repository.
+			// This is needed for verifying keyless certificates (both online and offline).
+			co.RootCerts, err = fulcio.GetRoots()
+			if err != nil {
+				return fmt.Errorf("getting Fulcio roots: %w", err)
+			}
+			co.IntermediateCerts, err = fulcio.GetIntermediates()
+			if err != nil {
+				return fmt.Errorf("getting Fulcio intermediates: %w", err)
 			}
 		}
 	}