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

Avoid unnecessary rescans #268

Merged
merged 16 commits into from
Apr 2, 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
69 changes: 67 additions & 2 deletions commands/scanandfixrepos.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package commands

import (
"context"
"errors"
"fmt"
"github.com/jfrog/frogbot/commands/utils"
"github.com/jfrog/froggit-go/vcsclient"
"github.com/jfrog/jfrog-client-go/utils/log"
"path/filepath"
"strings"
"time"
)

type ScanAndFixRepositories struct {
Expand All @@ -33,12 +36,23 @@ func (saf *ScanAndFixRepositories) Run(configAggregator utils.FrogbotConfigAggre

func (saf *ScanAndFixRepositories) scanAndFixSingleRepository(repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient) error {
for _, branch := range repoConfig.Branches {
err := saf.downloadAndRunScanAndFix(repoConfig, branch, client)
shouldScan, checkedCommit, err := saf.shouldScanLatestCommit(context.Background(), repoConfig, client, branch)
if err != nil {
return err
}
if !shouldScan {
log.Info(fmt.Sprintf("Commit '%s' in repo '%s', branch '%s' has already been scanned. Skipping the scan.", checkedCommit, repoConfig.RepoName, branch))
continue
}
if err = saf.downloadAndRunScanAndFix(repoConfig, branch, client); err != nil {
// Scan failed,mark commit status failed with error info
e := saf.setCommitBuildStatus(client, repoConfig, vcsclient.Fail, checkedCommit, fmt.Sprintf("Frogbot error: %s", err))
return errors.Join(err, e)
}
if err = saf.setCommitBuildStatus(client, repoConfig, vcsclient.Pass, checkedCommit, utils.CommitStatusDescription); err != nil {
return err
}
}

return nil
}

Expand Down Expand Up @@ -68,3 +82,54 @@ func (saf *ScanAndFixRepositories) downloadAndRunScanAndFix(repository *utils.Fr
cfp := CreateFixPullRequestsCmd{dryRun: saf.dryRun, dryRunRepoPath: filepath.Join(saf.dryRunRepoPath, repository.RepoName)}
return cfp.scanAndFixRepository(repository, branch, client)
}

func (saf ScanAndFixRepositories) setCommitBuildStatus(client vcsclient.VcsClient, repoConfig *utils.FrogbotRepoConfig, state vcsclient.CommitStatus, commitHash, description string) error {
if err := client.SetCommitStatus(context.Background(), state, repoConfig.RepoOwner, repoConfig.RepoName, commitHash, utils.FrogbotCreatorName, description, utils.CommitStatusDetailsUrl); err != nil {
return fmt.Errorf("failed to mark last commit as scanned due to: %s", err.Error())
}
log.Info("Commit '%s' in repo '%s', has successfully marked as scanned", commitHash, repoConfig.RepoName)
return nil
}

// Returns true if the latest commit hasn't been scanned
// or the time passed from the last scan exceeded the configured value.
func (saf ScanAndFixRepositories) shouldScanLatestCommit(ctx context.Context, repoConfig *utils.FrogbotRepoConfig, client vcsclient.VcsClient, branch string) (shouldScan bool, commitHash string, err error) {
owner := repoConfig.RepoOwner
repo := repoConfig.RepoName
latestCommit, err := client.GetLatestCommit(ctx, owner, repo, branch)
if err != nil {
return false, "", err
}
EyalDelarea marked this conversation as resolved.
Show resolved Hide resolved
ref := latestCommit.Hash
statuses, err := client.GetCommitStatuses(ctx, owner, repo, ref)
if err != nil {
return false, "", err
}
return shouldScanCommitByStatus(statuses), latestCommit.Hash, err
}

// Returns true if the latest commit status by Frogbot is not successful
// OR it's older than SkipRepoScanDays.
func shouldScanCommitByStatus(statuses []vcsclient.CommitStatusInfo) bool {
for _, status := range statuses {
if status.Creator == utils.FrogbotCreatorName && status.Description == utils.CommitStatusDescription {
return status.State != vcsclient.Pass || statusTimestampElapsed(status)
}
}
return true
}

// Checks if a commit status is older than SkipRepoScanDays number of days.
func statusTimestampElapsed(latestStatus vcsclient.CommitStatusInfo) bool {
if latestStatus.CreatedAt.IsZero() && latestStatus.LastUpdatedAt.IsZero() {
// In case non were initialized, address this as expired date
return true
}
statusLastUpdatedTime := latestStatus.LastUpdatedAt
if statusLastUpdatedTime.IsZero() {
// Fallback to creation time
statusLastUpdatedTime = latestStatus.CreatedAt
}
passDueDate := time.Now().UTC().AddDate(0, 0, -utils.SkipRepoScanDays)
return statusLastUpdatedTime.Before(passDueDate)
}
143 changes: 142 additions & 1 deletion commands/scanandfixrepos_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,135 @@ func TestScanAndFixRepos(t *testing.T) {
assert.NoError(t, cmd.Run(configAggregator, client))
}

func TestShouldScanBranchByStatus(t *testing.T) {
commitStatusTestCases := []struct {
statuses []vcsclient.CommitStatusInfo
description string
expected bool
}{
{
statuses: []vcsclient.CommitStatusInfo{},
description: "Empty statuses",
expected: true,
},
{
statuses: []vcsclient.CommitStatusInfo{
{
State: vcsclient.Fail,
Description: utils.CommitStatusDescription,
DetailsUrl: utils.CommitStatusDetailsUrl,
Creator: utils.FrogbotCreatorName,
LastUpdatedAt: time.Now().UTC(),
}, {
State: vcsclient.InProgress,
Description: utils.CommitStatusDescription,
DetailsUrl: "",
Creator: "im not frogbot",
LastUpdatedAt: time.Now().UTC(),
},
},
description: "Frogbot failed statues should scan",
expected: true,
},
{
statuses: []vcsclient.CommitStatusInfo{
{
State: vcsclient.Fail,
Description: "description",
DetailsUrl: "some other url",
Creator: "im not frogbot",
LastUpdatedAt: time.Now().UTC(),
}, {
State: vcsclient.InProgress,
Description: "this is the latest commit",
DetailsUrl: "some other url",
Creator: "im not frogbot",
LastUpdatedAt: time.Now().UTC(),
},
{
State: vcsclient.Pass,
Description: "this is the latest commit",
DetailsUrl: "some other url",
Creator: "im not frogbot",
LastUpdatedAt: time.Now().UTC(),
},
},
description: "Non Frogbot statues",
expected: true,
}, {
statuses: []vcsclient.CommitStatusInfo{
{
State: vcsclient.Pass,
Description: utils.CommitStatusDescription,
DetailsUrl: utils.CommitStatusDetailsUrl,
Creator: utils.FrogbotCreatorName,
LastUpdatedAt: time.Now().AddDate(0, -1, 0),
},
},
description: "Old statuse should scan",
expected: true,
},
}
for _, tt := range commitStatusTestCases {
t.Run(tt.description, func(t *testing.T) {
shouldScan := shouldScanCommitByStatus(tt.statuses)
assert.Equal(t, tt.expected, shouldScan)
})
}
}

func TestStatusTimestampElapsed(t *testing.T) {
testCases := []struct {
commitStatusInfo vcsclient.CommitStatusInfo
description string
expected bool
}{
{
commitStatusInfo: vcsclient.CommitStatusInfo{
State: 0,
Description: "",
DetailsUrl: "",
Creator: "",
CreatedAt: time.Now().UTC().AddDate(0, -3, 0),
LastUpdatedAt: time.Now().UTC().AddDate(0, 0, -utils.SkipRepoScanDays-1),
},
expected: true,
description: "Last Update time is priority",
},
{
commitStatusInfo: vcsclient.CommitStatusInfo{
State: 0,
Description: "",
DetailsUrl: "",
Creator: "",
CreatedAt: time.Now(),
LastUpdatedAt: time.Now(),
},
expected: false,
description: "No scan needed ",
},
{
commitStatusInfo: vcsclient.CommitStatusInfo{
State: 0,
Description: "",
DetailsUrl: "",
Creator: "",
CreatedAt: time.Now().UTC(),
LastUpdatedAt: time.Time{},
},
expected: false,
description: "Creation time fallback",
},
}

for _, tt := range testCases {
t.Run(tt.description, func(t *testing.T) {
needScan := statusTimestampElapsed(tt.commitStatusInfo)
assert.Equal(t, tt.expected, needScan)
})
}
}

func createReposGitEnvironment(t *testing.T, wd, port string, repositories ...string) {
for _, repository := range repositories {
fullWdPath := filepath.Join(wd, repository)
Expand Down Expand Up @@ -127,7 +256,19 @@ func createHttpHandler(t *testing.T, port *string, projectNames ...string) http.
return
}
if r.RequestURI == fmt.Sprintf("/repos/jfrog/%s/commits?page=1&per_page=1&sha=master", projectName) {
w.WriteHeader(http.StatusNotFound)
w.WriteHeader(http.StatusOK)
rawJson := "[\n {\n \"url\": \"https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e\",\n \"sha\": \"6dcb09b5b57875f334f61aebed695e2e4193db5e\",\n \"node_id\": \"MDY6Q29tbWl0NmRjYjA5YjViNTc4NzVmMzM0ZjYxYWViZWQ2OTVlMmU0MTkzZGI1ZQ==\",\n \"html_url\": \"https://github.com/octocat/Hello-World/commit/6dcb09b5b57875f334f61aebed695e2e4193db5e\",\n \"comments_url\": \"https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e/comments\",\n \"commit\": {\n \"url\": \"https://api.github.com/repos/octocat/Hello-World/git/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e\",\n \"author\": {\n \"name\": \"Monalisa Octocat\",\n \"email\": \"[email protected]\",\n \"date\": \"2011-04-14T16:00:49Z\"\n },\n \"committer\": {\n \"name\": \"Monalisa Octocat\",\n \"email\": \"[email protected]\",\n \"date\": \"2011-04-14T16:00:49Z\"\n },\n \"message\": \"Fix all the bugs\",\n \"tree\": {\n \"url\": \"https://api.github.com/repos/octocat/Hello-World/tree/6dcb09b5b57875f334f61aebed695e2e4193db5e\",\n \"sha\": \"6dcb09b5b57875f334f61aebed695e2e4193db5e\"\n },\n \"comment_count\": 0,\n \"verification\": {\n \"verified\": false,\n \"reason\": \"unsigned\",\n \"signature\": null,\n \"payload\": null\n }\n },\n \"author\": {\n \"login\": \"octocat\",\n \"id\": 1,\n \"node_id\": \"MDQ6VXNlcjE=\",\n \"avatar_url\": \"https://github.com/images/error/octocat_happy.gif\",\n \"gravatar_id\": \"\",\n \"url\": \"https://api.github.com/users/octocat\",\n \"html_url\": \"https://github.com/octocat\",\n \"followers_url\": \"https://api.github.com/users/octocat/followers\",\n \"following_url\": \"https://api.github.com/users/octocat/following{/other_user}\",\n \"gists_url\": \"https://api.github.com/users/octocat/gists{/gist_id}\",\n \"starred_url\": \"https://api.github.com/users/octocat/starred{/owner}{/repo}\",\n \"subscriptions_url\": \"https://api.github.com/users/octocat/subscriptions\",\n \"organizations_url\": \"https://api.github.com/users/octocat/orgs\",\n \"repos_url\": \"https://api.github.com/users/octocat/repos\",\n \"events_url\": \"https://api.github.com/users/octocat/events{/privacy}\",\n \"received_events_url\": \"https://api.github.com/users/octocat/received_events\",\n \"type\": \"User\",\n \"site_admin\": false\n },\n \"committer\": {\n \"login\": \"octocat\",\n \"id\": 1,\n \"node_id\": \"MDQ6VXNlcjE=\",\n \"avatar_url\": \"https://github.com/images/error/octocat_happy.gif\",\n \"gravatar_id\": \"\",\n \"url\": \"https://api.github.com/users/octocat\",\n \"html_url\": \"https://github.com/octocat\",\n \"followers_url\": \"https://api.github.com/users/octocat/followers\",\n \"following_url\": \"https://api.github.com/users/octocat/following{/other_user}\",\n \"gists_url\": \"https://api.github.com/users/octocat/gists{/gist_id}\",\n \"starred_url\": \"https://api.github.com/users/octocat/starred{/owner}{/repo}\",\n \"subscriptions_url\": \"https://api.github.com/users/octocat/subscriptions\",\n \"organizations_url\": \"https://api.github.com/users/octocat/orgs\",\n \"repos_url\": \"https://api.github.com/users/octocat/repos\",\n \"events_url\": \"https://api.github.com/users/octocat/events{/privacy}\",\n \"received_events_url\": \"https://api.github.com/users/octocat/received_events\",\n \"type\": \"User\",\n \"site_admin\": false\n },\n \"parents\": [\n {\n \"url\": \"https://api.github.com/repos/octocat/Hello-World/commits/6dcb09b5b57875f334f61aebed695e2e4193db5e\",\n \"sha\": \"6dcb09b5b57875f334f61aebed695e2e4193db5e\"\n }\n ]\n }\n]"
b := []byte(rawJson)
_, err := w.Write(b)
assert.NoError(t, err)
return
}
if r.RequestURI == fmt.Sprintf("/repos/jfrog/%v/code-scanning/sarifs", projectName) {
w.WriteHeader(http.StatusAccepted)
rawJson := "{\n \"id\": \"47177e22-5596-11eb-80a1-c1e54ef945c6\",\n \"url\": \"https://api.github.com/repos/octocat/hello-world/code-scanning/sarifs/47177e22-5596-11eb-80a1-c1e54ef945c6\"\n}"
b := []byte(rawJson)
_, err := w.Write(b)
assert.NoError(t, err)
return
}
}
Expand Down
9 changes: 9 additions & 0 deletions commands/utils/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,13 @@ const (

// The 'GITHUB_ACTIONS' environment variable exists when the CI is GitHub Actions
GitHubActionsEnv = "GITHUB_ACTIONS"

// When Frogbot periodically scans repositories, it skips scanning repositories for which the latest commit has already been scanned,
// unless the latest commit has been scanned more then 'SkipRepoScanDays' days ago.
SkipRepoScanDays = 4

// Used by Frogbot to create new commits statuses and recognize its own statuses.
CommitStatusDescription = "Scanned by Frogbot"
CommitStatusDetailsUrl = "https://github.com/jfrog/frogbot#readme"
FrogbotCreatorName = "Frogbot"
)
2 changes: 1 addition & 1 deletion docs/templates/jfrog-pipelines/pipelines-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pipelines:
image:
auto:
language: go
version: "1.19"
version: "1.20"
environmentVariables:

# [Mandatory]
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
module github.com/jfrog/frogbot

go 1.19
go 1.20

require (
github.com/go-git/go-git/v5 v5.6.0
github.com/golang/mock v1.6.0
github.com/jfrog/build-info-go v1.8.9
github.com/jfrog/froggit-go v1.6.3
github.com/jfrog/froggit-go v1.7.0
github.com/jfrog/gofrog v1.2.5
github.com/jfrog/jfrog-cli-core/v2 v2.29.9
github.com/jfrog/jfrog-client-go v1.26.5
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ github.com/jedib0t/go-pretty/v6 v6.4.6/go.mod h1:Ndk3ase2CkQbXLLNf5QDHoYb6J9WtVf
github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
github.com/jfrog/build-info-go v1.8.9 h1:2zShj2ATlNznyNd2F1R8vPJiYxnUEUoc98G/Chd5+S8=
github.com/jfrog/build-info-go v1.8.9/go.mod h1:dQ8OKddrbgtO3jK9uLYoqmRGNEjuDuNXV0bSRdpeTCI=
github.com/jfrog/froggit-go v1.6.3 h1:IExHc616HEQdk3Uk0rS3acbKJgJCrp9vYg5ZGim7C/o=
github.com/jfrog/froggit-go v1.6.3/go.mod h1:xfsfQXzSaAM04RV9IyU5heBiRrsm2oS6rFCfEofQr6U=
github.com/jfrog/froggit-go v1.7.0 h1:ID0X81Cp1JWYeTZ5Co2IxgFKFljqgAEXDqTFRPGjQR0=
github.com/jfrog/froggit-go v1.7.0/go.mod h1:xfsfQXzSaAM04RV9IyU5heBiRrsm2oS6rFCfEofQr6U=
github.com/jfrog/gofrog v1.2.5 h1:jCgJC0iGQ8bU7jCC+YEFJTNINyngApIrhd8BjZAVRIE=
github.com/jfrog/gofrog v1.2.5/go.mod h1:o00tSRff6IapTgaCMuX1Cs9MH08Y1JqnsKgRtx91Gc4=
github.com/jfrog/jfrog-cli-core/v2 v2.29.9 h1:Z04r+KkRDf+BqWnDX9vbiUeD/7nBzHQGYld2xjdjIgo=
Expand Down