diff --git a/internal/digest/digest.go b/internal/digest/digest.go index d065ea4f6..469ece5f4 100644 --- a/internal/digest/digest.go +++ b/internal/digest/digest.go @@ -343,14 +343,13 @@ func RemoteDockerImageSha256(client *requests.Client, imageName, imageTag, regis return strings.TrimPrefix(digestHeader, "sha256:"), nil } +const validSha256regex = "^([a-f0-9]{64})$" + +var validSha256CompiledRegex = regexp.MustCompile(validSha256regex) + // ValidateDigest checks if a digest matches the sha256 regex func ValidateDigest(sha256ToCheck string) error { - validSha256regex := "^([a-f0-9]{64})$" - r, err := regexp.Compile(validSha256regex) - if err != nil { - return fmt.Errorf("failed to validate the provided SHA256 fingerprint") - } - if !r.MatchString(sha256ToCheck) { + if !validSha256CompiledRegex.MatchString(sha256ToCheck) { return fmt.Errorf("%s is not a valid SHA256 fingerprint. It should match the pattern %v", sha256ToCheck, validSha256regex) } return nil diff --git a/internal/digest/digest_test.go b/internal/digest/digest_test.go index fcb471b6c..7f07ff913 100644 --- a/internal/digest/digest_test.go +++ b/internal/digest/digest_test.go @@ -973,3 +973,11 @@ func (suite *DigestTestSuite) TestGetExcludePathsFromIgnoreFile() { func TestDigestTestSuite(t *testing.T) { suite.Run(t, new(DigestTestSuite)) } + +func BenchmarkValidateDigest(b *testing.B) { + sha := "db40d79b3a15b17ee9fcc2f49aa73736e0073de6b5a35c459268bb9a31e55139" + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = ValidateDigest(sha) + } +} diff --git a/internal/filters/resourceFilter.go b/internal/filters/resourceFilter.go index 39668fbd5..45cc59c7c 100644 --- a/internal/filters/resourceFilter.go +++ b/internal/filters/resourceFilter.go @@ -11,6 +11,9 @@ type ResourceFilterOptions struct { IncludeNamesRegex []string ExcludeNames []string ExcludeNamesRegex []string + + compiledExclude []*regexp.Regexp + compiledInclude []*regexp.Regexp } // IsSet checks if the filter options are set @@ -18,6 +21,49 @@ func (filter *ResourceFilterOptions) IsSet() bool { return len(filter.IncludeNames) > 0 || len(filter.IncludeNamesRegex) > 0 || len(filter.ExcludeNames) > 0 || len(filter.ExcludeNamesRegex) > 0 } +// CompilePatterns pre-compiles all include and exclude regex patterns +func (filter *ResourceFilterOptions) CompilePatterns() error { + if _, err := filter.compileExcludeRegexes(); err != nil { + return err + } + if _, err := filter.compileIncludeRegexes(); err != nil { + return err + } + return nil +} + +func (filter *ResourceFilterOptions) compileExcludeRegexes() ([]*regexp.Regexp, error) { + if filter.compiledExclude != nil { + return filter.compiledExclude, nil + } + compiled := make([]*regexp.Regexp, 0, len(filter.ExcludeNamesRegex)) + for _, pattern := range filter.ExcludeNamesRegex { + re, err := regexp.Compile(pattern) + if err != nil { + return nil, fmt.Errorf("invalid exclude name regex pattern %s: %v", pattern, err) + } + compiled = append(compiled, re) + } + filter.compiledExclude = compiled + return compiled, nil +} + +func (filter *ResourceFilterOptions) compileIncludeRegexes() ([]*regexp.Regexp, error) { + if filter.compiledInclude != nil { + return filter.compiledInclude, nil + } + compiled := make([]*regexp.Regexp, 0, len(filter.IncludeNamesRegex)) + for _, pattern := range filter.IncludeNamesRegex { + re, err := regexp.Compile(pattern) + if err != nil { + return nil, fmt.Errorf("invalid include name regex pattern %s: %v", pattern, err) + } + compiled = append(compiled, re) + } + filter.compiledInclude = compiled + return compiled, nil +} + // ShouldInclude checks if a name should be included or not according to the filter options // the filter should only be used for one operation (include, exclude) func (filter *ResourceFilterOptions) ShouldInclude(name string) (bool, error) { @@ -25,11 +71,11 @@ func (filter *ResourceFilterOptions) ShouldInclude(name string) (bool, error) { if slices.Contains(filter.ExcludeNames, name) { return false, nil } - for _, pattern := range filter.ExcludeNamesRegex { - re, err := regexp.Compile(pattern) - if err != nil { - return false, fmt.Errorf("invalid exclude name regex pattern %s: %v", pattern, err) - } + excludeRegexes, err := filter.compileExcludeRegexes() + if err != nil { + return false, err + } + for _, re := range excludeRegexes { if re.MatchString(name) { return false, nil } @@ -41,11 +87,11 @@ func (filter *ResourceFilterOptions) ShouldInclude(name string) (bool, error) { return true, nil } - for _, pattern := range filter.IncludeNamesRegex { - re, err := regexp.Compile(pattern) - if err != nil { - return false, fmt.Errorf("invalid include name regex pattern %s: %v", pattern, err) - } + includeRegexes, err := filter.compileIncludeRegexes() + if err != nil { + return false, err + } + for _, re := range includeRegexes { if re.MatchString(name) { return true, nil } diff --git a/internal/filters/resourceFilter_test.go b/internal/filters/resourceFilter_test.go index f11ad5362..d5642006d 100644 --- a/internal/filters/resourceFilter_test.go +++ b/internal/filters/resourceFilter_test.go @@ -112,8 +112,50 @@ func (suite *FiltersSuite) TestShouldInclude() { } } +func (suite *FiltersSuite) TestCompilePatterns() { + validFilter := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"^foo.*$", "^bar.*$"}, + ExcludeNamesRegex: []string{"^baz.*$", "^qux.*$"}, + } + err := validFilter.CompilePatterns() + require.NoError(suite.T(), err) + require.Len(suite.T(), validFilter.compiledInclude, 2) + require.Len(suite.T(), validFilter.compiledExclude, 2) + + // Subsequent ShouldInclude calls reuse compiled regexes + inc, err := validFilter.ShouldInclude("baz123") + require.NoError(suite.T(), err) + require.False(suite.T(), inc) + + inc, err = validFilter.ShouldInclude("other") + require.NoError(suite.T(), err) + require.True(suite.T(), inc) + + invalidInclude := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"[invalid"}, + } + require.Error(suite.T(), invalidInclude.CompilePatterns()) + + invalidExclude := &ResourceFilterOptions{ + ExcludeNamesRegex: []string{"[invalid"}, + } + require.Error(suite.T(), invalidExclude.CompilePatterns()) +} + // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run func TestFiltersSuite(t *testing.T) { suite.Run(t, new(FiltersSuite)) } + +func BenchmarkShouldInclude_Regex(b *testing.B) { + filter := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"^prod-.*$", "^staging-.*$"}, + } + _ = filter.CompilePatterns() + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = filter.ShouldInclude("prod-namespace-app") + } +} diff --git a/internal/gitview/gitView.go b/internal/gitview/gitView.go index e9365817e..e6c95a2cf 100644 --- a/internal/gitview/gitView.go +++ b/internal/gitview/gitView.go @@ -282,12 +282,20 @@ func getCommitURL(repoURL, commitHash string) string { // matches lookup happens in the commit message first, and if none is found, matching against the branch name is done // if no matches are found in both the commit message and the branch name, an empty slice is returned func (gv *GitView) MatchPatternInCommitMessageORBranchName(pattern, commitSHA, secondarySource string, ignoreBranchMatch bool) ([]string, *CommitInfo, error) { + re, err := regexp.Compile(pattern) + if err != nil { + return []string{}, nil, fmt.Errorf("invalid pattern regex %s: %w", pattern, err) + } + return gv.MatchRegexpInCommitMessageORBranchName(re, commitSHA, secondarySource, ignoreBranchMatch) +} + +// MatchRegexpInCommitMessageORBranchName returns a slice of strings matching a pre-compiled regular expression in a commit message or branch name +func (gv *GitView) MatchRegexpInCommitMessageORBranchName(re *regexp.Regexp, commitSHA, secondarySource string, ignoreBranchMatch bool) ([]string, *CommitInfo, error) { commitInfo, err := gv.GetCommitInfoFromCommitSHA(commitSHA, true, []string{}) if err != nil { return []string{}, nil, err } - re := regexp.MustCompile(pattern) commitMatches := re.FindAllString(commitInfo.Message, -1) branchMatches := re.FindAllString(commitInfo.Branch, -1) secondaryMatches := re.FindAllString(secondarySource, -1) diff --git a/internal/gitview/gitView_test.go b/internal/gitview/gitView_test.go index 2ff79040f..3227fdf88 100644 --- a/internal/gitview/gitView_test.go +++ b/internal/gitview/gitView_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "testing" "github.com/kosli-dev/cli/internal/jira" @@ -513,6 +514,13 @@ func (suite *GitViewTestSuite) TestMatchPatternInCommitMessageORBranchName() { want: []string{"#324"}, wantError: false, }, + { + name: "Invalid regex pattern returns an error", + pattern: "[invalid", + commitMessage: "test commit", + want: []string{}, + wantError: true, + }, } { suite.Run(t.name, func() { @@ -538,6 +546,23 @@ func (suite *GitViewTestSuite) TestMatchPatternInCommitMessageORBranchName() { } } +func (suite *GitViewTestSuite) TestMatchRegexpInCommitMessageORBranchName() { + _, workTree, fs, err := testHelpers.InitializeGitRepo(suite.tmpDir) + require.NoError(suite.T(), err) + + commitSha, err := testHelpers.CommitToRepo(workTree, fs, "Resolves JIRA-100 and JIRA-200") + require.NoError(suite.T(), err) + + gitView, err := New(suite.tmpDir) + require.NoError(suite.T(), err) + + re := regexp.MustCompile(`JIRA-[0-9]+`) + matches, commitInfo, err := gitView.MatchRegexpInCommitMessageORBranchName(re, commitSha, "", false) + require.NoError(suite.T(), err) + require.NotNil(suite.T(), commitInfo) + require.ElementsMatch(suite.T(), []string{"JIRA-100", "JIRA-200"}, matches) +} + func (suite *GitViewTestSuite) TestResolveRevision() { _, workTree, fs, err := testHelpers.InitializeGitRepo(suite.tmpDir) require.NoError(suite.T(), err) diff --git a/internal/jira/jira.go b/internal/jira/jira.go index 2ba96445f..e0a91746f 100644 --- a/internal/jira/jira.go +++ b/internal/jira/jira.go @@ -107,6 +107,13 @@ func (jc *JiraConfig) GetJiraIssueInfo(issueID string, issueFields string) (*Jir return result, nil } +const defaultJiraIssueKeyPattern = `\b[A-Z][A-Z0-9]{1,9}-[0-9]+` + +var ( + defaultJiraKeyRegex = regexp.MustCompile(defaultJiraIssueKeyPattern) + dashDigitRegex = regexp.MustCompile(`^-\d`) +) + func MakeJiraIssueKeyPattern(projectKeys []string) string { // Jira issue keys consist of [project-key]-[sequential-number]. // FindJiraIssueKeys uppercases the text before applying this pattern, so the @@ -114,7 +121,7 @@ func MakeJiraIssueKeyPattern(projectKeys []string) string { // are also uppercased here for the same reason. // more info: https://support.atlassian.com/jira-software-cloud/docs/what-is-an-issue/#Workingwithissues-Projectandissuekeys if len(projectKeys) == 0 { - return `\b[A-Z][A-Z0-9]{1,9}-[0-9]+` + return defaultJiraIssueKeyPattern } upper := make([]string, len(projectKeys)) for i, k := range projectKeys { @@ -131,8 +138,12 @@ func MakeJiraIssueKeyPattern(projectKeys []string) string { // immediately followed by a hyphen and a digit. func FindJiraIssueKeys(text string, projectKeys []string) []string { upperText := strings.ToUpper(text) - pattern := MakeJiraIssueKeyPattern(projectKeys) - re := regexp.MustCompile(pattern) + var re *regexp.Regexp + if len(projectKeys) == 0 { + re = defaultJiraKeyRegex + } else { + re = regexp.MustCompile(MakeJiraIssueKeyPattern(projectKeys)) + } candidates := re.FindAllString(upperText, -1) // Deduplicate (all candidates are already uppercase). @@ -146,10 +157,9 @@ func FindJiraIssueKeys(text string, projectKeys []string) []string { } // Filter out matches that are always followed by - in the uppercased text. - dashDigit := regexp.MustCompile(`^-\d`) var result []string for _, m := range unique { - if isPartialMultiSegment(upperText, m, dashDigit) { + if isPartialMultiSegment(upperText, m, dashDigitRegex) { continue } result = append(result, m)