perf: optimise regex patterns compilation by mbevc1 · Pull Request #1114 · kosli-dev/cli

mbevc1 · 2026-08-20T15:12:32Z

Several hot paths recompiled the same regex patterns on every call, so filtering or scanning N items compiled the same patterns N times.

filters: ResourceFilterOptions.Compile returns a CompiledResourceFilter, which the AWS (Lambda, ECS clusters and services), Kubernetes and Cloud Run snapshot paths now build once per snapshot instead of once per resource name. Filtering 100 names against two patterns drops from ~348,000 to ~25,000 ns/op and from 5,000 to 108 allocs/op.
digest: the constant SHA256 fingerprint pattern is compiled at package level rather than on every ValidateDigest call.
jira: the default issue key pattern and the ^-\d helper are compiled at package level. A pattern built from project keys is still compiled per call, which cannot panic because validateJiraProjectKeys has already rejected any key that could carry regex metacharacters.

An invalid pattern is still reported exactly when it was before: only when matching a name reaches it. A name settled by ExcludeNames/IncludeNames, or by an earlier pattern that already matched, is filtered without the invalid pattern being consulted, so a filter that worked before still works. Compiling therefore cannot fail, and ShouldInclude returns the error.

Two clean-ups follow from the above:

kube: filterNamespaces matched names in one goroutine per namespace behind a mutex, and carried an error channel plus a cancellable context solely to report a compile failure out of those goroutines. With the patterns compiled up front the goroutine body cannot fail and the remaining work is microseconds of CPU, so it runs in a plain loop and the returned order is now deterministic. The pod fetching in GetPodsData stays concurrent.
gitview: MatchPatternInCommitMessageORBranchName had no production callers and called MustCompile on a caller-supplied pattern, so an invalid pattern would panic instead of returning an error. Removed; its search semantics are covered by the attest jira search-text test.

Closes#826

Checklist

Docs are autogenerated from CLI help — make sure the help text reflects your changes
Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

claude · 2026-08-20T15:13:20Z

Claude finished @mbevc1's task in 3m 43s —— View job

Review of the latest push (`d583bc9`, `a032f6f`)

Gather context (diff since the last round)
Static analysis — go build ./... ✅, go vet ./... ✅ (compiles tests too), go mod tidy → no diff ✅
Review the two new jira commits
Post review

I couldn't run the test suites — go test isn't in my allowlist here, and the integration suites need the docker-compose server / a KIND cluster anyway. Everything below is from reading and from build+vet.

The `""` sentinel lands correctly

makeJiraIssueKeyPattern unexported, three return meanings documented above the func line, and jiraIssueKeyRegexp is the only thing that maps "" → nil. I checked the reachability: jiraIssueKeyRegexp (internal/jira/jira.go:171) and jira_test.go:108 are the only references, both in-package, and FindJiraIssueKeys (jira.go:189-193) handles the nil before touching it. The one way to get the inversion — MustCompile("") matching at every position — now has no caller who could reach it.

The switch also reads better than the old len(projectKeys) == 0 check, because "is this the default pattern" is now asked of the pattern rather than re-derived from the input.

strings.TrimSpace closes the whitespace-key variant of the same hole, and the {" EX ", "", " ", "\t"} case pins all three of blank / whitespace / untrimmed at once.

Everything else re-verified on the final state

internal/filters/resourceFilter.go:71,73 — both literal slices cloned; TestCompileClonesLiteralNames has teeth.
internal/kube/kube.go:248 — collapsed to !filter.IsSet(), so ResourceFilterOptions.IsSet() keeps a production caller. internal/aws/aws.go:601 spells it the same way.
internal/aws/aws.go:640-641 — one compile per snapshot, compiledServiceFilter shared read-only across the per-cluster goroutines.
The -race Makefile target is gone and TestCompiledFilterConcurrentShouldInclude's comment now says plainly what it does establish ("concurrent matching returns the right answers … rather than that the sharing itself is synchronised") instead of pointing at a -race run nothing performs. Dropping the target and making the claim honest is the better of the two resolutions — the previous version asserted coverage that didn't exist.
kube_test.go — the invalid-pattern case no longer creates two namespaces it never needed, and survivors are derived from args.namespaces rather than naming filter-exc-b1 twice.

