Skip to content
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
40 changes: 23 additions & 17 deletions cmd/kosli/attestJira.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -303,15 +303,7 @@ func (o *attestJiraOptions) run(args []string) error {

// Search commit message, branch name, and secondary source for Jira issue keys,
// filtering out false positives from multi-segment identifiers like CVE-2026-41284.
searchTexts := []string{commitInfo.Message}
if !o.ignoreBranchMatch {
searchTexts = append(searchTexts, commitInfo.Branch)
}
if o.secondarySource != "" {
searchTexts = append(searchTexts, o.secondarySource)
}
combinedText := strings.Join(searchTexts, "\n")
issueIDs := jira.FindJiraIssueKeys(combinedText, o.projectKeys)
issueIDs := jira.FindJiraIssueKeys(jiraSearchText(commitInfo, o.secondarySource, o.ignoreBranchMatch), o.projectKeys)
logger.Debug("Checked for Jira issue references in Git commit %s on branch %s commit message:\n%s", commitInfo.Sha1, commitInfo.Branch, commitInfo.Message)
logger.Debug("the following Jira references are found in commit message or branch name: %v", issueIDs)

Expand DownExpand Up@@ -374,18 +366,32 @@ func (o *attestJiraOptions) run(args []string) error {
return wrapAttestationError(err)
}

func (o *attestJiraOptions) validateJiraProjectKeys() error {
// According to Jira documentation https://confluence.atlassian.com/adminjiraserver/changing-the-project-key-format-938847081.html
// the Jira project key has to start with a capital letter and can then have capital letters numbers and underscore.
// But Jira itself will accept lower case letters when searching a repository for matching branches and commits
matchesJiraProjectKeys, err := regexp.Compile("^[A-Za-z][A-Za-z0-9_]{1,9}$")
if err != nil {
return err
// jiraSearchText joins the texts that are searched for Jira issue keys: the commit
Comment thread
mbevc1 marked this conversation as resolved.
// message, the branch name unless ignoreBranchMatch is set, and the secondary source
// when one is given.
func jiraSearchText(commitInfo *gitview.CommitInfo, secondarySource string, ignoreBranchMatch bool) string {
searchTexts := []string{commitInfo.Message}
if !ignoreBranchMatch {
searchTexts = append(searchTexts, commitInfo.Branch)
}
if secondarySource != "" {
searchTexts = append(searchTexts, secondarySource)
}
return strings.Join(searchTexts, "\n")
}

// jiraProjectKeyRegexp is compiled once, so validateJiraProjectKeys does not re-compile a
// constant pattern on every invocation.
//
// According to Jira documentation https://confluence.atlassian.com/adminjiraserver/changing-the-project-key-format-938847081.html
// the Jira project key has to start with a capital letter and can then have capital letters numbers and underscore.
// But Jira itself will accept lower case letters when searching a repository for matching branches and commits.
var jiraProjectKeyRegexp = regexp.MustCompile("^[A-Za-z][A-Za-z0-9_]{1,9}$")

func (o *attestJiraOptions) validateJiraProjectKeys() error {
invalidKeys := []string{}
for _, projectKey := range o.projectKeys {
isValid := matchesJiraProjectKeys.MatchString(projectKey)
isValid := jiraProjectKeyRegexp.MatchString(projectKey)
if !isValid {
Comment thread
mbevc1 marked this conversation as resolved.
invalidKeys = append(invalidKeys, projectKey)
}
Expand Down
41 changes: 41 additions & 0 deletions cmd/kosli/attestJira_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ import (

billy "github.com/go-git/go-billy/v5"
git "github.com/go-git/go-git/v5"
"github.com/kosli-dev/cli/internal/gitview"
"github.com/kosli-dev/cli/internal/testHelpers"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
Expand DownExpand Up@@ -356,6 +357,46 @@ func execJiraTestCase(test cmdTestCase, suite *AttestJiraCommandTestSuite) {
runTestCmd(suite.T(), []cmdTestCase{test})
}

func TestJiraSearchText(t *testing.T) {
commitInfo := &gitview.CommitInfo{
BasicCommitInfo: gitview.BasicCommitInfo{
Message: "EX-1 fix the thing",
Branch: "bugfix/EX-2",
},
}
for _, tc := range []struct {
name string
secondarySource string
ignoreBranchMatch bool
want string
}{
{
name: "the commit message and the branch name are searched",
want: "EX-1 fix the thing\nbugfix/EX-2",
},
{
name: "the branch name is skipped when ignoreBranchMatch is set",
ignoreBranchMatch: true,
want: "EX-1 fix the thing",
},
{
name: "a secondary source is appended",
secondarySource: "EX-3",
want: "EX-1 fix the thing\nbugfix/EX-2\nEX-3",
},
{
name: "a secondary source is appended without the branch name",
secondarySource: "EX-3",
ignoreBranchMatch: true,
want: "EX-1 fix the thing\nEX-3",
},
} {
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.want, jiraSearchText(commitInfo, tc.secondarySource, tc.ignoreBranchMatch))
})
}
}

// 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 TestAttestJiraCommandTestSuite(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion cmd/kosli/attestation.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -180,9 +180,13 @@ func validateRepoFlags(repoURL, repoProvider string, validateURL bool) error {
return nil
}

// annotationKeyRegexp is compiled once, so processAnnotations does not re-compile a
// constant pattern for every annotation key it checks.
var annotationKeyRegexp = regexp.MustCompile(`^[A-Za-z0-9_]+$`)

func processAnnotations(annotations map[string]string) (map[string]string, error) {
for label := range annotations {
if !regexp.MustCompile(`^[A-Za-z0-9_]+$`).MatchString(label) {
if !annotationKeyRegexp.MatchString(label) {
return nil, fmt.Errorf("--annotate flag should be in the format key=value. Invalid key: '%s'. Key can only contain [A-Za-z0-9_]", label)
}
}
Expand Down
11 changes: 7 additions & 4 deletions cmd/kosli/snapshotCloudRun.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -158,6 +158,9 @@ func (o *snapshotCloudRunOptions) run(args []string) error {
return err
}

// compile the filter patterns once, instead of once per service and job
compiledFilter := o.resourceFilter.Compile()

ctx := context.Background()
client, err := newCloudRunClient(ctx, o.resolveNames)
if err != nil {
Expand All@@ -177,21 +180,21 @@ func (o *snapshotCloudRunOptions) run(args []string) error {

filteredServices := make([]cloudrun.Service, 0, len(services))
for _, svc := range services {
include, err := o.resourceFilter.ShouldInclude(svc.Name)
included, err := compiledFilter.ShouldInclude(svc.Name)
if err != nil {
return err
}
if include {
if included {
filteredServices = append(filteredServices, svc)
}
}
filteredJobs := make([]cloudrun.Job, 0, len(jobs))
for _, job := range jobs {
include, err := o.resourceFilter.ShouldInclude(job.Name)
included, err := compiledFilter.ShouldInclude(job.Name)
if err != nil {
return err
}
if include {
if included {
filteredJobs = append(filteredJobs, job)
}
}
Expand Down
24 changes: 24 additions & 0 deletions cmd/kosli/snapshotCloudRun_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -158,6 +158,20 @@ func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunCmd() {
cmd: fmt.Sprintf(`snapshot cloud-run %s --project p --region r --include-regex "^a" --exclude-regex "^b" %s`, suite.envName, suite.defaultKosliArguments),
golden: "Error: only one of --include-regex, --exclude-regex is allowed\n",
},
{
wantError: true,
name: "10 snapshot cloud-run fails if --include-regex is an invalid regex",
cmd: fmt.Sprintf(`snapshot cloud-run %s --project p --region r --include-regex "[invalid" --dry-run %s`, suite.envName, suite.defaultKosliArguments),
// no literal names are set, so filtering the first service reaches the
// invalid pattern and the snapshot fails
goldenRegex: `invalid include name regex pattern \[invalid: error parsing regexp: missing closing \]`,
},
{
wantError: true,
name: "11 snapshot cloud-run fails if --exclude-regex is an invalid regex",
cmd: fmt.Sprintf(`snapshot cloud-run %s --project p --region r --exclude-regex "[invalid" --dry-run %s`, suite.envName, suite.defaultKosliArguments),
goldenRegex: `invalid exclude name regex pattern \[invalid: error parsing regexp: missing closing \]`,
},
}

runTestCmd(suite.T(), tests)
Expand DownExpand Up@@ -198,6 +212,16 @@ func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunFilter_ExcludeRegex(
require.Contains(suite.T(), out, `"serviceName": "beta"`)
}

// TestSnapshotCloudRunFilter_ExcludeLiteralShortCircuitsInvalidRegex covers the reporting
// semantics of an invalid pattern: it is reported only when filtering a name reaches it.
// Both stub services are excluded by name, so neither reaches the invalid pattern and the
// snapshot succeeds with nothing in it.
func (suite *SnapshotCloudRunTestSuite) TestSnapshotCloudRunFilter_ExcludeLiteralShortCircuitsInvalidRegex() {
out := suite.runFilteredCmd(`--exclude alpha,beta --exclude-regex "[invalid"`)
require.NotContains(suite.T(), out, `"serviceName": "alpha"`)
require.NotContains(suite.T(), out, `"serviceName": "beta"`)
}

// TestSnapshotCloudRunCmd_HappyPathReportsToServer exercises the full
// CLI → local Kosli server roundtrip with the GCP client stubbed: the env is
// already created in SetupTest with type "cloud-run", and the command is
Expand Down
37 changes: 23 additions & 14 deletions internal/aws/aws.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -228,8 +228,9 @@ func (staticCreds *AWSStaticCreds) NewECSClient() (*ecs.Client, error) {
}

// getFilteredLambdaFuncs fetches a filtered set of lambda functions recursively (50 at a time) and returns a list of FunctionConfiguration
// filter is pre-compiled by the caller, so its regex patterns are not re-compiled per function or per page
func getFilteredLambdaFuncs(client LambdaAPI, nextMarker *string, allFunctions *[]types.FunctionConfiguration,
filter *filters.ResourceFilterOptions) (*[]types.FunctionConfiguration, error) {
filter *filters.CompiledResourceFilter) (*[]types.FunctionConfiguration, error) {
params := &lambda.ListFunctionsInput{}
if nextMarker != nil {
params.Marker = nextMarker
Expand All@@ -240,16 +241,15 @@ func getFilteredLambdaFuncs(client LambdaAPI, nextMarker *string, allFunctions *
return allFunctions, err
}

if len(filter.IncludeNames) == 0 && len(filter.IncludeNamesRegex) == 0 &&
len(filter.ExcludeNames) == 0 && len(filter.ExcludeNamesRegex) == 0 {
if !filter.IsSet() {
*allFunctions = append(*allFunctions, listFunctionsOutput.Functions...)
} else {
for _, f := range listFunctionsOutput.Functions {
include, err := filter.ShouldInclude(*f.FunctionName)
included, err := filter.ShouldInclude(*f.FunctionName)
if err != nil {
return allFunctions, err
}
if include {
if included {
*allFunctions = append(*allFunctions, f)
}
}
Expand DownExpand Up@@ -277,7 +277,10 @@ func (staticCreds *AWSStaticCreds) GetLambdaPackageData(filter *filters.Resource
func getLambdaPackageDataFromClient(client LambdaAPI, filter *filters.ResourceFilterOptions) ([]*LambdaData, error) {
lambdaData := []*LambdaData{}

filteredFunctions, err := getFilteredLambdaFuncs(client, nil, &[]types.FunctionConfiguration{}, filter)
// compile the filter patterns once, instead of once per function and per page
compiledFilter := filter.Compile()

filteredFunctions, err := getFilteredLambdaFuncs(client, nil, &[]types.FunctionConfiguration{}, compiledFilter)
if err != nil {
return lambdaData, err
}
Expand DownExpand Up@@ -577,8 +580,9 @@ func downloadFileFromBucket(downloader S3DownloadAPI, dirName, key, bucket strin
}

// getFilteredECSClusters fetches a filtered set of ECS clusters recursively (50 at a time) and returns a list of ecs Clusters
// clusterFilter is pre-compiled by the caller, so its regex patterns are not re-compiled per cluster or per page
func getFilteredECSClusters(client *ecs.Client, allClusters *[]ecsTypes.Cluster,
clusterFilter *filters.ResourceFilterOptions, nextToken *string, logger *logger.Logger) (*[]ecsTypes.Cluster, error) {
clusterFilter *filters.CompiledResourceFilter, nextToken *string, logger *logger.Logger) (*[]ecsTypes.Cluster, error) {
params := &ecs.ListClustersInput{}
if nextToken != nil {
params.NextToken = nextToken
Expand All@@ -599,11 +603,11 @@ func getFilteredECSClusters(client *ecs.Client, allClusters *[]ecsTypes.Cluster,
*allClusters = append(*allClusters, describeClustersOutput.Clusters...)
} else {
for _, c := range describeClustersOutput.Clusters {
include, err := clusterFilter.ShouldInclude(*c.ClusterName)
included, err := clusterFilter.ShouldInclude(*c.ClusterName)
if err != nil {
return allClusters, err
}
if include {
if included {
*allClusters = append(*allClusters, c)
}
}
Expand DownExpand Up@@ -633,7 +637,11 @@ func (staticCreds *AWSStaticCreds) GetEcsTasksData(clusterFilter, serviceFilter
return allTasksData, fmt.Errorf("failed to create ECS client: %w", err)
}

filteredClusters, err := getFilteredECSClusters(client, &[]ecsTypes.Cluster{}, clusterFilter, nil, logger)
// compile the filter patterns once, instead of once per cluster and per service
compiledClusterFilter := clusterFilter.Compile()
compiledServiceFilter := serviceFilter.Compile()

filteredClusters, err := getFilteredECSClusters(client, &[]ecsTypes.Cluster{}, compiledClusterFilter, nil, logger)
if err != nil {
return allTasksData, fmt.Errorf("failed to filter ECS clusters: %w", err)
}
Expand All@@ -649,7 +657,7 @@ func (staticCreds *AWSStaticCreds) GetEcsTasksData(clusterFilter, serviceFilter
go func(clusterName string) {
defer wg.Done()

filteredServices, err := getFilteredECSServicesInCluster(client, clusterName, &[]ecsTypes.Service{}, serviceFilter, nil, logger)
filteredServices, err := getFilteredECSServicesInCluster(client, clusterName, &[]ecsTypes.Service{}, compiledServiceFilter, nil, logger)
if err != nil {
errChan <- fmt.Errorf("failed to filter ECS services in cluster %s: %w", clusterName, err)
return
Expand DownExpand Up@@ -682,7 +690,8 @@ func (staticCreds *AWSStaticCreds) GetEcsTasksData(clusterFilter, serviceFilter
}

// getFilteredECSServicesInCluster fetches a filtered set of ECS services recursively (10 at a time) and returns a list of ecs Services
func getFilteredECSServicesInCluster(client ECSServicesAPI, cluster string, allServices *[]ecsTypes.Service, serviceFilter *filters.ResourceFilterOptions,
// serviceFilter is pre-compiled by the caller, so its regex patterns are not re-compiled per service, per page or per cluster
func getFilteredECSServicesInCluster(client ECSServicesAPI, cluster string, allServices *[]ecsTypes.Service, serviceFilter *filters.CompiledResourceFilter,
nextToken *string, logger *logger.Logger) (*[]ecsTypes.Service, error) {
listInput := &ecs.ListServicesInput{
Cluster: aws.String(cluster),
Expand DownExpand Up@@ -713,11 +722,11 @@ func getFilteredECSServicesInCluster(client ECSServicesAPI, cluster string, allS
*allServices = append(*allServices, describeServicesOutput.Services...)
} else {
for _, s := range describeServicesOutput.Services {
include, err := serviceFilter.ShouldInclude(*s.ServiceName)
included, err := serviceFilter.ShouldInclude(*s.ServiceName)
if err != nil {
return allServices, err
}
if include {
if included {
*allServices = append(*allServices, s)
}
}
Expand Down
24 changes: 23 additions & 1 deletion internal/aws/aws_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -763,13 +763,35 @@ func (suite *AWSTestSuite) TestGetFilteredLambdaFuncs() {
filter: &filters.ResourceFilterOptions{IncludeNamesRegex: []string{"invalid["}},
wantErr: true,
},
{
// an invalid pattern is only reported once filtering a function reaches it,
// so functions settled by ExcludeNames are filtered without error
name: "invalid regex behind an excluded literal name is not reached",
functions: []string{"alpha"},
filter: &filters.ResourceFilterOptions{
ExcludeNames: []string{"alpha"},
ExcludeNamesRegex: []string{"invalid["},
},
expectedNames: []string{},
},
{
name: "invalid regex is reported once a function does not match the excluded literal name",
functions: []string{"alpha", "beta"},
filter: &filters.ResourceFilterOptions{
ExcludeNames: []string{"alpha"},
ExcludeNamesRegex: []string{"invalid["},
},
wantErr: true,
},
} {
suite.Run(t.name, func() {
client := fakeLambdaClientWithFunctions(t.functions...)
if t.pageSize > 0 {
client.PageSize = t.pageSize
}
result, err := getFilteredLambdaFuncs(client, nil, &[]types.FunctionConfiguration{}, t.filter)
compiledFilter := t.filter.Compile()

result, err := getFilteredLambdaFuncs(client, nil, &[]types.FunctionConfiguration{}, compiledFilter)
if t.wantErr {
require.Error(suite.T(), err)
return
Expand Down
8 changes: 6 additions & 2 deletions internal/aws/ecs_services_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -21,11 +21,13 @@ func TestGetFilteredECSServicesInCluster_EmptyCluster(t *testing.T) {
Services: []ecsTypes.Service{},
}

emptyFilter := (&filters.ResourceFilterOptions{}).Compile()

allServices, err := getFilteredECSServicesInCluster(
client,
"empty-cluster",
&[]ecsTypes.Service{},
&filters.ResourceFilterOptions{},
emptyFilter,
nil,
logger.NewStandardLogger(),
)
Expand All@@ -43,11 +45,13 @@ func TestGetFilteredECSServicesInCluster_WithServices(t *testing.T) {
Services: []ecsTypes.Service{{ServiceName: &svcName}},
}

emptyFilter := (&filters.ResourceFilterOptions{}).Compile()

allServices, err := getFilteredECSServicesInCluster(
client,
"cluster",
&[]ecsTypes.Service{},
&filters.ResourceFilterOptions{},
emptyFilter,
nil,
logger.NewStandardLogger(),
)
Expand Down
Loading
Loading