Skip to content

Commit

Permalink
Fix > 250 char Branch name issue (#188)
Browse files Browse the repository at this point in the history
* Fix > 250 char Branch name issue

Move brnach name generation to a function.
Limit original branch substing to 200 charts
Switch target paths substring to sha1 of the target paths.
Tests

* Address isssus raised in PR reivew

Rename a var
Used Repeat to avoid hardcoding long string
Made some test input **a bit** more realistic
  • Loading branch information
Oded-B authored Jun 12, 2024
1 parent 55bcc24 commit a3b2abe
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
23 changes: 22 additions & 1 deletion internal/pkg/githubapi/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package githubapi
import (
"bytes"
"context"
"crypto/sha1" //nolint:gosec // G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec), this is not a cryptographic use case
"encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
"net/http"
Expand Down Expand Up @@ -353,7 +355,7 @@ func handleMergedPrEvent(ghPrClientDetails GhPrClientDetails, prApproverGithubCl
return err
}

newBranchName := fmt.Sprintf("promotions/%v-%v-%v8", ghPrClientDetails.PrNumber, strings.Replace(ghPrClientDetails.Ref, "/", "-", -1), strings.Replace(strings.Join(promotion.Metadata.TargetPaths, "_"), "/", "-", -1)) // TODO max branch name length is 250 - make sure this fit
newBranchName := generateSafePromotionBranchName(ghPrClientDetails.PrNumber, ghPrClientDetails.Ref, promotion.Metadata.TargetPaths)

newBranchRef, err := createBranch(ghPrClientDetails, commit, newBranchName)
if err != nil {
Expand Down Expand Up @@ -414,6 +416,25 @@ func handleMergedPrEvent(ghPrClientDetails GhPrClientDetails, prApproverGithubCl
return nil
}

// Creating a unique branch name based on the PR number, PR ref and the promotion target paths
// Max length of branch name is 250 characters
func generateSafePromotionBranchName(prNumber int, originalBranchName string, targetPaths []string) string {
targetPathsBa := []byte(strings.Join(targetPaths, "_"))
hasher := sha1.New() //nolint:gosec // G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec), this is not a cryptographic use case
hasher.Write(targetPathsBa)
uniqBranchNameSuffix := firstN(hex.EncodeToString(hasher.Sum(nil)), 12)
safeOriginalBranchName := firstN(strings.Replace(originalBranchName, "/", "-", -1), 200)
return fmt.Sprintf("promotions/%v-%v-%v", prNumber, safeOriginalBranchName, uniqBranchNameSuffix)
}

func firstN(str string, n int) string {
v := []rune(str)
if n >= len(v) {
return str
}
return string(v[:n])
}

func MergePr(details GhPrClientDetails, number *int) error {
operation := func() error {
err := tryMergePR(details, number)
Expand Down
64 changes: 64 additions & 0 deletions internal/pkg/githubapi/github_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package githubapi

import (
"bytes"
"testing"
)

func TestGenerateSafePromotionBranchName(t *testing.T) {
t.Parallel()
prNumber := 11
originBranch := "originBranch"
targetPaths := []string{"targetPath1", "targetPath2"}
result := generateSafePromotionBranchName(prNumber, originBranch, targetPaths)
expectedResult := "promotions/11-originBranch-676f02019f18"
if result != expectedResult {
t.Errorf("Expected %s, got %s", expectedResult, result)
}
}

// TestGenerateSafePromotionBranchNameLongBranchName tests the case where the original branch name is longer than 250 characters
func TestGenerateSafePromotionBranchNameLongBranchName(t *testing.T) {
t.Parallel()
prNumber := 11

originBranch := string(bytes.Repeat([]byte("originBranch"), 100))
targetPaths := []string{"targetPath1", "targetPath2"}
result := generateSafePromotionBranchName(prNumber, originBranch, targetPaths)
if len(result) > 250 {
t.Errorf("Expected branch name to be less than 250 characters, got %d", len(result))
}
}

// TestGenerateSafePromotionBranchNameLongTargets tests the case where the target paths are longer than 250 characters
func TestGenerateSafePromotionBranchNameLongTargets(t *testing.T) {
t.Parallel()
prNumber := 11
originBranch := "originBranch"
targetPaths := []string{
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/1",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/2",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/3",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/4",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/5",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/6",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/7",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/8",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/9",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/10",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/11",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/12",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/13",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/14",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/15",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/16",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/17",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/18",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/19",
"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong/target/path/20",
}
result := generateSafePromotionBranchName(prNumber, originBranch, targetPaths)
if len(result) > 250 {
t.Errorf("Expected branch name to be less than 250 characters, got %d", len(result))
}
}

0 comments on commit a3b2abe

Please sign in to comment.