Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ ProjectPackageClient: pass client to RunScorecard #4096

Merged
merged 2 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion attestor/command/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyRes
}
}

repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, err := checker.GetClients(
repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients(
ctx, repoURL, "", logger)
if err != nil {
return policy.Fail, fmt.Errorf("couldn't set up clients: %w", err)
Expand Down Expand Up @@ -117,6 +117,7 @@ func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyRes
ossFuzzRepoClient,
ciiClient,
vulnsClient,
projectClient,
)
if err != nil {
return policy.Fail, fmt.Errorf("RunScorecard: %w", err)
Expand Down
2 changes: 2 additions & 0 deletions checker/check_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"

"github.com/ossf/scorecard/v5/clients"
"github.com/ossf/scorecard/v5/internal/packageclient"
)

// CheckRequest struct encapsulates all data to be passed into a CheckFn.
Expand All @@ -29,6 +30,7 @@ type CheckRequest struct {
Dlogger DetailLogger
Repo clients.Repo
VulnerabilitiesClient clients.VulnerabilitiesClient
ProjectClient packageclient.ProjectPackageClient
// UPGRADEv6: return raw results instead of scores.
RawResults *RawResults
RequiredTypes []RequestType
Expand Down
5 changes: 5 additions & 0 deletions checker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
glrepo "github.com/ossf/scorecard/v5/clients/gitlabrepo"
"github.com/ossf/scorecard/v5/clients/localdir"
"github.com/ossf/scorecard/v5/clients/ossfuzz"
"github.com/ossf/scorecard/v5/internal/packageclient"
"github.com/ossf/scorecard/v5/log"
)

Expand All @@ -34,6 +35,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
clients.RepoClient, // ossFuzzClient
clients.CIIBestPracticesClient, // ciiClient
clients.VulnerabilitiesClient, // vulnClient
packageclient.ProjectPackageClient, // projectClient
error,
) {
var repo clients.Repo
Expand All @@ -50,6 +52,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
nil, /*ossFuzzClient*/
nil, /*ciiClient*/
clients.DefaultVulnerabilitiesClient(), /*vulnClient*/
nil,
retErr
}

Expand All @@ -68,6 +71,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
nil,
nil,
nil,
packageclient.CreateDepsDevClient(),
fmt.Errorf("error making github repo: %w", makeRepoError)
}
repoClient = ghrepo.CreateGithubRepoClient(ctx, logger)
Expand All @@ -78,5 +82,6 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
ossfuzz.CreateOSSFuzzClient(ossfuzz.StatusURL), /*ossFuzzClient*/
clients.DefaultCIIBestPracticesClient(), /*ciiClient*/
clients.DefaultVulnerabilitiesClient(), /*vulnClient*/
packageclient.CreateDepsDevClient(),
nil
}
27 changes: 16 additions & 11 deletions checker/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/ossf/scorecard/v5/log"
)

