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

✨ Gitlab: Add projects to cron #2936

Merged
merged 19 commits into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 6 additions & 21 deletions checker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,37 +57,22 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
_, experimental := os.LookupEnv("SCORECARD_EXPERIMENTAL")
var repoClient clients.RepoClient

//nolint:nestif
if experimental && glrepo.DetectGitLab(repoURI) {
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
if experimental {
repo, makeRepoError = glrepo.MakeGitlabRepo(repoURI)
if makeRepoError != nil {
return repo,
nil,
nil,
nil,
nil,
fmt.Errorf("getting local directory client: %w", makeRepoError)
if repo != nil && makeRepoError == nil {
repoClient, makeRepoError = glrepo.CreateGitlabClient(ctx, repo.Host())
}
}

var err error
repoClient, err = glrepo.CreateGitlabClientWithToken(ctx, os.Getenv("GITLAB_AUTH_TOKEN"), repo)
if err != nil {
return repo,
nil,
nil,
nil,
nil,
fmt.Errorf("error creating gitlab client: %w", err)
}
} else {
if makeRepoError != nil || repo == nil {
repo, makeRepoError = ghrepo.MakeGithubRepo(repoURI)
if makeRepoError != nil {
return repo,
nil,
nil,
nil,
nil,
fmt.Errorf("getting local directory client: %w", makeRepoError)
fmt.Errorf("error making github repo: %w", makeRepoError)
}
repoClient = ghrepo.CreateGithubRepoClient(ctx, logger)
}
Expand Down
21 changes: 8 additions & 13 deletions clients/gitlabrepo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"log"
"os"
"time"

"github.com/xanzy/go-gitlab"
Expand Down Expand Up @@ -250,8 +251,13 @@ func (client *Client) Close() error {
return nil
}

func CreateGitlabClientWithToken(ctx context.Context, token string, repo clients.Repo) (clients.RepoClient, error) {
client, err := gitlab.NewClient(token, gitlab.WithBaseURL(repo.Host()))
func CreateGitlabClient(ctx context.Context, host string) (clients.RepoClient, error) {
token := os.Getenv("GITLAB_AUTH_TOKEN")
return CreateGitlabClientWithToken(ctx, token, host)
}

func CreateGitlabClientWithToken(ctx context.Context, token, host string) (clients.RepoClient, error) {
client, err := gitlab.NewClient(token, gitlab.WithBaseURL(host))
if err != nil {
return nil, fmt.Errorf("could not create gitlab client with error: %w", err)
}
Expand Down Expand Up @@ -308,14 +314,3 @@ func CreateGitlabClientWithToken(ctx context.Context, token string, repo clients
func CreateOssFuzzRepoClient(ctx context.Context, logger *log.Logger) (clients.RepoClient, error) {
return nil, fmt.Errorf("%w, oss fuzz currently only supported for github repos", clients.ErrUnsupportedFeature)
}

// DetectGitLab: check whether the repoURI is a GitLab URI
// Makes HTTP request to GitLab API.
func DetectGitLab(repoURI string) bool {
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
var repo repoURL
if err := repo.parse(repoURI); err != nil {
return false
}

return repo.IsValid() == nil
}
2 changes: 1 addition & 1 deletion clients/gitlabrepo/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func Test_InitRepo(t *testing.T) {
t.Error("couldn't make gitlab repo", err)
}

client, err := CreateGitlabClientWithToken(context.Background(), "", repo)
client, err := CreateGitlabClient(context.Background(), repo.Host())
if err != nil {
t.Error("couldn't make gitlab client", err)
}
Expand Down
2 changes: 1 addition & 1 deletion clients/gitlabrepo/commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func Test_Setup(t *testing.T) {
t.Error("couldn't make gitlab repo", err)
}

client, err := CreateGitlabClientWithToken(context.Background(), "", repo)
client, err := CreateGitlabClient(context.Background(), repo.Host())
if err != nil {
t.Error("couldn't make gitlab client", err)
}
Expand Down
2 changes: 1 addition & 1 deletion clients/gitlabrepo/contributors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func Test_ContributorsSetup(t *testing.T) {
t.Error("couldn't make gitlab repo", err)
}

client, err := CreateGitlabClientWithToken(context.Background(), "", repo)
client, err := CreateGitlabClientWithToken(context.Background(), "", repo.Host())
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
t.Error("couldn't make gitlab client", err)
}
Expand Down
9 changes: 8 additions & 1 deletion clients/gitlabrepo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package gitlabrepo

import (
"errors"
"fmt"
"net/url"
"strings"
Expand All @@ -38,6 +39,8 @@ type repoURL struct {
metadata []string
}

var errInvalidGitlabRepoURL = errors.New("repo is not a gitlab repo")

// Parses input string into repoURL struct
/*
* Accepted input string formats are as follows:
Expand Down Expand Up @@ -98,6 +101,10 @@ func (r *repoURL) IsValid() error {
return nil
}

if strings.EqualFold(r.host, "github.com") {
return fmt.Errorf("%w: %s", errInvalidGitlabRepoURL, r.host)
}

client, err := gitlab.NewClient("", gitlab.WithBaseURL(fmt.Sprintf("%s://%s", r.scheme, r.host)))
if err != nil {
return sce.WithMessage(err,
Expand Down Expand Up @@ -141,7 +148,7 @@ func MakeGitlabRepo(input string) (clients.Repo, error) {
return nil, fmt.Errorf("error during parse: %w", err)
}
if err := repo.IsValid(); err != nil {
return nil, fmt.Errorf("error n IsValid: %w", err)
return nil, fmt.Errorf("error in IsValid: %w", err)
}
return &repo, nil
}
12 changes: 8 additions & 4 deletions clients/gitlabrepo/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func TestRepoURL_IsValid(t *testing.T) {
}
}

func TestRepoURL_DetectGitlab(t *testing.T) {
func TestRepoURL_MakeGitLabRepo(t *testing.T) {
tests := []struct {
repouri string
expected bool
Expand Down Expand Up @@ -157,9 +157,13 @@ func TestRepoURL_DetectGitlab(t *testing.T) {
if tt.flagRequired && os.Getenv("TEST_GITLAB_EXTERNAL") == "" {
continue
}
g := DetectGitLab(tt.repouri)
if g != tt.expected {
t.Errorf("got %s isgitlab: %t expected %t", tt.repouri, g, tt.expected)
g, err := MakeGitlabRepo(tt.repouri)
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
if (g != nil) != (err == nil) {
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
t.Errorf("got gitlabrepo: %s with err %s", g, err)
}
isGitlab := g != nil && err == nil
if isGitlab != tt.expected {
t.Errorf("got %s isgitlab: %t expected %t", tt.repouri, isGitlab, tt.expected)
}
}
}
2 changes: 1 addition & 1 deletion cron/config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ additional-params:
# TODO(#859): Re-add Contributors after fixing inconsistencies.
# TODO: Dependency-Update-Tool and SAST are search heavy
# TODO: Vulnerabilities is slow on repos with lots of dependencies
blacklisted-checks: CI-Tests,Contributors,Dependency-Update-Tool
blacklisted-checks: CI-Tests,Contributors,Dependency-Update-Tool,Webhooks
cii-data-bucket-url: gs://ossf-scorecard-cii-data
# Raw results.
raw-bigquery-table: scorecard-rawdata
Expand Down
2 changes: 1 addition & 1 deletion cron/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const (
prodCompletionThreshold = 0.99
prodWebhookURL = ""
prodCIIDataBucket = "gs://ossf-scorecard-cii-data"
prodBlacklistedChecks = "CI-Tests,Contributors,Dependency-Update-Tool"
prodBlacklistedChecks = "CI-Tests,Contributors,Dependency-Update-Tool,Webhooks"
prodShardSize int = 10
prodMetricExporter string = "stackdriver"
prodMetricStackdriverPrefix string = "scorecard-cron"
Expand Down
12 changes: 9 additions & 3 deletions cron/data/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/jszwec/csvutil"

"github.com/ossf/scorecard/v4/clients/githubrepo"
"github.com/ossf/scorecard/v4/clients/gitlabrepo"
)

// Iterator interface is used to iterate through list of input repos for the cron job.
Expand Down Expand Up @@ -83,9 +84,14 @@ func (reader *csvIterator) Next() (RepoFormat, error) {
if reader.err != nil {
return reader.next, fmt.Errorf("reader has error: %w", reader.err)
}
// Sanity check valid GitHub URL.
if _, err := githubrepo.MakeGithubRepo(reader.next.Repo); err != nil {
return reader.next, fmt.Errorf("invalid GitHub URL: %w", err)

repoURI := reader.next.Repo

// validate gitlab or github url
if _, err := gitlabrepo.MakeGitlabRepo(repoURI); err != nil {
if _, err := githubrepo.MakeGithubRepo(repoURI); err != nil {
return reader.next, fmt.Errorf("invalid URL, neither github nor gitlab: %w", err)
}
raghavkaul marked this conversation as resolved.
Show resolved Hide resolved
}
return reader.next, nil
}
Expand Down
59 changes: 58 additions & 1 deletion cron/data/iterator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,63 @@ func TestCsvIterator(t *testing.T) {
},
},
},
{
name: "BasicGitlabOnly",
filename: "testdata/basic-gitlab-only.csv",
outcomes: []outcome{
{
hasError: false,
repo: RepoFormat{
Repo: "gitlab.com/owner1/repo1",
},
},
{
hasError: false,
repo: RepoFormat{
Repo: "gitlab.com/owner3/path1/repo2",
Metadata: []string{"meta"},
},
},
},
},
{
name: "BasicWithGitlab",
filename: "testdata/basic-with-gitlab.csv",
outcomes: []outcome{
{
hasError: false,
repo: RepoFormat{
Repo: "github.com/owner1/repo1",
},
},
{
hasError: false,
repo: RepoFormat{
Repo: "github.com/owner2/repo2",
},
},
{
hasError: false,
repo: RepoFormat{
Repo: "github.com/owner3/repo3",
Metadata: []string{"meta"},
},
},
{
hasError: false,
repo: RepoFormat{
Repo: "gitlab.com/owner1/repo1",
},
},
{
hasError: false,
repo: RepoFormat{
Repo: "gitlab.com/owner3/path1/repo2",
Metadata: []string{"meta"},
},
},
},
},
{
name: "Comment",
filename: "testdata/comment.csv",
Expand Down Expand Up @@ -95,7 +152,7 @@ func TestCsvIterator(t *testing.T) {
outcomes: []outcome{
{
hasError: true,
expectedErr: sce.ErrorUnsupportedHost,
expectedErr: sce.ErrorInvalidURL,
},
{
hasError: true,
Expand Down
3 changes: 3 additions & 0 deletions cron/data/testdata/basic-gitlab-only.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
repo,metadata
gitlab.com/owner1/repo1,
gitlab.com/owner3/path1/repo2,meta
6 changes: 6 additions & 0 deletions cron/data/testdata/basic-with-gitlab.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
repo,metadata
github.com/owner1/repo1,
github.com/owner2/repo2,
github.com/owner3/repo3,meta
gitlab.com/owner1/repo1,
gitlab.com/owner3/path1/repo2,meta
2 changes: 1 addition & 1 deletion cron/data/testdata/failing_urls.csv
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
repo,metadata
gitlab.com/owner1/repo1,
gitlab.com//repo1,
github.com/owner2/,
github.com//repo3,meta
10 changes: 10 additions & 0 deletions cron/data/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,26 @@ func TestCsvWriter(t *testing.T) {
Repo: "github.com/owner1/repo1",
Metadata: []string{"meta1"},
},
{
Repo: "gitlab.com/owner3/repo3",
Metadata: []string{"meta3"},
},
},
newRepos: []RepoFormat{
{
Repo: "github.com/owner2/repo2",
Metadata: []string{"meta2"},
},
{
Repo: "gitlab.com/owner4/repo4",
Metadata: []string{"meta4"},
},
},
out: `repo,metadata
github.com/owner1/repo1,meta1
github.com/owner2/repo2,meta2
gitlab.com/owner3/repo3,meta3
gitlab.com/owner4/repo4,meta4
`,
},
}
Expand Down
Loading