From 007d36868115a644968031c2f430d17cdaed0d75 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 14 Mar 2024 22:38:33 -0600 Subject: [PATCH] use GH_DEBUG value for io handling Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/api/client.go | 6 +- pkg/cmd/attestation/api/client_test.go | 8 +-- pkg/cmd/attestation/download/download.go | 4 +- pkg/cmd/attestation/download/download_test.go | 4 +- pkg/cmd/attestation/download/options.go | 4 +- pkg/cmd/attestation/inspect/inspect.go | 4 +- pkg/cmd/attestation/inspect/inspect_test.go | 4 +- pkg/cmd/attestation/inspect/options.go | 4 +- pkg/cmd/attestation/io/handler.go | 61 +++++++++++++++++ pkg/cmd/attestation/logging/logger.go | 68 ------------------- pkg/cmd/attestation/verification/sigstore.go | 6 +- .../attestation/verification/sigstore_test.go | 4 +- pkg/cmd/attestation/verify/options.go | 4 +- pkg/cmd/attestation/verify/verify.go | 6 +- pkg/cmd/attestation/verify/verify_test.go | 6 +- 15 files changed, 93 insertions(+), 100 deletions(-) create mode 100644 pkg/cmd/attestation/io/handler.go delete mode 100644 pkg/cmd/attestation/logging/logger.go diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go index 1e7e43246..1660fdbb1 100644 --- a/pkg/cmd/attestation/api/client.go +++ b/pkg/cmd/attestation/api/client.go @@ -7,7 +7,7 @@ import ( "strings" "github.com/cli/cli/v2/api" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + ioconfig "github.com/cli/cli/v2/pkg/cmd/attestation/io" ) const ( @@ -28,10 +28,10 @@ type Client interface { type LiveClient struct { api apiClient host string - logger *logging.Logger + logger *ioconfig.Handler } -func NewLiveClient(hc *http.Client, l *logging.Logger) *LiveClient { +func NewLiveClient(hc *http.Client, l *ioconfig.Handler) *LiveClient { liveAPIClient := api.NewClientFromHTTP(hc) return &LiveClient{ api: liveAPIClient, diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go index bf7791b81..7044a1cc5 100644 --- a/pkg/cmd/attestation/api/client_test.go +++ b/pkg/cmd/attestation/api/client_test.go @@ -3,7 +3,7 @@ package api import ( "testing" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/stretchr/testify/require" ) @@ -18,7 +18,7 @@ func NewClientWithMockGHClient(hasNextPage bool) Client { fetcher := mockDataGenerator{ NumAttestations: 5, } - l := logging.NewTestLogger() + l := io.NewTestHandler() if hasNextPage { return &LiveClient{ @@ -138,7 +138,7 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) { api: mockAPIClient{ OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations, }, - logger: logging.NewTestLogger(), + logger: io.NewTestHandler(), } attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit) @@ -161,7 +161,7 @@ func TestGetByDigest_Error(t *testing.T) { api: mockAPIClient{ OnRESTWithNext: fetcher.OnRESTWithNextError, }, - logger: logging.NewTestLogger(), + logger: io.NewTestHandler(), } attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit) diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index c5fd3a794..cbaef21ff 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -8,7 +8,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" "github.com/cli/cli/v2/pkg/cmd/attestation/auth" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmdutil" @@ -61,7 +61,7 @@ func NewDownloadCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Comman // along with information about how use the command PreRunE: func(cmd *cobra.Command, args []string) error { // Create a logger for use throughout the download command - opts.Logger = logging.NewLogger(f.IOStreams, false, opts.Verbose) + opts.Logger = io.NewHandler(f.IOStreams) // set the artifact path opts.ArtifactPath = args[0] diff --git a/pkg/cmd/attestation/download/download_test.go b/pkg/cmd/attestation/download/download_test.go index 2b152b937..6f0385114 100644 --- a/pkg/cmd/attestation/download/download_test.go +++ b/pkg/cmd/attestation/download/download_test.go @@ -9,7 +9,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" @@ -185,7 +185,7 @@ func TestRunDownload(t *testing.T) { Owner: "sigstore", Store: store, Limit: 30, - Logger: logging.NewTestLogger(), + Logger: io.NewTestHandler(), } t.Run("fetch and store attestations successfully with owner", func(t *testing.T) { diff --git a/pkg/cmd/attestation/download/options.go b/pkg/cmd/attestation/download/options.go index edb3c1b7b..cb9cb8978 100644 --- a/pkg/cmd/attestation/download/options.go +++ b/pkg/cmd/attestation/download/options.go @@ -5,7 +5,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" ) const ( @@ -17,7 +17,7 @@ type Options struct { APIClient api.Client ArtifactPath string DigestAlgorithm string - Logger *logging.Logger + Logger *io.Handler Limit int Store MetadataStore OCIClient oci.Client diff --git a/pkg/cmd/attestation/inspect/inspect.go b/pkg/cmd/attestation/inspect/inspect.go index f0f46c3cf..a41922c88 100644 --- a/pkg/cmd/attestation/inspect/inspect.go +++ b/pkg/cmd/attestation/inspect/inspect.go @@ -8,7 +8,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" "github.com/cli/cli/v2/pkg/cmd/attestation/auth" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmdutil" @@ -53,7 +53,7 @@ func NewInspectCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command `), PreRunE: func(cmd *cobra.Command, args []string) error { // Create a logger for use throughout the inspect command - opts.Logger = logging.NewDefaultLogger(f.IOStreams) + opts.Logger = io.NewHandler(f.IOStreams) // set the artifact path opts.ArtifactPath = args[0] diff --git a/pkg/cmd/attestation/inspect/inspect_test.go b/pkg/cmd/attestation/inspect/inspect_test.go index 98e2c4a95..a3cc6a6de 100644 --- a/pkg/cmd/attestation/inspect/inspect_test.go +++ b/pkg/cmd/attestation/inspect/inspect_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmdutil" @@ -140,7 +140,7 @@ func TestRunInspect(t *testing.T) { ArtifactPath: artifactPath, BundlePath: bundlePath, DigestAlgorithm: "sha512", - Logger: logging.NewTestLogger(), + Logger: io.NewTestHandler(), OCIClient: oci.MockClient{}, } diff --git a/pkg/cmd/attestation/inspect/options.go b/pkg/cmd/attestation/inspect/options.go index 4e0a3fc49..1831190aa 100644 --- a/pkg/cmd/attestation/inspect/options.go +++ b/pkg/cmd/attestation/inspect/options.go @@ -4,7 +4,7 @@ import ( "path/filepath" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" ) // Options captures the options for the inspect command @@ -15,7 +15,7 @@ type Options struct { JsonResult bool Verbose bool Quiet bool - Logger *logging.Logger + Logger *io.Handler OCIClient oci.Client } diff --git a/pkg/cmd/attestation/io/handler.go b/pkg/cmd/attestation/io/handler.go new file mode 100644 index 000000000..b41d4ddbf --- /dev/null +++ b/pkg/cmd/attestation/io/handler.go @@ -0,0 +1,61 @@ +package io + +import ( + "fmt" + + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/utils" +) + +type Handler struct { + ColorScheme *iostreams.ColorScheme + IO *iostreams.IOStreams + debugEnabled bool +} + +func NewHandler(io *iostreams.IOStreams) *Handler { + enabled, _ := utils.IsDebugEnabled() + + return &Handler{ + ColorScheme: io.ColorScheme(), + IO: io, + debugEnabled: enabled, + } +} + +func NewTestHandler() *Handler { + testIO, _, _, _ := iostreams.Test() + return NewHandler(testIO) +} + +// Printf writes the formatted arguments to the stderr writer. +func (h *Handler) Printf(f string, v ...interface{}) (int, error) { + if !h.IO.IsStdoutTTY() { + return 0, nil + } + return fmt.Fprintf(h.IO.ErrOut, f, v...) +} + +// Println writes the arguments to the stderr writer with a newline at the end. +func (h *Handler) Println(v ...interface{}) (int, error) { + if !h.IO.IsStdoutTTY() { + return 0, nil + } + return fmt.Fprintln(h.IO.ErrOut, v...) +} + +func (h *Handler) VerbosePrint(msg string) (int, error) { + if !h.debugEnabled || !h.IO.IsStdoutTTY() { + return 0, nil + } + + return fmt.Fprintln(h.IO.ErrOut, msg) +} + +func (h *Handler) VerbosePrintf(f string, v ...interface{}) (int, error) { + if !h.debugEnabled || !h.IO.IsStdoutTTY() { + return 0, nil + } + + return fmt.Fprintf(h.IO.ErrOut, f, v...) +} diff --git a/pkg/cmd/attestation/logging/logger.go b/pkg/cmd/attestation/logging/logger.go deleted file mode 100644 index c82cbea06..000000000 --- a/pkg/cmd/attestation/logging/logger.go +++ /dev/null @@ -1,68 +0,0 @@ -package logging - -import ( - "fmt" - - "github.com/cli/cli/v2/pkg/iostreams" -) - -type Logger struct { - ColorScheme *iostreams.ColorScheme - IO *iostreams.IOStreams - quiet bool - verbose bool -} - -func NewLogger(io *iostreams.IOStreams, isQuiet, isVerbose bool) *Logger { - return &Logger{ - ColorScheme: io.ColorScheme(), - IO: io, - quiet: isQuiet, - verbose: isVerbose, - } -} - -// NewDefaultLogger returns a Logger that with the default logging settings -func NewDefaultLogger(io *iostreams.IOStreams) *Logger { - isQuiet := false - isVerbose := false - - return NewLogger(io, isQuiet, isVerbose) -} - -func NewTestLogger() *Logger { - testIO, _, _, _ := iostreams.Test() - return NewDefaultLogger(testIO) -} - -// Printf writes the formatted arguments to the stderr writer. -func (l *Logger) Printf(f string, v ...interface{}) (int, error) { - if l.quiet || !l.IO.IsStdoutTTY() { - return 0, nil - } - return fmt.Fprintf(l.IO.ErrOut, f, v...) -} - -// Println writes the arguments to the stderr writer with a newline at the end. -func (l *Logger) Println(v ...interface{}) (int, error) { - if l.quiet || !l.IO.IsStdoutTTY() { - return 0, nil - } - return fmt.Fprintln(l.IO.ErrOut, v...) -} - -func (l *Logger) VerbosePrint(msg string) (int, error) { - if !l.verbose || !l.IO.IsStdoutTTY() { - return 0, nil - } - - return fmt.Fprintln(l.IO.ErrOut, msg) -} - -func (l *Logger) VerbosePrintf(f string, v ...interface{}) (int, error) { - if !l.verbose || !l.IO.IsStdoutTTY() { - return 0, nil - } - - return fmt.Fprintf(l.IO.ErrOut, f, v...) -} diff --git a/pkg/cmd/attestation/verification/sigstore.go b/pkg/cmd/attestation/verification/sigstore.go index 938280133..0f99313cc 100644 --- a/pkg/cmd/attestation/verification/sigstore.go +++ b/pkg/cmd/attestation/verification/sigstore.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/cli/cli/v2/pkg/cmd/attestation/api" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/sigstore/sigstore-go/pkg/bundle" "github.com/sigstore/sigstore-go/pkg/root" @@ -30,7 +30,7 @@ type SigstoreResults struct { type SigstoreConfig struct { CustomTrustedRoot string - Logger *logging.Logger + Logger *io.Handler NoPublicGood bool } @@ -40,7 +40,7 @@ type SigstoreVerifier struct { customVerifier *verify.SignedEntityVerifier policy verify.PolicyBuilder onlyVerifyWithGithub bool - Logger *logging.Logger + Logger *io.Handler } // NewSigstoreVerifier creates a new SigstoreVerifier struct diff --git a/pkg/cmd/attestation/verification/sigstore_test.go b/pkg/cmd/attestation/verification/sigstore_test.go index 7fd2a8590..204b5e583 100644 --- a/pkg/cmd/attestation/verification/sigstore_test.go +++ b/pkg/cmd/attestation/verification/sigstore_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/sigstore/sigstore-go/pkg/verify" @@ -30,7 +30,7 @@ func TestNewSigstoreVerifier(t *testing.T) { require.NoError(t, err) c := SigstoreConfig{ - Logger: logging.NewTestLogger(), + Logger: io.NewTestHandler(), } verifier, err := NewSigstoreVerifier(c, policy) require.NoError(t, err) diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index b38f31369..de23679fd 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -7,7 +7,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" ) // Options captures the options for the verify command @@ -27,7 +27,7 @@ type Options struct { SANRegex string Verbose bool APIClient api.Client - Logger *logging.Logger + Logger *io.Handler Limit int OCIClient oci.Client } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 227f8ea5c..b1d4884a3 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -9,7 +9,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" "github.com/cli/cli/v2/pkg/cmd/attestation/auth" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmdutil" @@ -70,7 +70,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command // along with information about how use the command PreRunE: func(cmd *cobra.Command, args []string) error { // Create a logger for use throughout the verify command - opts.Logger = logging.NewLogger(f.IOStreams, opts.Quiet, opts.Verbose) + opts.Logger = io.NewHandler(f.IOStreams) // set the artifact path opts.ArtifactPath = args[0] @@ -215,7 +215,7 @@ func runVerify(opts *Options) error { return nil } -func verifySLSAPredicateType(logger *logging.Logger, apr []*verification.AttestationProcessingResult) error { +func verifySLSAPredicateType(logger *io.Handler, apr []*verification.AttestationProcessingResult) error { logger.VerbosePrint("Evaluating attestations have valid SLSA predicate type") for _, result := range apr { diff --git a/pkg/cmd/attestation/verify/verify_test.go b/pkg/cmd/attestation/verify/verify_test.go index 7f26713d7..86d40b32c 100644 --- a/pkg/cmd/attestation/verify/verify_test.go +++ b/pkg/cmd/attestation/verify/verify_test.go @@ -7,7 +7,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/cli/cli/v2/pkg/cmd/attestation/logging" + "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmdutil" @@ -222,7 +222,7 @@ func TestNewVerifyCmd(t *testing.T) { } func TestRunVerify(t *testing.T) { - logger := logging.NewTestLogger() + logger := io.NewTestHandler() publicGoodOpts := Options{ ArtifactPath: test.NormalizeRelativePath("../test/data/sigstore-js-2.1.0.tgz"), @@ -372,7 +372,7 @@ func TestVerifySLSAPredicateType_InvalidPredicate(t *testing.T) { }, } - err := verifySLSAPredicateType(logging.NewTestLogger(), apr) + err := verifySLSAPredicateType(io.NewTestHandler(), apr) require.Error(t, err) require.ErrorIs(t, err, ErrNoMatchingSLSAPredicate) }