//nolint:gocognit
func TestGetClients(t *testing.T) {
type args struct { //nolint:govet
ctx context.Context
Expand All @@ -28,16 +29,17 @@ func TestGetClients(t *testing.T) {
logger *log.Logger
}
tests := []struct { //nolint:govet
name string
args args
shouldOSSFuzzBeNil bool
shouldRepoClientBeNil bool
shouldVulnClientBeNil bool
shouldRepoBeNil bool
shouldCIIBeNil bool
wantErr bool
experimental bool
isGhHost bool
name string
args args
shouldOSSFuzzBeNil bool
shouldRepoClientBeNil bool
shouldVulnClientBeNil bool
shouldRepoBeNil bool
shouldCIIBeNil bool
shouldProjectClientBeNil bool
wantErr bool
experimental bool
isGhHost bool
}{
{
name: "localURI is not empty",
Expand Down Expand Up @@ -105,7 +107,7 @@ func TestGetClients(t *testing.T) {
t.Setenv("GH_HOST", "github.corp.com")
t.Setenv("GH_TOKEN", "PAT")
}
got, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := GetClients(tt.args.ctx, tt.args.repoURI, tt.args.localURI, tt.args.logger)
got, repoClient, ossFuzzClient, ciiClient, vulnsClient, projectClient, err := GetClients(tt.args.ctx, tt.args.repoURI, tt.args.localURI, tt.args.logger)
if (err != nil) != tt.wantErr {
t.Fatalf("GetClients() error = %v, wantErr %v", err, tt.wantErr)
}
Expand All @@ -124,6 +126,9 @@ func TestGetClients(t *testing.T) {
if vulnsClient != nil && tt.shouldVulnClientBeNil {
t.Errorf("GetClients() vulnsClient = %v", vulnsClient)
}
if projectClient != nil && tt.shouldProjectClientBeNil {
t.Errorf("GetClients() projectClient = %v", projectClient)
}
})
}
}
6 changes: 5 additions & 1 deletion cmd/internal/scdiff/app/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/ossf/scorecard/v5/clients/gitlabrepo"
"github.com/ossf/scorecard/v5/clients/ossfuzz"
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/internal/packageclient"
"github.com/ossf/scorecard/v5/log"
"github.com/ossf/scorecard/v5/pkg"
)
Expand All @@ -45,6 +46,7 @@ type Runner struct {
ossFuzz clients.RepoClient
cii clients.CIIBestPracticesClient
vuln clients.VulnerabilitiesClient
deps packageclient.ProjectPackageClient
}

// Creates a Runner which will run the listed checks. If no checks are provided, all will run.
Expand Down Expand Up @@ -79,7 +81,9 @@ func (r *Runner) Run(repoURI string) (pkg.ScorecardResult, error) {
if err != nil {
return pkg.ScorecardResult{}, err
}
return pkg.RunScorecard(r.ctx, repo, commit, commitDepth, r.enabledChecks, repoClient, r.ossFuzz, r.cii, r.vuln)
return pkg.RunScorecard(
r.ctx, repo, commit, commitDepth, r.enabledChecks, repoClient, r.ossFuzz, r.cii, r.vuln, r.deps,
)
}

