Skip to content

make redis optional in committer-only mode - #209

Merged
sunib merged 3 commits into
ConfigButler:mainfrom
consooo:feat/optional-redis-204
Jul 8, 2026
Merged

make redis optional in committer-only mode#209
sunib merged 3 commits into
ConfigButler:mainfrom
consooo:feat/optional-redis-204

Conversation

@consooo

@consooo consooo commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

in committer-only mode, redis/valkey is no longer a hard requirement

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test coverage improvement

Testing

  • Unit tests pass locally
  • Integration tests pass locally
  • Manual testing completed (if applicable)
  • New tests added for new functionality

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Related Issues

Closes #204

Summary by CodeRabbit

  • New Features

    • Commits can now run in committer-only mode without requiring Redis/Valkey.
    • When Redis is configured, watch resume cursors persist across restarts for warmer recovery.
  • Bug Fixes

    • Improved startup validation so Redis is only required for attribution mode and related webhook use.
    • Clarified configuration behavior and defaults across setup, upgrade, and architecture guidance.
  • Tests

    • Updated flag-parsing coverage for committer-only and attribution-enabled configurations.

@consooo
consooo requested a review from sunib as a code owner July 8, 2026 07:38
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@consooo, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9bf391cb-1b77-4031-9112-4489e92287fc

📥 Commits

Reviewing files that changed from the base of the PR and between 73c9a91 and a487ff8.

📒 Files selected for processing (8)
  • README.md
  • charts/gitops-reverser/values.yaml
  • cmd/main.go
  • cmd/main_audit_server_test.go
  • docs/UPGRADING.md
  • docs/architecture.md
  • docs/config-flag-conventions.md
  • docs/configuration.md
📝 Walkthrough

Walkthrough

This PR makes Redis/Valkey optional when running in committer-only mode (--author-attribution=false). Redis wiring and readiness gates in cmd/main.go are now conditional on --redis-addr being set; validation requires Redis only for attribution or the admission webhook. Tests and documentation are updated accordingly.

Changes

Optional Redis wiring and validation

Layer / File(s) Summary
Conditional Redis wiring and attribution startup
cmd/main.go
Redis cursor store/readiness gate created only when --redis-addr is set; attribution startup restructured into a switch with updated committer-only logging; flag help text updated.
Config validation for redis-addr requirements
cmd/main.go
validateAuditConfig allows empty --redis-addr unless attribution is enabled; validateAdmissionWebhookConfig now rejects admission webhook with empty --redis-addr.
Flag-parsing test updates
cmd/main_audit_server_test.go
Renamed/added tests for redis-required-when-attribution, committer-only-no-redis, and admission-webhook-without-redis error case.
Documentation updates
README.md, charts/gitops-reverser/values.yaml, docs/UPGRADING.md, docs/architecture.md, docs/config-flag-conventions.md, docs/configuration.md
Docs revised to describe Redis as optional in committer-only mode and required only for attribution/admission webhook, with warm vs cold-replay restart behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
    participant Operator as "main()"
    participant Config as "Config Validation"
    participant Redis as "Redis/Valkey"
    participant Watch as "Watch Cursor Store"

    Operator->>Config: validateAuditConfig(redisAddr, authorAttribution)
    Config-->>Operator: error if attribution enabled and redisAddr empty
    Operator->>Config: validateAdmissionWebhookConfig(redisAddr, webhookEnabled)
    Config-->>Operator: error if webhook enabled and redisAddr empty

    alt redisAddr set
        Operator->>Redis: NewRedisStore + readiness gate
        Operator->>Watch: wire Redis-backed cursor store
    else redisAddr empty
        Operator->>Watch: leave cursor store nil (cold replay on restart)
    end

    alt authorAttribution enabled
        Operator->>Redis: build attribution index, start audit ingress
    else committer-only mode
        Operator->>Operator: log committer identity or no-redis mode
    end
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The admission-webhook Redis validation change adds behavior not requested in #204, so the change set is not fully confined to the linked issue. Move admission-webhook Redis handling into a separate PR or add that requirement to #204 before merging.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: making Redis optional in committer-only mode.
Description check ✅ Passed The PR description matches the template well, with description, type, testing, checklist, and related issue filled in.
Linked Issues check ✅ Passed The code changes align with #204 by skipping Redis wiring when unset, cold-replaying on restart, and failing fast for attribution without Redis.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

consooo added 2 commits July 8, 2026 09:40
Signed-off-by: Constantin Ewert <c.ewert@micromata.de>
Signed-off-by: Constantin Ewert <c.ewert@micromata.de>
@consooo
consooo force-pushed the feat/optional-redis-204 branch from 73c9a91 to f35fbe3 Compare July 8, 2026 07:41

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@charts/gitops-reverser/values.yaml`:
- Around line 163-169: Update the Redis/Valkey comments in the queue.redis
section to remove the stale committer-only requirement note that says Redis is
still required. Keep the guidance in sync with the newer text so it clearly
states that an empty addr is allowed in committer-only mode and that Redis is
only required when attribution.enabled is true; check the nearby queue.redis
documentation block and remove or rewrite the outdated sentence.

In `@cmd/main.go`:
- Around line 176-197: Normalize cfg.redisAddr before the Redis setup branch in
cmd/main.go so whitespace-only values are treated the same as empty input;
otherwise the redisStore/newRedisReadinessGate path still runs for blank
addresses. Update the conditional around the Redis initialization to use the
trimmed value (or normalize cfg.redisAddr once near validation/config loading)
and keep the rest of the redisStore, watchMgr.WatchCursorStore, and
newRedisReadinessGate flow unchanged for real addresses.

In `@README.md`:
- Line 141: The quick-start still hardcodes Valkey installation without showing
the committer-only no-Redis path, so update the README quick-start section to
make the optional flow runnable. In the quick-start commands around the “Install
Valkey with auth” step, add a clear skip path and show the `queue.redis.addr=""`
override for the committer-only mode, keeping the instructions aligned with the
optional Valkey heading and the runnable setup.
- Around line 50-54: The README’s Redis guidance is inconsistent: the
operating-modes text still says every install requires Valkey/Redis even though
the newer committer-only paragraph says it is optional. Update the older
operating-modes section to match the new behavior, using the same Redis
terminology and the “committer-only” mode wording so the README stays consistent
end-to-end.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 787150ce-7acf-4560-baf3-23e98afc8a6b

📥 Commits

Reviewing files that changed from the base of the PR and between 99dcc63 and 73c9a91.

📒 Files selected for processing (8)
  • README.md
  • charts/gitops-reverser/values.yaml
  • cmd/main.go
  • cmd/main_audit_server_test.go
  • docs/UPGRADING.md
  • docs/architecture.md
  • docs/config-flag-conventions.md
  • docs/configuration.md

Comment thread charts/gitops-reverser/values.yaml
Comment thread cmd/main.go
Comment thread README.md
Comment thread README.md
@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/main.go 81.2% 5 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@sunib sunib left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Thanks for doing this, and fast! Good step in making the first steps easier.

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.

[FEATURE] Make Redis optional when author attribution is disabled

2 participants