From bbddac1f9db68ee79f3897a6c16d678d011636d2 Mon Sep 17 00:00:00 2001 From: William Murphy Date: Wed, 31 Jan 2024 16:39:17 -0500 Subject: [PATCH] Fix attest with --key (#2551) Fix passing "--key" to the attest command. Additionally, pull in an update to the clio CLI library to permit unit testing that flags and env vars are parsed to the correct field on command options structs. This testing strategy was needed here because testing attestation in an end to end test requires a prohibitive amount of setup. --------- Signed-off-by: Alex Goodman Signed-off-by: Will Murphy Co-authored-by: Alex Goodman --- cmd/syft/cli/cli.go | 46 +-------------- cmd/syft/internal/clio_setup_config.go | 55 ++++++++++++++++++ cmd/syft/internal/commands/attest.go | 2 +- cmd/syft/internal/commands/attest_test.go | 62 +++++++++++++++++++++ cmd/syft/internal/commands/commands_test.go | 20 +++++++ cmd/syft/internal/options/attest.go | 4 +- go.mod | 2 +- go.sum | 4 +- test/cli/scan_cmd_test.go | 1 + 9 files changed, 147 insertions(+), 49 deletions(-) create mode 100644 cmd/syft/internal/clio_setup_config.go create mode 100644 cmd/syft/internal/commands/commands_test.go diff --git a/cmd/syft/cli/cli.go b/cmd/syft/cli/cli.go index 534e90ef6..1d1b3cf3a 100644 --- a/cmd/syft/cli/cli.go +++ b/cmd/syft/cli/cli.go @@ -8,13 +8,8 @@ import ( "github.com/spf13/cobra" "github.com/anchore/clio" - "github.com/anchore/stereoscope" - handler "github.com/anchore/syft/cmd/syft/cli/ui" + "github.com/anchore/syft/cmd/syft/internal" "github.com/anchore/syft/cmd/syft/internal/commands" - "github.com/anchore/syft/cmd/syft/internal/ui" - "github.com/anchore/syft/internal/bus" - "github.com/anchore/syft/internal/log" - "github.com/anchore/syft/internal/redact" ) // Application constructs the `syft packages` command and aliases the root command to `syft packages`. @@ -34,44 +29,7 @@ func Command(id clio.Identification) *cobra.Command { } func create(id clio.Identification, out io.Writer) (clio.Application, *cobra.Command) { - clioCfg := clio.NewSetupConfig(id). - WithGlobalConfigFlag(). // add persistent -c for reading an application config from - WithGlobalLoggingFlags(). // add persistent -v and -q flags tied to the logging config - WithConfigInRootHelp(). // --help on the root command renders the full application config in the help text - WithUIConstructor( - // select a UI based on the logging configuration and state of stdin (if stdin is a tty) - func(cfg clio.Config) ([]clio.UI, error) { - noUI := ui.None(out, cfg.Log.Quiet) - if !cfg.Log.AllowUI(os.Stdin) || cfg.Log.Quiet { - return []clio.UI{noUI}, nil - } - - return []clio.UI{ - ui.New(out, cfg.Log.Quiet, - handler.New(handler.DefaultHandlerConfig()), - ), - noUI, - }, nil - }, - ). - WithInitializers( - func(state *clio.State) error { - // clio is setting up and providing the bus, redact store, and logger to the application. Once loaded, - // we can hoist them into the internal packages for global use. - stereoscope.SetBus(state.Bus) - bus.Set(state.Bus) - - redact.Set(state.RedactStore) - - log.Set(state.Logger) - stereoscope.SetLogger(state.Logger) - - return nil - }, - ). - WithPostRuns(func(state *clio.State, err error) { - stereoscope.Cleanup() - }) + clioCfg := internal.AppClioSetupConfig(id, out) app := clio.New(*clioCfg) diff --git a/cmd/syft/internal/clio_setup_config.go b/cmd/syft/internal/clio_setup_config.go new file mode 100644 index 000000000..d1a34a692 --- /dev/null +++ b/cmd/syft/internal/clio_setup_config.go @@ -0,0 +1,55 @@ +package internal + +import ( + "io" + "os" + + "github.com/anchore/clio" + "github.com/anchore/stereoscope" + ui2 "github.com/anchore/syft/cmd/syft/cli/ui" + "github.com/anchore/syft/cmd/syft/internal/ui" + "github.com/anchore/syft/internal/bus" + "github.com/anchore/syft/internal/log" + "github.com/anchore/syft/internal/redact" +) + +func AppClioSetupConfig(id clio.Identification, out io.Writer) *clio.SetupConfig { + clioCfg := clio.NewSetupConfig(id). + WithGlobalConfigFlag(). // add persistent -c for reading an application config from + WithGlobalLoggingFlags(). // add persistent -v and -q flags tied to the logging config + WithConfigInRootHelp(). // --help on the root command renders the full application config in the help text + WithUIConstructor( + // select a UI based on the logging configuration and state of stdin (if stdin is a tty) + func(cfg clio.Config) ([]clio.UI, error) { + noUI := ui.None(out, cfg.Log.Quiet) + if !cfg.Log.AllowUI(os.Stdin) || cfg.Log.Quiet { + return []clio.UI{noUI}, nil + } + + return []clio.UI{ + ui.New(out, cfg.Log.Quiet, + ui2.New(ui2.DefaultHandlerConfig()), + ), + noUI, + }, nil + }, + ). + WithInitializers( + func(state *clio.State) error { + // clio is setting up and providing the bus, redact store, and logger to the application. Once loaded, + // we can hoist them into the internal packages for global use. + stereoscope.SetBus(state.Bus) + bus.Set(state.Bus) + + redact.Set(state.RedactStore) + + log.Set(state.Logger) + stereoscope.SetLogger(state.Logger) + return nil + }, + ). + WithPostRuns(func(state *clio.State, err error) { + stereoscope.Cleanup() + }) + return clioCfg +} diff --git a/cmd/syft/internal/commands/attest.go b/cmd/syft/internal/commands/attest.go index 0d9bbdc59..e349a1cff 100644 --- a/cmd/syft/internal/commands/attest.go +++ b/cmd/syft/internal/commands/attest.go @@ -42,7 +42,7 @@ type attestOptions struct { options.Output `yaml:",inline" mapstructure:",squash"` options.UpdateCheck `yaml:",inline" mapstructure:",squash"` options.Catalog `yaml:",inline" mapstructure:",squash"` - options.Attest `yaml:",inline" mapstructure:",squash"` + Attest options.Attest `yaml:"attest" mapstructure:"attest"` } func Attest(app clio.Application) *cobra.Command { diff --git a/cmd/syft/internal/commands/attest_test.go b/cmd/syft/internal/commands/attest_test.go index c090c861b..41a268226 100644 --- a/cmd/syft/internal/commands/attest_test.go +++ b/cmd/syft/internal/commands/attest_test.go @@ -4,16 +4,21 @@ import ( "bytes" "context" "fmt" + "io" "os/exec" "regexp" "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/scylladb/go-set/strset" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/anchore/clio" + "github.com/anchore/clio/cliotestutils" + "github.com/anchore/syft/cmd/syft/internal" "github.com/anchore/syft/cmd/syft/internal/options" "github.com/anchore/syft/syft/sbom" "github.com/anchore/syft/syft/source" @@ -267,3 +272,60 @@ func Test_buildSBOMForAttestation(t *testing.T) { }) } } + +func Test_attestCLIWiring(t *testing.T) { + id := clio.Identification{ + Name: "syft", + Version: "testing", + } + cfg := internal.AppClioSetupConfig(id, io.Discard) + tests := []struct { + name string + assertionFunc func(*testing.T, *cobra.Command, []string, ...any) + wantOpts attestOptions + args []string + env map[string]string + }{ + { + name: "key flag is accepted", + args: []string{"some-image:some-tag", "--key", "some-cosign-key.key"}, + assertionFunc: hasAttestOpts(options.Attest{Key: "some-cosign-key.key"}), + }, + { + name: "key password is read from env", + args: []string{"some-image:some-tag", "--key", "cosign.key"}, + env: map[string]string{ + "SYFT_ATTEST_PASSWORD": "some-password", + }, + assertionFunc: hasAttestOpts(options.Attest{ + Key: "cosign.key", + Password: "some-password", + }), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + t.Setenv(k, v) + } + } + app := cliotestutils.NewApplication(t, cfg, tt.assertionFunc) + cmd := Attest(app) + cmd.SetArgs(tt.args) + err := cmd.Execute() + assert.NoError(t, err) + }) + } +} + +func hasAttestOpts(wantOpts options.Attest) cliotestutils.AssertionFunc { + return func(t *testing.T, _ *cobra.Command, _ []string, cfgs ...any) { + assert.Equal(t, len(cfgs), 1) + attestOpts, ok := cfgs[0].(*attestOptions) + require.True(t, ok) + if d := cmp.Diff(wantOpts, attestOpts.Attest); d != "" { + t.Errorf("mismatched attest options (-want +got):\n%s", d) + } + } +} diff --git a/cmd/syft/internal/commands/commands_test.go b/cmd/syft/internal/commands/commands_test.go new file mode 100644 index 000000000..4ffc937d8 --- /dev/null +++ b/cmd/syft/internal/commands/commands_test.go @@ -0,0 +1,20 @@ +package commands + +import ( + "os" + "testing" + + gologgerredact "github.com/anchore/go-logger/adapter/redact" + "github.com/anchore/syft/internal/redact" +) + +func TestMain(m *testing.M) { + // Initialize global state needed to test clio/cobra commands directly + // Should be kept minimal. + + // Initialize redact store once for all tests in the commands package + // Redact store must be wired up here because syft will panic unless + // a redact store is wired up exactly once + redact.Set(gologgerredact.NewStore()) + os.Exit(m.Run()) +} diff --git a/cmd/syft/internal/options/attest.go b/cmd/syft/internal/options/attest.go index 8c0420386..7507ae36e 100644 --- a/cmd/syft/internal/options/attest.go +++ b/cmd/syft/internal/options/attest.go @@ -4,6 +4,8 @@ import ( "github.com/anchore/clio" ) +var _ clio.FlagAdder = (*Attest)(nil) + type Attest struct { // IMPORTANT: do not show the attestation key/password in any YAML/JSON output (sensitive information) Key secret `yaml:"key" json:"key" mapstructure:"key"` @@ -12,6 +14,6 @@ type Attest struct { var _ clio.FlagAdder = (*Attest)(nil) -func (o Attest) AddFlags(flags clio.FlagSet) { +func (o *Attest) AddFlags(flags clio.FlagSet) { flags.StringVarP((*string)(&o.Key), "key", "k", "the key to use for the attestation") } diff --git a/go.mod b/go.mod index 64fe66334..eafb06f99 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d github.com/acobaugh/osrelease v0.1.0 github.com/anchore/bubbly v0.0.0-20231115134915-def0aba654a9 - github.com/anchore/clio v0.0.0-20231016125544-c98a83e1c7fc + github.com/anchore/clio v0.0.0-20240131202212-9eba61247448 github.com/anchore/fangs v0.0.0-20231201140849-5075d28d6d8b github.com/anchore/go-logger v0.0.0-20230725134548-c21dafa1ec5a github.com/anchore/go-macholibre v0.0.0-20220308212642-53e6d0aaf6fb diff --git a/go.sum b/go.sum index d50c31f40..0788680ae 100644 --- a/go.sum +++ b/go.sum @@ -93,8 +93,8 @@ github.com/anchore/archiver/v3 v3.5.2 h1:Bjemm2NzuRhmHy3m0lRe5tNoClB9A4zYyDV58Pa github.com/anchore/archiver/v3 v3.5.2/go.mod h1:e3dqJ7H78uzsRSEACH1joayhuSyhnonssnDhppzS1L4= github.com/anchore/bubbly v0.0.0-20231115134915-def0aba654a9 h1:p0ZIe0htYOX284Y4axJaGBvXHU0VCCzLN5Wf5XbKStU= github.com/anchore/bubbly v0.0.0-20231115134915-def0aba654a9/go.mod h1:3ZsFB9tzW3vl4gEiUeuSOMDnwroWxIxJelOOHUp8dSw= -github.com/anchore/clio v0.0.0-20231016125544-c98a83e1c7fc h1:A1KFO+zZZmbNlz1+WKsCF0RKVx6XRoxsAG3lrqH9hUQ= -github.com/anchore/clio v0.0.0-20231016125544-c98a83e1c7fc/go.mod h1:QeWvNzxsrUNxcs6haQo3OtISfXUXW0qAuiG4EQiz0GU= +github.com/anchore/clio v0.0.0-20240131202212-9eba61247448 h1:ZgecmkxhH5im+9jPs7Ra1Thmv/p4IBDsoCFD6W8pENg= +github.com/anchore/clio v0.0.0-20240131202212-9eba61247448/go.mod h1:t5Mld8naKcG8RTPjW/2n7bfyBKFl1A6PvtXw+v64gK0= github.com/anchore/fangs v0.0.0-20231201140849-5075d28d6d8b h1:L/djgY7ZbZ/38+wUtdkk398W3PIBJLkt1N8nU/7e47A= github.com/anchore/fangs v0.0.0-20231201140849-5075d28d6d8b/go.mod h1:TLcE0RE5+8oIx2/NPWem/dq1DeaMoC+fPEH7hoSzPLo= github.com/anchore/go-logger v0.0.0-20230725134548-c21dafa1ec5a h1:nJ2G8zWKASyVClGVgG7sfM5mwoZlZ2zYpIzN2OhjWkw= diff --git a/test/cli/scan_cmd_test.go b/test/cli/scan_cmd_test.go index b3d3e3b4b..1615ae517 100644 --- a/test/cli/scan_cmd_test.go +++ b/test/cli/scan_cmd_test.go @@ -191,6 +191,7 @@ func TestPackagesCmdFlags(t *testing.T) { }, }, { + // TODO: this could be a unit test name: "responds-to-package-cataloger-search-options", args: []string{"--help"}, env: map[string]string{