Skip to content

Commit d57c19b

Browse files
orto17eyalk007
authored andcommitted
Added hash part to aggregated branch name (jfrog#970)
1 parent a557f59 commit d57c19b

File tree

4 files changed

+31
-15
lines changed

4 files changed

+31
-15
lines changed

scanrepository/scanrepository.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,10 @@ func (cfp *ScanRepositoryCmd) fixMultiplePackages(fullProjectPath string, vulner
330330
// Otherwise, it performs a force push to the same branch and reopens the pull request if it was closed.
331331
// Only one aggregated pull request should remain open at all times.
332332
func (cfp *ScanRepositoryCmd) fixIssuesSinglePR(repository *utils.Repository, vulnerabilitiesMap map[string]map[string]*utils.VulnerabilityDetails) (err error) {
333-
aggregatedFixBranchName := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech)
333+
aggregatedFixBranchName, err := cfp.gitManager.GenerateAggregatedFixBranchName(cfp.scanDetails.BaseBranch(), cfp.projectTech)
334+
if err != nil {
335+
return
336+
}
334337
existingPullRequestDetails, err := cfp.getOpenPullRequestBySourceBranch(aggregatedFixBranchName)
335338
if err != nil {
336339
return

scanrepository/scanrepository_test.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,27 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
100100
}{
101101
{
102102
testName: "aggregate",
103-
expectedPackagesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"uuid", "minimist", "mpath"}},
104-
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"^1.2.6", "^9.0.0", "^0.8.4"}},
103+
expectedPackagesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"uuid", "minimist", "mpath"}},
104+
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"^1.2.6", "^9.0.0", "^0.8.4"}},
105105
packageDescriptorPaths: []string{"package.json"},
106106
aggregateFixes: true,
107107
},
108108
{
109109
testName: "aggregate-multi-dir",
110-
expectedPackagesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"uuid", "minimatch", "mpath", "minimist"}},
111-
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"^1.2.6", "^9.0.0", "^0.8.4", "^3.0.5"}},
112-
expectedMissingFilesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"npm1/package-lock.json", "npm2/package-lock.json"}},
110+
expectedPackagesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"uuid", "minimatch", "mpath", "minimist"}},
111+
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"^1.2.6", "^9.0.0", "^0.8.4", "^3.0.5"}},
112+
expectedMissingFilesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"npm1/package-lock.json", "npm2/package-lock.json"}},
113113
packageDescriptorPaths: []string{"npm1/package.json", "npm2/package.json"},
114114
aggregateFixes: true,
115115
},
116+
{
117+
testName: "aggregate-multi-project",
118+
expectedPackagesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"uuid", "minimatch", "mpath"}, "frogbot-update-e8fa179873704bb1362147aff9c40040-dependencies-master": {"pyjwt", "pexpect"}},
119+
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"^9.0.0", "^0.8.4", "^3.0.5"}, "frogbot-update-e8fa179873704bb1362147aff9c40040-dependencies-master": {"2.4.0"}},
120+
expectedMissingFilesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"npm/package-lock.json"}},
121+
packageDescriptorPaths: []string{"npm/package.json", "pip/requirements.txt"},
122+
aggregateFixes: true,
123+
},
116124
{
117125
testName: "aggregate-no-vul",
118126
// No branch is being created because there are no vulnerabilities.
@@ -140,8 +148,8 @@ func TestScanRepositoryCmd_Run(t *testing.T) {
140148
{
141149
// This testcase checks the partial results feature. It simulates a failure in the dependency tree construction in the test's project inner module
142150
testName: "partial-results-enabled",
143-
expectedPackagesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"minimist", "mpath"}},
144-
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-npm-dependencies-master": {"1.2.6", "0.8.4"}},
151+
expectedPackagesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"minimist", "mpath"}},
152+
expectedVersionUpdatesInBranch: map[string][]string{"frogbot-update-68d9dee2475e5986e783d85dfa11baa0-dependencies-master": {"1.2.6", "0.8.4"}},
145153
packageDescriptorPaths: []string{"package.json", "inner-project/package.json"},
146154
aggregateFixes: true,
147155
allowPartialResults: true,

utils/git.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,12 +496,16 @@ func (gm *GitManager) getPullRequestTitleTemplate(tech []techutils.Technology) s
496496

497497
// GenerateAggregatedFixBranchName Generating a consistent branch name to enable branch updates
498498
// and to ensure that there is only one Frogbot aggregate pull request from each base branch scanned.
499-
func (gm *GitManager) GenerateAggregatedFixBranchName(baseBranch string, tech []techutils.Technology) (fixBranchName string) {
499+
func (gm *GitManager) GenerateAggregatedFixBranchName(baseBranch string, tech []techutils.Technology) (fixBranchName string, err error) {
500500
branchFormat := gm.customTemplates.branchNameTemplate
501501
if branchFormat == "" {
502502
branchFormat = AggregatedBranchNameTemplate
503503
}
504-
return formatStringWithPlaceHolders(branchFormat, "", "", techArrayToString(tech, fixBranchTechSeparator), baseBranch, false)
504+
hash, err := Md5Hash("frogbot", baseBranch, techArrayToString(tech, fixBranchTechSeparator))
505+
if err != nil {
506+
return "", err
507+
}
508+
return formatStringWithPlaceHolders(branchFormat, techArrayToString(tech, fixBranchTechSeparator), "", hash, baseBranch, false), nil
505509
}
506510

507511
// dryRunClone clones an existing repository from our testdata folder into the destination folder for testing purposes.

utils/git_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,31 +149,32 @@ func TestGitManager_GenerateAggregatedFixBranchName(t *testing.T) {
149149
desc string
150150
}{
151151
{
152-
expected: "frogbot-update-Go-dependencies-main",
152+
expected: "frogbot-update-e4e1fa318f12b3bed84b13ae5c293108-dependencies-main",
153153
baseBranch: "main",
154154
desc: "No template",
155155
gitManager: GitManager{},
156156
}, {
157-
expected: "frogbot-update-Go-dependencies-v2",
157+
expected: "frogbot-update-144734671657efb7f0d252bd99ca25d8-dependencies-v2",
158158
baseBranch: "v2",
159159
desc: "No template",
160160
gitManager: GitManager{},
161161
},
162162
{
163-
expected: "[feature]-Go-main",
163+
expected: "[feature]-e4e1fa318f12b3bed84b13ae5c293108-main",
164164
baseBranch: "main",
165165
desc: "Custom template hash only",
166166
gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "[feature]-${BRANCH_NAME_HASH}"}},
167167
}, {
168-
expected: "[feature]-Go-master",
168+
expected: "[feature]-697bdb58caaed95527fc709da59ca47f-master",
169169
baseBranch: "master",
170170
desc: "Custom template hash only",
171171
gitManager: GitManager{customTemplates: CustomTemplates{branchNameTemplate: "[feature]-${BRANCH_NAME_HASH}"}},
172172
},
173173
}
174174
for _, test := range testCases {
175175
t.Run(test.desc, func(t *testing.T) {
176-
titleOutput := test.gitManager.GenerateAggregatedFixBranchName(test.baseBranch, []techutils.Technology{techutils.Go})
176+
titleOutput, err := test.gitManager.GenerateAggregatedFixBranchName(test.baseBranch, []techutils.Technology{techutils.Go})
177+
assert.NoError(t, err)
177178
assert.Equal(t, test.expected, titleOutput)
178179
})
179180
}

0 commit comments

Comments
 (0)