diff --git a/entrypoint/entrypoint.go b/entrypoint/entrypoint.go index ae768a52..4f5ddba5 100644 --- a/entrypoint/entrypoint.go +++ b/entrypoint/entrypoint.go @@ -29,7 +29,11 @@ import ( // New creates a new scorecard command which can be used as an entrypoint for // GitHub Actions. func New() (*cobra.Command, error) { - opts := options.New() + opts, err := options.New() + if err != nil { + return nil, fmt.Errorf("creating new options: %w", err) + } + if err := opts.Initialize(); err != nil { return nil, fmt.Errorf("initializing options: %w", err) } diff --git a/go.mod b/go.mod index fab839c7..b758c326 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.17 require ( github.com/caarlos0/env/v6 v6.9.1 + github.com/google/go-cmp v0.5.7 github.com/ossf/scorecard/v4 v4.1.1-0.20220303013834-3070b3ca1b59 github.com/spf13/cobra v1.3.0 ) @@ -23,7 +24,6 @@ require ( github.com/golang-jwt/jwt/v4 v4.0.0 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.2 // indirect - github.com/google/go-cmp v0.5.7 // indirect github.com/google/go-github/v38 v38.1.0 // indirect github.com/google/go-github/v41 v41.0.0 // indirect github.com/google/go-querystring v1.1.0 // indirect diff --git a/options/options.go b/options/options.go index d821ae9b..85b5558a 100644 --- a/options/options.go +++ b/options/options.go @@ -33,12 +33,13 @@ import ( var ( // Errors. errGithubEventPathEmpty = errors.New("GitHub event path is empty") + errResultsPathEmpty = errors.New("results path is empty") errOnlyDefaultBranchSupported = errors.New("only default branch is supported") trueStr = "true" ) -// Options TODO(lint): should have comment or be unexported (revive). +// Options are options for running scorecard via GitHub Actions. type Options struct { // Scorecard options. ScorecardOpts *options.Options @@ -46,19 +47,6 @@ type Options struct { // Scorecard command-line options. EnabledChecks string `env:"ENABLED_CHECKS"` - // Input options. - // TODO(options): These input options shadow the some of the SCORECARD_ - // env vars: - // export SCORECARD_RESULTS_FILE="$INPUT_RESULTS_FILE" - // export SCORECARD_RESULTS_FORMAT="$INPUT_RESULTS_FORMAT" - // export SCORECARD_PUBLISH_RESULTS="$INPUT_PUBLISH_RESULTS" - // - // Let's target them for deletion, but only after confirming - // that there isn't anything that surprisingly needs them. - InputResultsFile string `env:"INPUT_RESULTS_FILE"` - InputResultsFormat string `env:"INPUT_RESULTS_FORMAT"` - InputPublishResults string `env:"INPUT_PUBLISH_RESULTS"` - // Scorecard checks. EnableLicense string `env:"ENABLE_LICENSE"` EnableDangerousWorkflow string `env:"ENABLE_DANGEROUS_WORKFLOW"` @@ -84,56 +72,45 @@ const ( formatSarif = options.FormatSarif ) -// New TODO(lint): should have comment or be unexported (revive). -func New() *Options { - opts := &Options{} +// New creates a new options set for running scorecard via GitHub Actions. +func New() (*Options, error) { + // Enable scorecard command to use SARIF format. + os.Setenv(options.EnvVarEnableSarif, trueStr) + + opts := &Options{ + ScorecardOpts: options.New(), + } if err := env.Parse(opts); err != nil { - // TODO(options): Consider making this an error. - fmt.Printf("parsing entrypoint env vars: %+v", err) + return opts, fmt.Errorf("parsing entrypoint env vars: %w", err) } - // TODO(options): Push options into scorecard.Options once/if it supports - // validation. - scOpts := options.New() - if err := opts.Initialize(); err != nil { - // TODO(options): Consider making this an error. - fmt.Printf("initializing scorecard-action options: %+v\n", err) + return opts, fmt.Errorf( + "initializing scorecard-action options: %w", + err, + ) } // TODO(options): Move this set-or-default logic to its own function. - if opts.InputResultsFormat != "" { - scOpts.Format = opts.InputResultsFormat - } else { - scOpts.EnableSarif = true - scOpts.Format = formatSarif - os.Setenv(options.EnvVarEnableSarif, trueStr) - if scOpts.Format == "" { - // Default the scorecard command to using SARIF format. - if scOpts.PolicyFile == "" { - // TODO(policy): Should we default or error here? - scOpts.PolicyFile = defaultScorecardPolicyFile - } + opts.ScorecardOpts.EnableSarif = true + if opts.ScorecardOpts.Format == formatSarif { + if opts.ScorecardOpts.PolicyFile == "" { + // TODO(policy): Should we default or error here? + opts.ScorecardOpts.PolicyFile = defaultScorecardPolicyFile } } - if err := scOpts.Validate(); err != nil { - // TODO(options): Consider making this an error. - fmt.Printf("validating scorecard options: %+v\n", err) + if err := opts.ScorecardOpts.Validate(); err != nil { + return opts, fmt.Errorf("validating scorecard options: %w", err) } - opts.ScorecardOpts = scOpts opts.SetPublishResults() - - if opts.ScorecardOpts.PublishResults { - if scOpts.ResultsFile == "" { - scOpts.ResultsFile = opts.InputResultsFile - // TODO(options): We should check if this is empty. - } + if opts.ScorecardOpts.ResultsFile == "" { + return opts, errResultsPathEmpty } // TODO(options): Consider running Validate() before returning. - return opts + return opts, nil } // Initialize initializes the environment variables required for the action. @@ -147,8 +124,9 @@ func (o *Options) Initialize() error { GITHUB_ACTIONS is true in GitHub env. */ - o.EnableLicense = "1" - o.EnableDangerousWorkflow = "1" + // TODO(checks): Do we actually expect to use these? + // o.EnableLicense = "1" + // o.EnableDangerousWorkflow = "1" return o.SetRepoInfo() } @@ -197,23 +175,6 @@ func (o *Options) Print() { // SetPublishResults sets whether results should be published based on a // repository's visibility. func (o *Options) SetPublishResults() { - // Check INPUT_PUBLISH_RESULTS - var inputBool bool - var inputErr error - input := os.Getenv(EnvInputPublishResults) - if input != "" { - inputBool, inputErr = strconv.ParseBool(o.InputPublishResults) - if inputErr != nil { - // TODO(options): Consider making this an error. - fmt.Printf( - "could not parse a valid bool from %s (%s): %+v\n", - input, - EnvInputPublishResults, - inputErr, - ) - } - } - privateRepo, err := strconv.ParseBool(o.PrivateRepoStr) if err != nil { // TODO(options): Consider making this an error. @@ -226,8 +187,6 @@ func (o *Options) SetPublishResults() { if privateRepo { o.ScorecardOpts.PublishResults = false - } else { - o.ScorecardOpts.PublishResults = o.ScorecardOpts.PublishResults || inputBool } } diff --git a/options/options_test.go b/options/options_test.go index a531a7e8..f70ffd1e 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -16,13 +16,16 @@ package options import ( + "os" "testing" + "github.com/google/go-cmp/cmp" "github.com/ossf/scorecard/v4/options" ) const ( - testRepo = "good/repo" + testRepo = "good/repo" + testResultsFile = "results.sarif" githubEventPathNonFork = "testdata/non-fork.json" githubEventPathFork = "testdata/fork.json" @@ -31,13 +34,90 @@ const ( githubEventPathBadData = "testdata/bad-data.json" ) +func TestNew(t *testing.T) { + tests := []struct { + name string + githubEventPath string + repo string + resultsFile string + resultsFormat string + publishResults string + want options.Options + wantErr bool + }{ + { + name: "SuccessFormatSARIF", + githubEventPath: githubEventPathNonFork, + repo: testRepo, + resultsFormat: "sarif", + resultsFile: testResultsFile, + want: options.Options{ + Repo: testRepo, + EnableSarif: true, + Format: formatSarif, + PolicyFile: defaultScorecardPolicyFile, + ResultsFile: testResultsFile, + Commit: options.DefaultCommit, + LogLevel: options.DefaultLogLevel, + }, + wantErr: false, + }, + { + name: "SuccessFormatJSON", + githubEventPath: githubEventPathNonFork, + repo: testRepo, + resultsFormat: "json", + resultsFile: testResultsFile, + want: options.Options{ + Repo: testRepo, + EnableSarif: true, + Format: options.FormatJSON, + ResultsFile: testResultsFile, + Commit: options.DefaultCommit, + LogLevel: options.DefaultLogLevel, + }, + wantErr: false, + }, + /* + { + name: "FailureNoEnvVars", + want: options.Options{}, + wantErr: true, + }, + */ + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.githubEventPath != "" { + os.Setenv(EnvGithubEventPath, tt.githubEventPath) + } + if tt.repo != "" { + os.Setenv(EnvGithubRepository, tt.repo) + } + if tt.resultsFile != "" { + os.Setenv("SCORECARD_RESULTS_FILE", tt.resultsFile) + } + if tt.resultsFormat != "" { + os.Setenv("SCORECARD_RESULTS_FORMAT", tt.resultsFormat) + } + + opts, err := New() + got := *opts.ScorecardOpts + if (err != nil) != tt.wantErr { + t.Errorf("New() error = %+v, wantErr %+v", err, tt.wantErr) + return + } + if !cmp.Equal(tt.want, got) { + t.Errorf("New(): -want, +got:\n%s", cmp.Diff(tt.want, got)) + } + }) + } +} + func TestInitialize(t *testing.T) { type fields struct { ScorecardOpts *options.Options EnabledChecks string - InputResultsFile string - InputResultsFormat string - InputPublishResults string EnableLicense string EnableDangerousWorkflow string GithubEventName string @@ -85,9 +165,6 @@ func TestInitialize(t *testing.T) { o := &Options{ ScorecardOpts: tt.fields.ScorecardOpts, EnabledChecks: tt.fields.EnabledChecks, - InputResultsFile: tt.fields.InputResultsFile, - InputResultsFormat: tt.fields.InputResultsFormat, - InputPublishResults: tt.fields.InputPublishResults, EnableLicense: tt.fields.EnableLicense, EnableDangerousWorkflow: tt.fields.EnableDangerousWorkflow, GithubEventName: tt.fields.GithubEventName,