✨ Gitlab: Add projects to cron (ossf#2936)

* cron: add gitlab projects * support gitlab client * simplify gitlab detection Signed-off-by: Raghav Kaul <[email protected]> * fix MakeGitlabRepo * shortcut when repo url is github.com * fixes add-projects, validate-projects Signed-off-by: Raghav Kaul <[email protected]> * Move gitlab repos to release controller Signed-off-by: Raghav Kaul <[email protected]> * Add csv headers Signed-off-by: Raghav Kaul <[email protected]> * Use gitlab.WithBaseURL Signed-off-by: Raghav Kaul <[email protected]> * formatting & logging Signed-off-by: Raghav Kaul <[email protected]> * remove spurious test Signed-off-by: Raghav Kaul <[email protected]> * consolidate logic Signed-off-by: Raghav Kaul <[email protected]> * Turn on experimental flag Signed-off-by: Raghav Kaul <[email protected]> * Add projects Signed-off-by: Raghav Kaul <[email protected]> * Update client Signed-off-by: Raghav Kaul <[email protected]> * update Signed-off-by: Raghav Kaul <[email protected]> * update Signed-off-by: Raghav Kaul <[email protected]> * update Signed-off-by: Raghav Kaul <[email protected]> * update Signed-off-by: Raghav Kaul <[email protected]> --------- Signed-off-by: Raghav Kaul <[email protected]> Signed-off-by: Avishay <[email protected]>
balteravishay · Jun 11, 2023 · 11086a5 · 11086a5
1 parent 4887d43
commit 11086a5
Show file tree

Hide file tree

Showing 26 changed files with 441 additions and 72 deletions.
diff --git a/checker/client.go b/checker/client.go
@@ -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) {
+	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)
 	}

diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go
@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"os"
 	"time"
 
 	"github.com/xanzy/go-gitlab"
@@ -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)
 	}
@@ -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 {
-	var repo repoURL
-	if err := repo.parse(repoURI); err != nil {
-		return false
-	}
-
-	return repo.IsValid() == nil
-}
diff --git a/clients/gitlabrepo/client_test.go b/clients/gitlabrepo/client_test.go
@@ -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)
 			}

diff --git a/clients/gitlabrepo/commits_test.go b/clients/gitlabrepo/commits_test.go
@@ -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)
 			}

diff --git a/clients/gitlabrepo/contributors_test.go b/clients/gitlabrepo/contributors_test.go
@@ -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())
 			if err != nil {
 				t.Error("couldn't make gitlab client", err)
 			}

diff --git a/clients/gitlabrepo/repo.go b/clients/gitlabrepo/repo.go
@@ -17,6 +17,7 @@
 package gitlabrepo
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@@ -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:
@@ -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,
@@ -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
 }
diff --git a/clients/gitlabrepo/repo_test.go b/clients/gitlabrepo/repo_test.go
@@ -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
@@ -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)
+		if (g != nil) != (err == nil) {
+			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)
 		}
 	}
 }
diff --git a/cron/config/config.yaml b/cron/config/config.yaml
@@ -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

diff --git a/cron/config/config_test.go b/cron/config/config_test.go
@@ -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"

diff --git a/cron/data/iterator.go b/cron/data/iterator.go
@@ -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.
@@ -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)
+		}
 	}
 	return reader.next, nil
 }

diff --git a/cron/data/iterator_test.go b/cron/data/iterator_test.go
@@ -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",
@@ -95,7 +152,7 @@ func TestCsvIterator(t *testing.T) {
 			outcomes: []outcome{
 				{
 					hasError:    true,
-					expectedErr: sce.ErrorUnsupportedHost,
+					expectedErr: sce.ErrorInvalidURL,
 				},
 				{
 					hasError:    true,

diff --git a/cron/data/testdata/basic-gitlab-only.csv b/cron/data/testdata/basic-gitlab-only.csv
@@ -0,0 +1,3 @@
+repo,metadata
+gitlab.com/owner1/repo1,
+gitlab.com/owner3/path1/repo2,meta
diff --git a/cron/data/testdata/basic-with-gitlab.csv b/cron/data/testdata/basic-with-gitlab.csv
@@ -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
diff --git a/cron/data/testdata/failing_urls.csv b/cron/data/testdata/failing_urls.csv
@@ -1,4 +1,4 @@
 repo,metadata
-gitlab.com/owner1/repo1,
+gitlab.com//repo1,
 github.com/owner2/,
 github.com//repo3,meta
diff --git a/cron/data/writer_test.go b/cron/data/writer_test.go
@@ -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
 `,
 		},
 	}