Skip to content

fix(docs): correct invalid regex in snapshot ecs help examples - #1080

Merged
sami-alajrami merged 1 commit into
mainfrom
fix-ecs-regex-examples
Aug 10, 2026
Merged

fix(docs): correct invalid regex in snapshot ecs help examples#1080
sami-alajrami merged 1 commit into
mainfrom
fix-ecs-regex-examples

Conversation

@dangrondahl

Copy link
Copy Markdown
Contributor

Fixes #1079.

The --*-regex flags on kosli snapshot ecs compile their values as Go RE2 regexes (internal/filters/resourceFilter.go), but two help examples used "*-prod-*":

error parsing regexp: missing argument to repetition operator: `*`

Anyone copying those examples gets a failed snapshot, and only after the AWS API calls have already run.

Changes:

  • "*-prod-*" -> ".*-prod-.*" (the two broken examples)
  • "my-cluster-*" / "backend-*" -> "^my-cluster-.*" / "^backend-.*". These compiled, but read as globs: -* means "zero or more hyphens" and matching is unanchored, so "backend-*" really meant "contains backend" (it matched backendfoo).
  • A test asserting every regex pattern in the examples compiles. It fails on the old text with example regex "*-prod-*" does not compile.

Help text only - no behaviour change.

@claude

claude Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dangrondahl's task in 1m 33s —— View job


Review: PR #1080 — fix invalid regex in snapshot ecs help examples

  • Read PR diff
  • Gather context (help text + filter compilation)
  • Static analysis (build, vet)
  • Code review
  • Post feedback

Summary

This is a clean, well-scoped documentation fix. I confirmed the root cause: the --*-regex flags feed their values straight into regexp.Compile in internal/filters/resourceFilter.go (lines 29 & 45), so "*-prod-*" genuinely fails at runtime with "missing argument to repetition operator" — and only after the AWS API calls have run, exactly as the PR body states. The corrected patterns all compile and the anchoring change (^my-cluster-.*, ^backend-.*) fixes the subtler bug where unanchored backend-* matched substrings like backendfoo.

Verification on this checkout:

  • go build ./... — passes
  • go vet ./cmd/kosli/ ./internal/filters/ — clean
  • ⚠️ go test is blocked by tool permissions in this environment, so I reviewed the new test by inspection rather than execution (see note below).

What's good

  • A test guards the fix, not just the symptom. TestSnapshotECSExampleRegexesAreValid extracts every --*-regex "..." pattern from snapshotECSExample and asserts each compiles. This prevents regressions in future examples too, which is more valuable than a one-off correction. The require.NotEmpty guard is a nice touch — it fails loudly if the extraction regex ever stops matching (e.g. if the example format changes), rather than silently passing on zero patterns.
  • Correct diagnosis of the anchoring issue. Moving from glob-style -* to anchored ^...-.* matches user intent much better and the PR body explains the reasoning clearly.
  • Appropriately narrow. Help text + test only, no behaviour change.

Minor observations (non-blocking)

  1. Extraction regex coverage. The test pattern --[a-z-]*regex "([^"]*)" correctly captures all four flag forms (--clusters-regex, --exclude-regex, --services-regex, --exclude-services-regex). Just be aware it only matches double-quoted values on the same line — if someone ever adds an unquoted regex example it would be silently skipped. Fine for now given every example uses quotes.

  2. ^-anchoring but not $. The corrected examples anchor the start (^backend-.*) but stay open-ended at the end, which is the right call for prefix-style matching. Worth noting the two .*-prod-.* examples remain intentionally unanchored (substring match), consistent with their "by naming convention" comment. No change needed — just confirming this looks deliberate.

Verdict

Looks good to merge. The fix is correct, the reasoning is sound, and the regression test is a genuine improvement over the minimum required. My only gap is that I couldn't execute the test suite here due to sandbox permissions — recommend confirming CI is green before merge.
· fix-ecs-regex-examples

The --*-regex flags compile their values as Go RE2 regexes, but two
examples used "*-prod-*", which fails with "missing argument to
repetition operator" - and only after the AWS API calls have run.

Also anchor the four remaining examples that read as globs: "backend-*"
compiles, but -* means "zero or more hyphens" and matching is
unanchored, so it really means "contains backend".

Add a test that every regex pattern in the examples compiles.
@dangrondahl
dangrondahl force-pushed the fix-ecs-regex-examples branch from c8fb475 to 1dcc2f3 Compare August 10, 2026 12:55
@sami-alajrami
sami-alajrami merged commit f260dee into main Aug 10, 2026
27 of 28 checks passed
@sami-alajrami
sami-alajrami deleted the fix-ecs-regex-examples branch August 10, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: kosli snapshot ecs help examples use invalid regex *-prod-*

2 participants