Add --timeout support to sign command (sigstore#1379)

We can now specify a global `-t`/`--timeout` option to specify a timeout for any command. It is implemented for `sign` for now, which resolves a leftover `TODO`. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
sota1235 · Feb 2, 2022 · f97a146 · f97a146
1 parent 3dac238
commit f97a146
Show file tree

Hide file tree

Showing 59 changed files with 83 additions and 55 deletions.
diff --git a/cmd/cosign/cli/attest.go b/cmd/cosign/cli/attest.go
@@ -71,7 +71,7 @@ func Attest() *cobra.Command {
 			}
 			for _, img := range args {
 				if err := attest.AttestCmd(cmd.Context(), ko, o.Registry, img, o.Cert, o.NoUpload,
-					o.Predicate.Path, o.Force, o.Predicate.Type, o.Replace, o.Timeout); err != nil {
+					o.Predicate.Path, o.Force, o.Predicate.Type, o.Replace, ro.Timeout); err != nil {
 					return errors.Wrapf(err, "signing %s", img)
 				}
 			}

diff --git a/cmd/cosign/cli/options/attest.go b/cmd/cosign/cli/options/attest.go
@@ -16,8 +16,6 @@
 package options
 
 import (
-	"time"
-
 	"github.com/spf13/cobra"
 )
 
@@ -29,7 +27,6 @@ type AttestOptions struct {
 	Force     bool
 	Recursive bool
 	Replace   bool
-	Timeout   time.Duration
 
 	Rekor       RekorOptions
 	Fulcio      FulcioOptions
@@ -67,7 +64,4 @@ func (o *AttestOptions) AddFlags(cmd *cobra.Command) {
 
 	cmd.Flags().BoolVarP(&o.Replace, "replace", "", false,
 		"")
-
-	cmd.Flags().DurationVar(&o.Timeout, "timeout", time.Second*30,
-		"HTTP Timeout defaults to 30 seconds")
 }
diff --git a/cmd/cosign/cli/options/policy.go b/cmd/cosign/cli/options/policy.go
@@ -16,8 +16,6 @@
 package options
 
 import (
-	"time"
-
 	"github.com/spf13/cobra"
 )
 
@@ -63,7 +61,6 @@ type PolicySignOptions struct {
 	Registry RegistryOptions
 	Fulcio   FulcioOptions
 	Rekor    RekorOptions
-	Timeout  time.Duration
 
 	OIDC OIDCOptions
 }
@@ -78,9 +75,6 @@ func (o *PolicySignOptions) AddFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&o.OutFile, "out", "o",
 		"output policy locally")
 
-	cmd.Flags().DurationVar(&o.Timeout, "timeout", time.Second*30,
-		"HTTP Timeout defaults to 30 seconds")
-
 	o.Registry.AddFlags(cmd)
 	o.Fulcio.AddFlags(cmd)
 	o.Rekor.AddFlags(cmd)

diff --git a/cmd/cosign/cli/options/root.go b/cmd/cosign/cli/options/root.go
@@ -16,15 +16,21 @@
 package options
 
 import (
+	"time"
+
 	"github.com/spf13/cobra"
 )
 
 // RootOptions define flags and options for the root cosign cli.
 type RootOptions struct {
 	OutputFile string
 	Verbose    bool
+	Timeout    time.Duration
 }
 
+// DefaultTimeout specifies the default timeout for commands.
+const DefaultTimeout = 3 * time.Minute
+
 var _ Interface = (*RootOptions)(nil)
 
 // AddFlags implements Interface
@@ -34,4 +40,7 @@ func (o *RootOptions) AddFlags(cmd *cobra.Command) {
 
 	cmd.PersistentFlags().BoolVarP(&o.Verbose, "verbose", "d", false,
 		"log debug output")
+
+	cmd.PersistentFlags().DurationVarP(&o.Timeout, "timeout", "t", DefaultTimeout,
+		"timeout for commands")
 }
diff --git a/cmd/cosign/cli/options/signblob.go b/cmd/cosign/cli/options/signblob.go
@@ -16,8 +16,6 @@
 package options
 
 import (
-	"time"
-
 	"github.com/spf13/cobra"
 )
 
@@ -34,7 +32,6 @@ type SignBlobOptions struct {
 	Rekor             RekorOptions
 	OIDC              OIDCOptions
 	Registry          RegistryOptions
-	Timeout           time.Duration
 	BundlePath        string
 }
 
@@ -63,9 +60,6 @@ func (o *SignBlobOptions) AddFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVar(&o.OutputCertificate, "output-certificate", "",
 		"write the certificate to FILE")
 
-	cmd.Flags().DurationVar(&o.Timeout, "timeout", time.Second*30,
-		"HTTP Timeout defaults to 30 seconds")
-
 	cmd.Flags().StringVar(&o.BundlePath, "bundle", "",
 		"write everything required to verify the blob to a FILE")
 }
