fix(auth): persist generated auth secret and enforce fallback projection - #5
Conversation
- patch `RedisCluster.spec.authSecret` when auto-generating `<cluster>-auth` - resolve effective auth secret consistently in controller and instance manager - fail instance reconcile when fallback `<cluster>-auth/password` is missing or empty - add unit tests for persisted spec and fatal fallback-auth behavior - harden smoke test to assert unauthenticated `PING` fails and authenticated `PING` succeeds - update docs to describe default auth generation, persistence, and enforcement
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on March 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb5e740f85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| authSecretName := effectiveAuthSecretName(cluster) |
There was a problem hiding this comment.
reconcileSecrets now derives authSecretName via effectiveAuthSecretName, which falls back to <cluster>-auth when spec.authSecret.name is blank, but the auto-generation/persistence path still runs only when spec.authSecret is nil. Because the CRD schema requires name but does not enforce a non-empty value (config/crd/bases/redis.io_redisclusters.yaml), a cluster with authSecret: {name: ""} skips ensureAuthSecret yet still uses fallback resolution, so the projected password file is never guaranteed and the instance reconciler can repeatedly hit errFallbackAuthSecretUnavailable instead of converging.
Useful? React with 👍 / 👎.
- install envtest tools before running integration tests in CI - make cross-cluster integration test use explicit auth secret config - replace pgrep-based redis kill with /proc scan for container portability - remove unsupported redis-benchmark flag in chaos rolling update test - isolate chaos write/probe keys from baseline integrity key prefixes - harden fencing-annotation polling with bounded API get timeout - retry unauthenticated smoke ping checks and exec explicitly in redis container - ignore temp logs directory in .gitignore
- project auth/ACL keys as /projected/<secret>/<key> for data and sentinel pods - update projection tests to assert KeyToPath mappings - run rolling-update benchmark in a dedicated client pod outside Redis restarts - wait for failover via fence/phase/primary-change and fix fence poll context
- authenticate local instance-manager Redis client using projected auth secret - add helper functions and tests for effective auth secret resolution - force authenticated leader key scans and prefer ready pods for chaos client commands - wait for leader reachability before chaos baseline flush/write operations - run primary-kill workload from dedicated workload client pod - replace rolling-update redis-benchmark with a resilient cancellable write loop - avoid rolling workload key-prefix collisions with integrity assertions - drop brittle rolling-update offset regression check across primary restarts
1 participant
Summary
This PR fixes the auth-default regression where clusters could run without requirepass when spec.authSecret was omitted.
Closes #4.
Problem
When
spec.authSecretwas nil:<cluster>-authbut did not persist it in desired state.spec.authSecretwas present.What changed
spec.authSecreton the mainRedisClusterresource.<cluster>-auth.requirepass/masterauth.<cluster>-auth/passwordis missing or empty, reconcile now returns an error instead of silently continuing.redis-cli pingmust fail (NOAUTH/auth-required),pingmust succeed.Tests
spec.authSecretfor auto-generated auth.go test ./...passedmake lintpassed