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.

Sign up for GitHub

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

🐛 Bug fixing: Using the wrong URI to initialize the repo in Dependencydiff #2072

Merged
merged 7 commits into from
Jul 19, 2022
Merged

🐛 Bug fixing: Using the wrong URI to initialize the repo in Dependencydiff #2072

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
771e3a4
temp
aidenwang9867 Jul 18, 2022
779674b
temp
aidenwang9867 Jul 18, 2022
82179ba
temp
aidenwang9867 Jul 18, 2022
3d00c1a
temp
aidenwang9867 Jul 18, 2022
ad88fe5
temp
aidenwang9867 Jul 18, 2022
51841f1
temp
aidenwang9867 Jul 19, 2022
1f8ca4f
temp
aidenwang9867 Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions dependencydiff/dependencydiff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ package dependencydiff
import (
"context"
"fmt"
"path"

"github.com/ossf/scorecard/v4/checker"
"github.com/ossf/scorecard/v4/checks"
"github.com/ossf/scorecard/v4/clients"
"github.com/ossf/scorecard/v4/clients/githubrepo"
sce "github.com/ossf/scorecard/v4/errors"
"github.com/ossf/scorecard/v4/log"
"github.com/ossf/scorecard/v4/pkg"
Expand Down Expand Up @@ -69,8 +67,6 @@ func GetDependencyDiffResults(
return nil, fmt.Errorf("error in fetchRawDependencyDiffData: %w", err)
}

// Initialize the repo and client(s) corresponding to the checks to run.
err = initRepoAndClientByChecks(&dCtx)
if err != nil {
return nil, fmt.Errorf("error in initRepoAndClientByChecks: %w", err)
}
Expand All @@ -81,21 +77,20 @@ func GetDependencyDiffResults(
return dCtx.results, nil
}

func initRepoAndClientByChecks(dCtx *dependencydiffContext) error {
func initRepoAndClientByChecks(dCtx *dependencydiffContext, dSrcRepo string) error {
repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := checker.GetClients(
dCtx.ctx, path.Join(dCtx.ownerName, dCtx.repoName), "", dCtx.logger,
dCtx.ctx, dSrcRepo, "", dCtx.logger,
)
if err != nil {
return fmt.Errorf("error creating the github repo: %w", err)
return fmt.Errorf("error getting the github repo and clients: %w", err)
}
// If the caller doesn't specify the checks to run, run all checks and return all the clients.
dCtx.ghRepo = repo
dCtx.ghRepoClient = repoClient
// If the caller doesn't specify the checks to run, run all the checks and return all the clients.
if dCtx.checkNamesToRun == nil || len(dCtx.checkNamesToRun) == 0 {
dCtx.ghRepo, dCtx.ghRepoClient, dCtx.ossFuzzClient, dCtx.ciiClient, dCtx.vulnsClient =
repo, repoClient, ossFuzzClient, ciiClient, vulnsClient
dCtx.ossFuzzClient, dCtx.ciiClient, dCtx.vulnsClient = ossFuzzClient, ciiClient, vulnsClient
return nil
}
dCtx.ghRepo = repo
dCtx.ghRepoClient = githubrepo.CreateGithubRepoClient(dCtx.ctx, dCtx.logger)
for _, cn := range dCtx.checkNamesToRun {
switch cn {
case checks.CheckFuzzing:
Expand Down Expand Up @@ -127,10 +122,16 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) error {
}
// For now we skip those without source repo urls.
// TODO (#2063): use the BigQuery dataset to supplement null source repo URLs to fetch the Scorecard results for them.
if d.SourceRepository != nil && *d.SourceRepository != "" {
if d.ChangeType != nil && (dCtx.changeTypesToCheck[*d.ChangeType] || dCtx.changeTypesToCheck == nil) {
if d.SourceRepository != nil && d.ChangeType != nil {
// Run the checks on all types if (1) the type is found in changeTypesToCheck or (2) no types are specified.
if dCtx.changeTypesToCheck[*d.ChangeType] || (dCtx.changeTypesToCheck == nil || len(dCtx.changeTypesToCheck) == 0) {
// Initialize the repo and client(s) corresponding to the checks to run.
err = initRepoAndClientByChecks(dCtx, *d.SourceRepository)
if err != nil {
return fmt.Errorf("error initializing repo and clients: %w", err)
}
// Run scorecard on those types of dependencies that the caller would like to check.
// If the input map changeTypesToCheck is empty, by default, we run checks for all valid types.
// If the input map changeTypesToCheck is empty, by default, we run the checks for all valid types.
// TODO (#2064): use the Scorecare REST API to retrieve the Scorecard result statelessly.
scorecardResult, err := pkg.RunScorecards(
dCtx.ctx,
Expand Down
34 changes: 14 additions & 20 deletions dependencydiff/dependencydiff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,22 +71,25 @@ func Test_initRepoAndClientByChecks(t *testing.T) {
t.Parallel()
//nolint
tests := []struct {
name string
dCtx dependencydiffContext
wantGhRepo, wantRepoClient, wantFuzzClient bool
wantVulnClient, wantCIIClient bool
wantErr bool
name string
dCtx dependencydiffContext
srcRepo string
wantRepoClient, wantFuzzClient bool
wantVulnClient, wantCIIClient bool
wantErr bool
}{
{
name: "error creating repo",
dCtx: dependencydiffContext{
logger: log.NewLogger(log.InfoLevel),
ctx: context.Background(),
ownerName: path.Join("host_not_exist.com", "owner_not_exist"),
repoName: "repo_not_exist",
checkNamesToRun: []string{},
},
wantGhRepo: false,
srcRepo: path.Join(
"host_not_exist.com",
"owner_not_exist",
"repo_not_exist",
),
wantRepoClient: false,
wantFuzzClient: false,
wantVulnClient: false,
Expand All @@ -99,13 +102,9 @@ func Test_initRepoAndClientByChecks(t *testing.T) {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := initRepoAndClientByChecks(&tt.dCtx)
err := initRepoAndClientByChecks(&tt.dCtx, tt.srcRepo)
if (err != nil) != tt.wantErr {
t.Errorf("initRepoAndClientByChecks() error = {%v}, want error: %v", err, tt.wantErr)
return
}
if (tt.dCtx.ghRepo != nil) != tt.wantGhRepo {
t.Errorf("init repo error, wantGhRepo: %v, got %v", tt.wantGhRepo, tt.dCtx.ghRepo)
t.Errorf("initClientByChecks() error = {%v}, want error: %v", err, tt.wantErr)
return
}
if (tt.dCtx.ghRepoClient != nil) != tt.wantRepoClient {
Expand Down Expand Up @@ -151,12 +150,7 @@ func Test_getScorecardCheckResults(t *testing.T) {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
err := initRepoAndClientByChecks(&tt.dCtx)
if err != nil {
t.Errorf("init repo and client error")
return
}
err = getScorecardCheckResults(&tt.dCtx)
err := getScorecardCheckResults(&tt.dCtx)
if (err != nil) != tt.wantErr {
t.Errorf("getScorecardCheckResults() error = {%v}, want error: %v", err, tt.wantErr)
return
Expand Down