diff --git a/cmd/cosign/cli/policy_init.go b/cmd/cosign/cli/policy_init.go
@@ -168,9 +168,9 @@ func signPolicy() *cobra.Command {
 		Long:  "policy is used to manage a root.json policy\nfor keyless signing delegation. This is used to establish a policy for a registry namespace,\na signing threshold and a list of maintainers who can sign over the body section.",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := cmd.Context()
-			if o.Timeout != 0 {
+			if ro.Timeout != 0 {
 				var cancelFn context.CancelFunc
-				ctx, cancelFn = context.WithTimeout(ctx, o.Timeout)
+				ctx, cancelFn = context.WithTimeout(ctx, ro.Timeout)
 				defer cancelFn()
 			}
 

diff --git a/cmd/cosign/cli/sign.go b/cmd/cosign/cli/sign.go
@@ -89,7 +89,7 @@ func Sign() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			if err := sign.SignCmd(cmd.Context(), ko, o.Registry, annotationsMap.Annotations, args, o.Cert, o.Upload, o.OutputSignature, o.OutputCertificate, o.PayloadPath, o.Force, o.Recursive, o.Attachment); err != nil {
+			if err := sign.SignCmd(ro, ko, o.Registry, annotationsMap.Annotations, args, o.Cert, o.Upload, o.OutputSignature, o.OutputCertificate, o.PayloadPath, o.Force, o.Recursive, o.Attachment); err != nil {
 				if o.Attachment == "" {
 					return errors.Wrapf(err, "signing %v", args)
 				}

diff --git a/cmd/cosign/cli/sign/sign.go b/cmd/cosign/cli/sign/sign.go
@@ -95,7 +95,7 @@ func GetAttachedImageRef(ref name.Reference, attachment string, opts ...ociremot
 }
 
 // nolint
-func SignCmd(ctx context.Context, ko KeyOpts, regOpts options.RegistryOptions, annotations map[string]interface{},
+func SignCmd(ro *options.RootOptions, ko KeyOpts, regOpts options.RegistryOptions, annotations map[string]interface{},
 	imgs []string, certPath string, upload bool, outputSignature, outputCertificate string, payloadPath string, force bool, recursive bool, attachment string) error {
 	if options.EnableExperimental() {
 		if options.NOf(ko.KeyRef, ko.Sk) > 1 {
@@ -107,12 +107,8 @@ func SignCmd(ctx context.Context, ko KeyOpts, regOpts options.RegistryOptions, a
 		}
 	}
 
-	// TODO: accept a timeout argument and uncomment the block below
-	// if timeout != 0 {
-	// 	var cancelFn context.CancelFunc
-	// 	ctx, cancelFn = context.WithTimeout(ctx, timeout)
-	// 	defer cancelFn()
-	// }
+	ctx, cancel := context.WithTimeout(context.Background(), ro.Timeout)
+	defer cancel()
 
 	sv, err := SignerFromKeyOpts(ctx, certPath, ko)
 	if err != nil {

diff --git a/cmd/cosign/cli/sign/sign_test.go b/cmd/cosign/cli/sign/sign_test.go
@@ -16,7 +16,6 @@
 package sign
 
 import (
-	"context"
 	"errors"
 	"testing"
 
@@ -27,7 +26,7 @@ import (
 // TestSignCmdLocalKeyAndSk verifies the SignCmd returns an error
 // if both a local key path and a sk are specified
 func TestSignCmdLocalKeyAndSk(t *testing.T) {
-	ctx := context.Background()
+	ro := &options.RootOptions{Timeout: options.DefaultTimeout}
 
 	for _, ko := range []KeyOpts{
 		// local and sk keys
@@ -37,7 +36,7 @@ func TestSignCmdLocalKeyAndSk(t *testing.T) {
 			Sk:       true,
 		},
 	} {
-		err := SignCmd(ctx, ko, options.RegistryOptions{}, nil, nil, "", false, "", "", "", false, false, "")
+		err := SignCmd(ro, ko, options.RegistryOptions{}, nil, nil, "", false, "", "", "", false, false, "")
 		if (errors.Is(err, &options.KeyParseError{}) == false) {
 			t.Fatal("expected KeyParseError")
 		}

diff --git a/cmd/cosign/cli/signblob.go b/cmd/cosign/cli/signblob.go
@@ -84,7 +84,7 @@ func SignBlob() *cobra.Command {
 					fmt.Fprintln(os.Stderr, "WARNING: the '--output' flag is deprecated and will be removed in the future. Use '--output-signature'")
 					o.OutputSignature = o.Output
 				}
-				if _, err := sign.SignBlobCmd(cmd.Context(), ko, o.Registry, blob, o.Base64Output, o.OutputSignature, o.OutputCertificate, o.Timeout); err != nil {
+				if _, err := sign.SignBlobCmd(cmd.Context(), ko, o.Registry, blob, o.Base64Output, o.OutputSignature, o.OutputCertificate, ro.Timeout); err != nil {
 					return errors.Wrapf(err, "signing %s", blob)
 				}
 			}

diff --git a/doc/cosign.md b/doc/cosign.md
diff --git a/doc/cosign_attach.md b/doc/cosign_attach.md
diff --git a/doc/cosign_attach_attestation.md b/doc/cosign_attach_attestation.md
diff --git a/doc/cosign_attach_sbom.md b/doc/cosign_attach_sbom.md
diff --git a/doc/cosign_attach_signature.md b/doc/cosign_attach_signature.md
diff --git a/doc/cosign_attest.md b/doc/cosign_attest.md
diff --git a/doc/cosign_clean.md b/doc/cosign_clean.md
diff --git a/doc/cosign_completion.md b/doc/cosign_completion.md
diff --git a/doc/cosign_copy.md b/doc/cosign_copy.md
diff --git a/doc/cosign_dockerfile.md b/doc/cosign_dockerfile.md
diff --git a/doc/cosign_dockerfile_verify.md b/doc/cosign_dockerfile_verify.md
diff --git a/doc/cosign_download.md b/doc/cosign_download.md
diff --git a/doc/cosign_download_attestation.md b/doc/cosign_download_attestation.md
diff --git a/doc/cosign_download_sbom.md b/doc/cosign_download_sbom.md
diff --git a/doc/cosign_download_signature.md b/doc/cosign_download_signature.md
diff --git a/doc/cosign_generate-key-pair.md b/doc/cosign_generate-key-pair.md
diff --git a/doc/cosign_generate.md b/doc/cosign_generate.md
diff --git a/doc/cosign_import-key-pair.md b/doc/cosign_import-key-pair.md
diff --git a/doc/cosign_initialize.md b/doc/cosign_initialize.md
diff --git a/doc/cosign_load.md b/doc/cosign_load.md
diff --git a/doc/cosign_manifest.md b/doc/cosign_manifest.md
diff --git a/doc/cosign_manifest_verify.md b/doc/cosign_manifest_verify.md
diff --git a/doc/cosign_piv-tool.md b/doc/cosign_piv-tool.md
diff --git a/doc/cosign_piv-tool_attestation.md b/doc/cosign_piv-tool_attestation.md