Avoid unnecessary rescans (#268)

avoid unnecessary rescans by checking Frogbot's latest build commit status

commands/scanandfixrepos.go

@@ -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 {
@@ -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
 }
 
@@ -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
+	}
+	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)
+}

commands/scanandfixrepos_test.go

@@ -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)
@@ -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
 			}
 		}

commands/utils/consts.go

@@ -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"
 )

docs/templates/jfrog-pipelines/pipelines-go.yml

@@ -30,7 +30,7 @@ pipelines:
           image:
             auto:
               language: go
-              version: "1.19"
+              version: "1.20"
         environmentVariables:
 
           # [Mandatory]

go.mod

@@ -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

go.sum

@@ -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=