One nit, optional

Left inline on cmd/kosli/attestJira.go:395: --jira-project-key is a StringSliceVar, and cobra's CSV split doesn't trim, so --jira-project-key "ABC, DEF" is rejected for the space alone — while makeJiraIssueKeyPattern would now happily trim it. Pre-existing and legible when it fires; a TrimSpace in validateJiraProjectKeys would make the two sides agree. Behaviour change, so a follow-up is fine.

Verdict

No blocking issues. The perf claim holds (compile-per-snapshot rather than per-resource on the Lambda / ECS / k8s / Cloud Run paths), the lazy error semantics are preserved and now pinned by tests on both the unit and CLI level, and the jira changes turn two prose invariants into structural ones. Nice piece of work — particularly the decision to distinguish "no projects named" from "projects named, none usable" rather than collapsing them into the default pattern.
· branch 20260820_perf_improvement

mbevc1 · 2026-08-20T15:44:15Z

Fixed as suggested and addressed latest feedback:

slices.Clone on both literal name slices in Compile, plus a doc line saying so. Added TestCompileClonesLiteralNames and verified it has teeth — reverting the clones makes it fail on foo was excluded when the filter was compiled.
kube.go four-way check collapsed to !filter.IsSet(), which also keeps ResourceFilterOptions.IsSet() in production use.
validateJiraProjectKeys' constant pattern hoisted to a package-level jiraProjectKeyRegexp, dropping the dead error handling; internal/jira's doc comment now names it instead of repeating the literal.

Several hot paths recompiled the same regex patterns on every call, so filtering or scanning N items compiled the same patterns N times. - filters: ResourceFilterOptions.Compile returns a CompiledResourceFilter, which the AWS (Lambda, ECS clusters and services), Kubernetes and Cloud Run snapshot paths now build once per snapshot instead of once per resource name. Filtering 100 names against two patterns drops from ~348,000 to ~25,000 ns/op and from 5,000 to 108 allocs/op. - digest: the constant SHA256 fingerprint pattern is compiled at package level rather than on every ValidateDigest call. - jira: the default issue key pattern and the ^-\d helper are compiled at package level. A pattern built from project keys is still compiled per call, which cannot panic because validateJiraProjectKeys has already rejected any key that could carry regex metacharacters. An invalid pattern is still reported exactly when it was before: only when matching a name reaches it. A name settled by ExcludeNames/IncludeNames, or by an earlier pattern that already matched, is filtered without the invalid pattern being consulted, so a filter that worked before still works. Compiling therefore cannot fail, and ShouldInclude returns the error. Two clean-ups follow from the above: - kube: filterNamespaces matched names in one goroutine per namespace behind a mutex, and carried an error channel plus a cancellable context solely to report a compile failure out of those goroutines. With the patterns compiled up front the goroutine body cannot fail and the remaining work is microseconds of CPU, so it runs in a plain loop and the returned order is now deterministic. The pod fetching in GetPodsData stays concurrent. - gitview: MatchPatternInCommitMessageORBranchName had no production callers and called MustCompile on a caller-supplied pattern, so an invalid pattern would panic instead of returning an error. Removed; its search semantics are covered by the attest jira search-text test. Closes#826

- filters: Compile clones the literal name slices, so the immutability that CompiledResourceFilter's doc comment promises - and that makes sharing one compiled filter across the ECS goroutines safe - holds for the literals and not just the patterns. Covered by a test that fails without the clone. - kube: filterNamespaces' four-way length check is exactly !filter.IsSet(), as getFilteredLambdaFuncs already spells it. Collapsing it also keeps ResourceFilterOptions.IsSet() in production use. - attest jira: validateJiraProjectKeys compiled a constant pattern on every invocation, with error handling for a failure that cannot happen. It is now a package-level jiraProjectKeyRegexp, which internal/jira's jiraIssueKeyRegexp doc comment can name instead of repeating the literal. - digest: the pattern constant is validSha256Pattern rather than validSha256regex, which was neither idiomatic Go nor distinguishable at a glance from the compiled validSha256Regexp beside it. - kube test: TestFilterNamespaces asserted with ElementsMatch, which cannot fail on ordering, while this change claims the returned order is deterministic. It now builds the expectation in the cluster's listing order and asserts equality, with a guard so that assertion cannot pass vacuously. Also adds the k8s case for an invalid pattern behind an excluded literal name: the snapshot still fails there, because the cluster's own namespaces are not in ExcludeNames and one of them always reaches the pattern. Refs #826

