Skip to content

Commit

Permalink
fixed linter errors
Browse files Browse the repository at this point in the history
Signed-off-by: N8BWert <[email protected]>
Signed-off-by: nathaniel.wert <[email protected]>
  • Loading branch information
nathaniel.wert committed Nov 28, 2022
1 parent 249b65f commit b976af7
Show file tree
Hide file tree
Showing 16 changed files with 57 additions and 73 deletions.
4 changes: 2 additions & 2 deletions checks/raw/security_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
if strings.Contains(c.Repo.Org().String(), "gitlab.") {
client, err = gitlabrepo.CreateGitlabClientWithToken(c.Ctx, os.Getenv("GITLAB_AUTH_TOKEN"), c.Repo)
if err != nil {
return checker.SecurityPolicyData{}, err
return checker.SecurityPolicyData{}, fmt.Errorf("unable to create gitlab client: %w", err)
}
err = client.InitRepo(c.Repo, clients.HeadSHA)
} else {
Expand All @@ -72,7 +72,7 @@ func SecurityPolicy(c *checker.CheckRequest) (checker.SecurityPolicyData, error)
data.uri = client.URI()
err = fileparser.OnAllFilesDo(client, isSecurityPolicyFile, &data)
if err != nil {
return checker.SecurityPolicyData{}, err
return checker.SecurityPolicyData{}, fmt.Errorf("unable to create github client: %w", err)
}

case errors.Is(err, sce.ErrRepoUnreachable):
Expand Down
22 changes: 12 additions & 10 deletions clients/gitlabrepo/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package gitlabrepo

import (
"errors"
"fmt"
"strings"
"sync"
Expand All @@ -25,6 +26,8 @@ import (
"github.com/ossf/scorecard/v4/clients"
)

var errNoBranchFound = errors.New("branch given in ref could not be found")

type branchesHandler struct {
glClient *gitlab.Client
once *sync.Once
Expand Down Expand Up @@ -137,7 +140,7 @@ func (handler *branchesHandler) getBranch(commitOrBranch string) (*clients.Branc
beforeTime := commit.CreatedAt.Add(-1 * time.Minute)
afterTime := commit.CreatedAt.Add(1 * time.Minute)
var branchName string
for _, name := range handler.branchNames {
for i, name := range handler.branchNames {
// The main branch seems to always be involved in release branches so we will disregard it here.
if strings.EqualFold(name, "main") {
continue
Expand All @@ -147,20 +150,18 @@ func (handler *branchesHandler) getBranch(commitOrBranch string) (*clients.Branc
// branch of the commit seems to be to query each branch until we find a commit with the same CreatedAt time.
possibleCommits, resp, err := handler.glClient.Commits.ListCommits(handler.repourl.projectID,
&gitlab.ListCommitsOptions{
RefName: &name,
RefName: &handler.branchNames[i],
Since: &beforeTime,
Until: &afterTime,
})
if err != nil && resp.StatusCode != 404 {
return nil, fmt.Errorf("error finding possible list of commits to find release branch: %w", err)
}

if possibleCommits != nil {
for _, com := range possibleCommits {
if com.ID == commitOrBranch {
branchName = name
break
}
for _, com := range possibleCommits {
if com.ID == commitOrBranch {
branchName = name
break
}
}

Expand All @@ -170,7 +171,7 @@ func (handler *branchesHandler) getBranch(commitOrBranch string) (*clients.Branc
}

if branchName == "" {
return nil, fmt.Errorf("branch was not available from given committish")
return nil, errNoBranchFound
}

bran, _, err = handler.glClient.Branches.GetBranch(handler.repourl.projectID, branchName)
Expand All @@ -187,7 +188,8 @@ func (handler *branchesHandler) getBranch(commitOrBranch string) (*clients.Branc

projectStatusChecks, resp, err := handler.glClient.ExternalStatusChecks.ListProjectStatusChecks(
handler.repourl.projectID, &gitlab.ListOptions{})
// Project Status Checks are only allowed for GitLab ultimate members so we will assume them to be null if user does not have permissions.
// Project Status Checks are only allowed for GitLab ultimate members so we will assume they are
// null if user does not have permissions.
if err != nil && resp.StatusCode != 404 && resp.StatusCode != 401 {
return nil, fmt.Errorf("request for external status checks failed with error %w", err)
}
Expand Down
48 changes: 17 additions & 31 deletions clients/gitlabrepo/commits.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,39 +127,25 @@ func (handler *commitsHandler) setup() error {
})
}

// append the commits to the handler.
commit := clients.Commit{
CommittedDate: *commit.CommittedDate,
Message: commit.Message,
SHA: commit.ID,
AssociatedMergeRequest: clients.PullRequest{
Number: mergeRequest.ID,
MergedAt: *mergeRequest.MergedAt,
HeadSHA: mergeRequest.SHA,
Author: clients.User{ID: int64(mergeRequest.Author.ID)},
Labels: labels,
Reviews: reviews,
},
}

if user != nil {
handler.commits = append(handler.commits,
clients.Commit{
CommittedDate: *commit.CommittedDate,
Message: commit.Message,
SHA: commit.ID,
AssociatedMergeRequest: clients.PullRequest{
Number: mergeRequest.ID,
MergedAt: *mergeRequest.MergedAt,
HeadSHA: mergeRequest.SHA,
Author: clients.User{ID: int64(mergeRequest.Author.ID)},
Labels: labels,
Reviews: reviews,
},
Committer: clients.User{ID: int64(user.ID)},
})
} else {
handler.commits = append(handler.commits,
clients.Commit{
CommittedDate: *commit.CommittedDate,
Message: commit.Message,
SHA: commit.ID,
AssociatedMergeRequest: clients.PullRequest{
Number: mergeRequest.ID,
MergedAt: *mergeRequest.MergedAt,
HeadSHA: mergeRequest.SHA,
Author: clients.User{ID: int64(mergeRequest.Author.ID)},
Labels: labels,
Reviews: reviews,
},
})
commit.Committer = clients.User{ID: int64(user.ID)}
}

handler.commits = append(handler.commits, commit)
}
})

Expand Down
2 changes: 1 addition & 1 deletion clients/gitlabrepo/contributors.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (handler *contributorsHandler) setup() error {

// TODO: Handle many users of same name

if users == nil || len(users) == 0 {
if len(users) == 0 {
continue
}

Expand Down
7 changes: 4 additions & 3 deletions clients/gitlabrepo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ type repoURL struct {
* "https://gitlab.<companyDomain:string>.com/<owner:string>/<projectID:int>"
*/
func (r *repoURL) parse(input string) error {
if strings.Contains(input, "https://") {
switch {
case strings.Contains(input, "https://"):
input = strings.TrimPrefix(input, "https://")
} else if strings.Contains(input, "http://") {
case strings.Contains(input, "http://"):
input = strings.TrimPrefix(input, "http://")
} else if strings.Contains(input, "://") {
case strings.Contains(input, "://"):
return sce.WithMessage(sce.ErrScorecardInternal, "unknown input format")
}

Expand Down
2 changes: 1 addition & 1 deletion clients/gitlabrepo/tarball.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (handler *tarballHandler) getTarball() error {
return nil
}

// nolint: gocognit
//nolint: gocognit
func (handler *tarballHandler) extractTarball() error {
in, err := os.OpenFile(handler.tempTarFile, os.O_RDONLY, 0o644)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion clients/gitlabrepo/tarball_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func setup(inputFile string) (tarballHandler, error) {
return tarballHandler, nil
}

// nolint: gocognit
//nolint: gocognit
func TestExtractTarball(t *testing.T) {
t.Parallel()
testcases := []struct {
Expand Down
3 changes: 1 addition & 2 deletions e2e/binary_artifacts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package e2e

import (
"context"
"io/ioutil"
"os"

"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -174,7 +173,7 @@ var _ = Describe("E2E TEST:"+checks.CheckBinaryArtifacts, func() {
})
It("Should return binary artifacts present at commit in source code when using local repoClient", func() {
// create temp dir
tmpDir, err := ioutil.TempDir("", "")
tmpDir, err := os.MkdirTemp("", "")
Expect(err).Should(BeNil())
defer os.RemoveAll(tmpDir)

Expand Down
3 changes: 0 additions & 3 deletions e2e/branch_protection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
Expect(scut.ValidateTestReturn(nil, "branch protection accessible", &expected, &result, &dl)).Should(BeTrue())
Expect(repoClient.Close()).Should(BeNil())
})
// GitLab values are slightly different as some features are only available to certain tiers, however the below test should
// work as an e2e test
It("Should get non-admin branch protection on other repositories - GitLab", func() {
skipIfTokenIsNot(patTokenType, "GitLab pac only")

Expand Down Expand Up @@ -198,7 +196,6 @@ var _ = Describe("E2E TEST PAT:"+checks.CheckBranchProtection, func() {
Repo: repo,
Dlogger: &dl,
}
// Due to some slight differences in base configs and allowed free user changes these values are correct for the repository given.
expected := scut.TestReturn{
Error: nil,
Score: 2,
Expand Down
8 changes: 4 additions & 4 deletions e2e/code_review_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
Expect(scut.ValidateTestReturn(nil, "use code reviews", &expected, &result, &dl)).Should(BeTrue())
Expect(repoClient.Close()).Should(BeNil())
})
// GitLab doesn't seem to preserve merge requests (pull requests in github) and some users are messed up in the transfer so this
// returns a different value than the above GitHub test.
// GitLab doesn't seem to preserve merge requests (pull requests in github) and some users had data lost in
// the transfer from github so this returns a different value than the above GitHub test.
It("Should return use of code reviews - GitLab", func() {
dl := scut.TestDetailLogger{}
// Project url is gitlab.com/N8BWert/airflow.
Expand Down Expand Up @@ -112,8 +112,8 @@ var _ = Describe("E2E TEST:"+checks.CheckCodeReview, func() {
Expect(scut.ValidateTestReturn(nil, "use code reviews", &expected, &result, &dl)).Should(BeTrue())
Expect(repoClient.Close()).Should(BeNil())
})
// GitLab doesn't seem to preserve merge requests (pull requests in github) and some users are messed up in the transfer so this
// returns a different value than the above GitHub test.
// GitLab doesn't seem to preserve merge requests (pull requests in github) and some users had data lost in
// the transfer from github so this returns a different value than the above GitHub test.
It("Should return use of code reviews at commit - GitLab", func() {
dl := scut.TestDetailLogger{}
// project url is gitlab.com/N8BWert/airflow.
Expand Down
9 changes: 5 additions & 4 deletions e2e/dangerous_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package e2e

import (
"context"
"io/ioutil"
"os"

"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -83,7 +82,7 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
It("Should return dangerous workflow for local repoClient", func() {
dl := scut.TestDetailLogger{}

tmpDir, err := ioutil.TempDir("", "")
tmpDir, err := os.MkdirTemp("", "")
Expect(err).Should(BeNil())
defer os.RemoveAll(tmpDir)

Expand Down Expand Up @@ -122,7 +121,8 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
// // TODO: Write a test repository with a dangerous workflow.
// repo, err := gitlabrepo.MakeGitlabRepo("gitlab.ossf.com/ossf-tests/scorecard-check-dangerous-workflow-e2e")
// Expect(err).Should(BeNil())
// repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(), os.Getenv("GITLAB_AUTH_TOKNE"), repo)
// repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(),
// os.Getenv("GITLAB_AUTH_TOKNE"), repo)
// Expect(err).Should(BeNil())
// err = repoClient.InitRepo(repo, clients.HeadSHA)
// Expect(err).Should(BeNil())
Expand All @@ -148,7 +148,8 @@ var _ = Describe("E2E TEST:"+checks.CheckTokenPermissions, func() {
// // TODO: Write a test repository with a dangerous workflow.
// repo, err := gitlabrepo.MakeGitlabRepo("gitlab.ossf.com/ossf-tests/scorecard-check-dangerous-workflow-e2e")
// Expect(err).Should(BeNil())
// repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(), os.Getenv("GITLAB_AUTH_TOKEN"), repo)
// repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(),
// os.Getenv("GITLAB_AUTH_TOKEN"), repo)
// Expect(err).Should(BeNil())
// err = repoClient.InitRepo(repo, "8db326e9ba20517feeefd157524a89184ed41f7f")
// Expect(err).Should(BeNil())
Expand Down
3 changes: 1 addition & 2 deletions e2e/license_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package e2e

import (
"context"
"io/ioutil"
"os"

"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -86,7 +85,7 @@ var _ = Describe("E2E TEST:"+checks.CheckLicense, func() {
It("Should return license check works for the local repoclient", func() {
dl := scut.TestDetailLogger{}

tmpDir, err := ioutil.TempDir("", "")
tmpDir, err := os.MkdirTemp("", "")
Expect(err).Should(BeNil())
defer os.RemoveAll(tmpDir)

Expand Down
6 changes: 4 additions & 2 deletions e2e/maintained_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,16 @@ var _ = Describe("E2E TEST:"+checks.CheckMaintained, func() {
Expect(repoClient.Close()).Should(BeNil())
})
// To check the below maintanace status, the person running the test must own the repository.
// Therefore, the below test works it just requires a maintained repository to exist under someone's personal account, which I do not have.
// Therefore, the below test works it just requires a maintained repository to exist under
// someone's personal account, which I do not have.

// It("Should return valid maintained status - GitLab", func() {
// dl := scut.TestDetailLogger{}
// // project url is gitlab.com/gitlab-org/gitlab
// repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/gitlab-org/278964")
// Expect(err).Should(BeNil())
// repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(), os.Getenv("GITLAB_AUTH_TOKEN"), repo)
// repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(),
// os.Getenv("GITLAB_AUTH_TOKEN"), repo)
// Expect(err).Should(BeNil())
// err = repoClient.InitRepo(repo, clients.HeadSHA)
// Expect(err).Should(BeNil())
Expand Down
3 changes: 1 addition & 2 deletions e2e/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package e2e

import (
"context"
"io/ioutil"
"os"

"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -88,7 +87,7 @@ var _ = Describe("E2E TEST:"+checks.CheckPinnedDependencies, func() {
It("Should return dependencies check for a local repoClient", func() {
dl := scut.TestDetailLogger{}

tmpDir, err := ioutil.TempDir("", "")
tmpDir, err := os.MkdirTemp("", "")
Expect(err).Should(BeNil())
defer os.RemoveAll(tmpDir)

Expand Down
4 changes: 1 addition & 3 deletions e2e/security_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ package e2e

import (
"context"
"io/ioutil"
"os"

"github.com/go-git/go-git/v5"
Expand Down Expand Up @@ -140,7 +139,7 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
It("Should return valid security policy for local repoClient at head", func() {
dl := scut.TestDetailLogger{}

tmpDir, err := ioutil.TempDir("", "")
tmpDir, err := os.MkdirTemp("", "")
Expect(err).Should(BeNil())
defer os.RemoveAll(tmpDir)

Expand Down Expand Up @@ -174,7 +173,6 @@ var _ = Describe("E2E TEST:"+checks.CheckSecurityPolicy, func() {
Expect(scut.ValidateTestReturn(nil, "policy found", &expected, &result, &dl)).Should(BeTrue())
Expect(x.Close()).Should(BeNil())
})
// TODO: find a smaller repository to find the security policy of (or at least a project that a tar file can be gotten of).
It("Should return valid security policy - GitLab", func() {
dl := scut.TestDetailLogger{}
// project url is gitlab.com/bramw/baserow.
Expand Down
4 changes: 2 additions & 2 deletions e2e/vulnerabilities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
Expect(repoClient.Close()).Should(BeNil())
})
It("Should return that there are vulnerabilities - GitLab", func() {
// project url is gitlab.com/N8BWert/scorecard-check-vulnerabilites-open62541.
// project url is gitlab.com/N8BWert/scorecard-check-vulnerabilities-open62541.
repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/39539557")
Expect(err).Should(BeNil())
repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(), os.Getenv("GITLAB_AUTH_TOKEN"), repo)
Expand Down Expand Up @@ -144,7 +144,7 @@ var _ = Describe("E2E TEST:"+checks.CheckVulnerabilities, func() {
Expect(repoClient.Close()).Should(BeNil())
})
It("Should return that there are vulnerabilities at commit - GitLab", func() {
// project url is gitlab.com/N8BWert/scorecard-check-vulnerabilites-open62541.
// project url is gitlab.com/N8BWert/scorecard-check-vulnerabilities-open62541.
repo, err := gitlabrepo.MakeGitlabRepo("gitlab.com/N8BWert/39539557")
Expect(err).Should(BeNil())
repoClient, err := gitlabrepo.CreateGitlabClientWithToken(context.Background(), os.Getenv("GITLAB_AUTH_TOKEN"), repo)
Expand Down

0 comments on commit b976af7

Please sign in to comment.