// logs only if logger is set.
Expand Down
3 changes: 2 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func rootCmd(o *options.Options) error {

ctx := context.Background()
logger := sclog.NewLogger(sclog.ParseLevel(o.LogLevel))
repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, err := checker.GetClients(
repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients(
ctx, o.Repo, o.Local, logger) // MODIFIED
if err != nil {
return fmt.Errorf("GetClients: %w", err)
Expand Down Expand Up @@ -142,6 +142,7 @@ func rootCmd(o *options.Options) error {
ossFuzzRepoClient,
ciiClient,
vulnsClient,
projectClient,
)
if err != nil {
return fmt.Errorf("RunScorecard: %w", err)
Expand Down
4 changes: 3 additions & 1 deletion cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/ossf/scorecard/v5/clients"
"github.com/ossf/scorecard/v5/clients/githubrepo"
"github.com/ossf/scorecard/v5/clients/ossfuzz"
"github.com/ossf/scorecard/v5/internal/packageclient"
"github.com/ossf/scorecard/v5/log"
"github.com/ossf/scorecard/v5/options"
"github.com/ossf/scorecard/v5/pkg"
Expand Down Expand Up @@ -69,10 +70,11 @@ func serveCmd(o *options.Options) *cobra.Command {
}
defer ossFuzzRepoClient.Close()
ciiClient := clients.DefaultCIIBestPracticesClient()
projectClient := packageclient.CreateDepsDevClient()
checksToRun := checks.GetAll()
repoResult, err := pkg.RunScorecard(
ctx, repo, clients.HeadSHA /*commitSHA*/, o.CommitDepth, checksToRun, repoClient,
ossFuzzRepoClient, ciiClient, vulnsClient)
ossFuzzRepoClient, ciiClient, vulnsClient, projectClient)
if err != nil {
logger.Error(err, "running enabled scorecard checks on repo")
rw.WriteHeader(http.StatusInternalServerError)
Expand Down
8 changes: 6 additions & 2 deletions cron/internal/worker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/ossf/scorecard/v5/cron/worker"
docs "github.com/ossf/scorecard/v5/docs/checks"
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/internal/packageclient"
"github.com/ossf/scorecard/v5/log"
"github.com/ossf/scorecard/v5/pkg"
"github.com/ossf/scorecard/v5/policy"
Expand Down Expand Up @@ -89,6 +90,7 @@ type ScorecardWorker struct {
ciiClient clients.CIIBestPracticesClient
ossFuzzRepoClient clients.RepoClient
vulnsClient clients.VulnerabilitiesClient
projectClient packageclient.ProjectPackageClient
apiBucketURL string
rawBucketURL string
blacklistedChecks []string
Expand Down Expand Up @@ -156,7 +158,8 @@ func (sw *ScorecardWorker) Close() {

func (sw *ScorecardWorker) Process(ctx context.Context, req *data.ScorecardBatchRequest, bucketURL string) error {
return processRequest(ctx, req, sw.blacklistedChecks, bucketURL, sw.rawBucketURL, sw.apiBucketURL,
sw.checkDocs, sw.githubClient, sw.gitlabClient, sw.ossFuzzRepoClient, sw.ciiClient, sw.vulnsClient, sw.logger)
sw.checkDocs, sw.githubClient, sw.gitlabClient, sw.ossFuzzRepoClient, sw.ciiClient,
sw.vulnsClient, sw.projectClient, sw.logger)
}

func (sw *ScorecardWorker) PostProcess() {
Expand All @@ -171,6 +174,7 @@ func processRequest(ctx context.Context,
githubClient, gitlabClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient,
projectClient packageclient.ProjectPackageClient,
logger *log.Logger,
) error {
filename := worker.ResultFilename(batchRequest)
Expand Down Expand Up @@ -210,7 +214,7 @@ func processRequest(ctx context.Context,
}

result, err := pkg.RunScorecard(ctx, repo, commitSHA, 0, checksToRun,
repoClient, ossFuzzRepoClient, ciiClient, vulnsClient)
repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient)
if errors.Is(err, sce.ErrRepoUnreachable) {
// Not accessible repo - continue.
continue
Expand Down
7 changes: 6 additions & 1 deletion dependencydiff/dependencydiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/ossf/scorecard/v5/checks"
"github.com/ossf/scorecard/v5/clients"
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/internal/packageclient"
sclog "github.com/ossf/scorecard/v5/log"
"github.com/ossf/scorecard/v5/pkg"
"github.com/ossf/scorecard/v5/policy"
Expand All @@ -44,6 +45,7 @@ type dependencydiffContext struct {
ossFuzzClient clients.RepoClient
vulnsClient clients.VulnerabilitiesClient
ciiClient clients.CIIBestPracticesClient
projectClient packageclient.ProjectPackageClient
changeTypesToCheck []string
checkNamesToRun []string
dependencydiffs []dependency
Expand Down Expand Up @@ -95,7 +97,7 @@ func GetDependencyDiffResults(
}

func initRepoAndClientByChecks(dCtx *dependencydiffContext, dSrcRepo string) error {
repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := checker.GetClients(
repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients(
dCtx.ctx, dSrcRepo, "", dCtx.logger)
if err != nil {
return fmt.Errorf("error getting the github repo and clients: %w", err)
Expand All @@ -115,6 +117,8 @@ func initRepoAndClientByChecks(dCtx *dependencydiffContext, dSrcRepo string) err
dCtx.ciiClient = ciiClient
case checks.CheckVulnerabilities:
dCtx.vulnsClient = vulnsClient
case checks.CheckSignedReleases:
dCtx.projectClient = projectClient
}
}
return nil
Expand Down Expand Up @@ -171,6 +175,7 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) error {
dCtx.ossFuzzClient,
dCtx.ciiClient,
dCtx.vulnsClient,
dCtx.projectClient,
)
// If the run fails, we leave the current dependency scorecard result empty and record the error
// rather than letting the entire API return nil since we still expect results for other dependencies.
Expand Down
4 changes: 2 additions & 2 deletions e2e/attestor_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,11 @@ func getScorecardResult(repoURL string) (pkg.ScorecardResult, error) {
Fn: checks.PinningDependencies,
},
}
repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, err := checker.GetClients(
repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients(
ctx, repoURL, "", logger)
if err != nil {
return pkg.ScorecardResult{}, fmt.Errorf("couldn't set up clients: %w", err)
}

return pkg.RunScorecard(ctx, repo, clients.HeadSHA, 0, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient)
return pkg.RunScorecard(ctx, repo, clients.HeadSHA, 0, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient)
}
6 changes: 6 additions & 0 deletions pkg/scorecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/ossf/scorecard/v5/config"
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/finding"
"github.com/ossf/scorecard/v5/internal/packageclient"
proberegistration "github.com/ossf/scorecard/v5/internal/probes"
sclog "github.com/ossf/scorecard/v5/log"
"github.com/ossf/scorecard/v5/options"
Expand Down Expand Up @@ -91,6 +92,7 @@ func runScorecard(ctx context.Context,
ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient,
projectClient packageclient.ProjectPackageClient,
) (ScorecardResult, error) {
if err := repoClient.InitRepo(repo, commitSHA, commitDepth); err != nil {
// No need to call sce.WithMessage() since InitRepo will do that for us.
Expand Down Expand Up @@ -232,6 +234,7 @@ func RunScorecard(ctx context.Context,
ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient,
projectClient packageclient.ProjectPackageClient,
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
) (ScorecardResult, error) {
return runScorecard(ctx,
repo,
Expand All @@ -243,6 +246,7 @@ func RunScorecard(ctx context.Context,
ossFuzzRepoClient,
ciiClient,
vulnsClient,
projectClient,
)
}

Expand All @@ -257,6 +261,7 @@ func ExperimentalRunProbes(ctx context.Context,
ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient,
projectClient packageclient.ProjectPackageClient,
) (ScorecardResult, error) {
return runScorecard(ctx,
repo,
Expand All @@ -268,5 +273,6 @@ func ExperimentalRunProbes(ctx context.Context,
ossFuzzRepoClient,
ciiClient,
vulnsClient,
projectClient,
)
}
8 changes: 4 additions & 4 deletions pkg/scorecard_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,20 @@ var _ = Describe("E2E TEST: RunScorecard with re-used repoClient", func() {

isolatedLogger := sclog.NewLogger(sclog.DebugLevel)
lastRepo := repos[len(repos)-1]
repo, rc, ofrc, cc, vc, err := checker.GetClients(ctx, lastRepo, "", isolatedLogger)
repo, rc, ofrc, cc, vc, dc, err := checker.GetClients(ctx, lastRepo, "", isolatedLogger)
Expect(err).Should(BeNil())
isolatedResult, err := RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc, ofrc, cc, vc)
isolatedResult, err := RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc, ofrc, cc, vc, dc)
Expect(err).Should(BeNil())

logger := sclog.NewLogger(sclog.DebugLevel)
_, rc2, ofrc2, cc2, vc2, err := checker.GetClients(ctx, repos[0], "", logger)
_, rc2, ofrc2, cc2, vc2, dc2, err := checker.GetClients(ctx, repos[0], "", logger)
Expect(err).Should(BeNil())

var sharedResult ScorecardResult
for i := range repos {
repo, err = githubrepo.MakeGithubRepo(repos[i])
Expect(err).Should(BeNil())
sharedResult, err = RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc2, ofrc2, cc2, vc2)
sharedResult, err = RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc2, ofrc2, cc2, vc2, dc2)
Expect(err).Should(BeNil())
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/scorecard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func TestRunScorecard(t *testing.T) {
}, nil
})
defer ctrl.Finish()
got, err := RunScorecard(context.Background(), repo, tt.args.commitSHA, 0, nil, mockRepoClient, nil, nil, nil)
got, err := RunScorecard(context.Background(), repo, tt.args.commitSHA, 0, nil, mockRepoClient, nil, nil, nil, nil)
if (err != nil) != tt.wantErr {
t.Errorf("RunScorecard() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -315,7 +315,9 @@ func TestExperimentalRunProbes(t *testing.T) {
mockRepoClient,
nil,
nil,
nil)
nil,
nil,
)
if (err != nil) != tt.wantErr {
t.Errorf("RunScorecard() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down