processAnnotations re-compiled a constant pattern for every annotation key, which is the same shape as the digest and jira changes except that this one is inside the loop rather than once per invocation. Hoist it to a package-level annotationKeyRegexp, matching jiraProjectKeyRegexp. The pattern and the error message are unchanged, so the existing --annotate cases still pin the behaviour. This is the last constant pattern compiled inline outside tests; the remaining regexp.Compile calls all take a user-supplied pattern and have to stay dynamic. Also clone the namespace list in filterNamespaces' early return, which handed back the caller's slice fifteen lines above a Compile that clones precisely so callers cannot alias. Nothing mutates it today, so this is for consistency rather than a fix. Refs #826

MakeJiraIssueKeyPattern interpolated project keys into a regex verbatim, so "the pattern always compiles" held only because validateJiraProjectKeys had already rejected anything containing a metacharacter. That contract lived in a comment in cmd/kosli and was relied on by a MustCompile in internal/jira, two packages away from the check enforcing it. QuoteMeta each key instead, which makes it a property of the function rather than a promise its callers have to keep - and lets MakeJiraIssueKeyPattern, which is exported, be safe on its own terms. No behaviour change for a key a Jira project could actually have: none of [A-Za-z0-9_] is a metacharacter, so quoting leaves every valid key untouched and the existing pattern expectations are unchanged. Adds a case for keys that do carry metacharacters, which fails without the quoting. Refs #826

Follow-up to the quoting change, addressing review of it. makeJiraIssueKeyPattern is now unexported. Its "" return means "no issue key can match", but it is the one sentinel that inverts when used: MustCompile("") succeeds and matches at every position, so a caller who compiles it gets match-everything, the widening the sentinel exists to prevent. Only jiraIssueKeyRegexp stood between the two, and the function had just two references, both in this package. Its rationale also sat inside the function body, so go doc printed a bare signature for a return value with three meanings; that is now a doc comment listing all three and saying the empty one must not be compiled. Keys that all drop out no longer fall back to the default pattern. Asking for no project in particular and asking for projects that all turn out to be unusable are different questions: the first means every project, the second means none can match. Answering the second with the default would widen a caller who named projects to every project, and on an attestation path the keys that widening invents are then looked up and attested. Blank keys are dropped and the rest trimmed. An empty key interpolates an empty alternative that matches any -[0-9]+, so {"EX", ""} reported -41284 out of CVE-2026-41284 as an issue key; a whitespace-only key does the same one column over, since a space is not a metacharacter for QuoteMeta to escape; and an untrimmed " EX" reports " EX-12", which is not a key any project has. None of this is reachable from the CLI - validateJiraProjectKeys rejects all three before FindJiraIssueKeys sees them - so the guard is kept to one line and one test case rather than covered exhaustively. Also in internal/kube's TestFilterNamespaces: drop the namespaces from the invalid-pattern case, which created and deleted two against a real cluster without needing them, since the cluster's own namespaces reach the pattern. The other error case keeps its namespace, which is load-bearing - it exists to be settled by ExcludeNames while the others still reach the invalid pattern. And state plainly what TestCompiledFilterConcurrentShouldInclude does establish: every goroutine writes its own slot and reads only immutable state, so its assertions check that concurrent matching returns the right answers, not that the sharing is synchronised. Refs #826

mbevc1 added enhancement New feature or request go Pull requests that update go code performance Issues related to performance bottlenecks or improvements labels Aug 20, 2026

mbevc1 mentioned this pull request Aug 20, 2026
perf: compile regex patterns once instead of per-call in hot paths #1098
Closed