Skip to content

feat(snapshot s3): add --include-regex / --exclude-regex path filters#882

Merged
mbevc1 merged 5 commits into
mainfrom
20260511_s3_regex
May 11, 2026
Merged

feat(snapshot s3): add --include-regex / --exclude-regex path filters#882
mbevc1 merged 5 commits into
mainfrom
20260511_s3_regex

Conversation

@mbevc1
Copy link
Copy Markdown
Contributor

@mbevc1 mbevc1 commented May 11, 2026

  • Path filters on kosli snapshot s3 previously only supported literal prefix matching, so excluding (e.g.) every PNG in a bucket required enumerating each key. Add regex variants in the same style as the existing --exclude / --exclude-regex pair on snapshot ecs, snapshot lambda, snapshot k8s, and snapshot cloudrun.

  • Also cancel old CI runs on on new push

Path filters on `kosli snapshot s3` previously only supported literal
prefix matching, so excluding (e.g.) every PNG in a bucket required
enumerating each key. Add regex variants in the same style as the
existing `--exclude` / `--exclude-regex` pair on `snapshot ecs`,
`snapshot lambda`, `snapshot k8s`, and `snapshot cloudrun`.
@mbevc1 mbevc1 added the enhancement New feature or request label May 11, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 11, 2026

Claude finished @mbevc1's task in 2m 7s —— View job


Re-review after latest push

  • Run static analysis (build, vet, mod tidy)
  • Review code changes
  • Post review feedback

Static Analysis

All clean — go build, go vet, and go mod tidy pass with no issues.

Review Summary

The author addressed all prior review feedback well:

  • Pre-compiled regexcompilePathRegex now compiles patterns once in GetS3Data before the object loop, passing []*regexp.Regexp to shouldExcludePath. This is actually better than the existing resourceFilter.go which still recompiles per call.
  • Multiple regex OR test — Added test cases for multiple patterns in a single slice (both include and exclude), verifying the OR semantics.
  • Renamed objectMatchesPathsobjectMatchesFilter for clarity.

No issues found. The code is clean and ready to merge. Details:

Area Assessment
Correctness Logic is sound. shouldExcludePath properly ORs literal prefix + regex within the same direction, and blocks cross-direction mixing via MuXRequiredFlags in PreRunE.
Consistency Follows the same mutual-exclusivity pattern used in snapshot cloudrun (snapshotCloudRun.go:104-108). Flag descriptions and help text are clear.
Testing TestShouldExcludePath covers: no filters, literal include/exclude, regex include/exclude, multi-pattern OR, and mixed literal+regex within the same direction. TestCompilePathRegex covers nil/empty/valid/invalid inputs. Command-level tests cover all four cross-direction mutual-exclusivity errors. Solid coverage.
Performance Regex pre-compilation means per-object cost is just MatchString, which is the right approach for large S3 buckets.
Security Regex patterns come from CLI flags (user-controlled), not external input. No ReDoS risk from the patterns shown, and regexp (RE2) is linear-time by design.

Nice work addressing the review comments. LGTM.

Comment thread internal/aws/aws.go Outdated
Comment thread cmd/kosli/snapshotS3.go
Comment thread internal/aws/aws_test.go Outdated
Comment thread internal/aws/aws.go
Comment thread internal/aws/aws.go
Copy link
Copy Markdown
Contributor

@gsavage gsavage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, @mbevc1

@mbevc1 mbevc1 enabled auto-merge (squash) May 11, 2026 15:55
@mbevc1 mbevc1 merged commit baba868 into main May 11, 2026
16 checks passed
@mbevc1 mbevc1 deleted the 20260511_s3_regex branch May 11, 2026 15:56
mbevc1 pushed a commit to kosli-dev/docs that referenced this pull request May 11, 2026
)

Updates the `kosli snapshot s3` CLI reference to document the new
`--include-regex` and `--exclude-regex` path filters introduced in
[kosli-dev/cli#882](kosli-dev/cli#882).

- Mention regex variants alongside literal-prefix
`--include`/`--exclude` in the long description
- Add `--include-regex` and `--exclude-regex` rows to the Flags table
- Update existing `--include`/`--exclude` descriptions to note mutual
exclusivity with the new regex flags
- Add an example using `--exclude-regex '.*\.png$'`

Co-authored-by: mintlify[bot] <109931778+mintlify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants