Add redis auth #528
Add redis auth #528
Conversation
dosubot
bot
added
the
size:M
WalkthroughThe pull request includes several updates to the Redis application files. Key changes involve the increment of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
packages/apps/redis/values.yaml (1)
7-13: Enhance security documentation for authEnabled parameterWhile enabling authentication by default is a good security practice, consider expanding the documentation comment to include:
- Security implications of disabling authentication
- Where the generated password can be found
- How to provide a custom password if needed
-## @param authEnabled Enable password generation +## @param authEnabled Enable Redis password authentication +## @description When enabled, a password will be auto-generated and stored in a Secret named <release-name>-auth. +## Disabling authentication is not recommended in production environments.packages/apps/redis/values.schema.json (1)
24-28: Consider enhancing schema validationWhile the schema is correctly defined, consider adding these validations:
- Add
requiredarray to mark essential security parameters- Consider adding
examplesfield to improve documentation{ "title": "Chart Values", "type": "object", + "required": ["authEnabled"], "properties": { ... "authEnabled": { "type": "boolean", "description": "Enable password generation", - "default": true + "default": true, + "examples": [true, false] } } }packages/apps/redis/README.md (1)
22-22: Documentation needs more details about authenticationWhile enabling authentication by default is a good security practice, the documentation should include:
- The implications of enabling authentication
- How to access the generated password
- Impact on existing applications that need to connect
- Instructions for rotation or manual password management
packages/apps/redis/templates/redisfailover.yaml (1)
73-76: Consider implementing password rotation mechanismThe current implementation doesn't provide a mechanism for password rotation. Consider:
- Adding annotations to track password age
- Implementing a method to trigger rotation
- Ensuring applications can handle password updates gracefully
Example enhancement:
metadata: annotations: redis.internal.example.com/password-updated: {{ now | date "2006-01-02T15:04:05Z07:00" }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/apps/redis/Chart.yaml(1 hunks)packages/apps/redis/README.md(1 hunks)packages/apps/redis/templates/dashboard-resourcemap.yaml(1 hunks)packages/apps/redis/templates/redisfailover.yaml(2 hunks)packages/apps/redis/values.schema.json(1 hunks)packages/apps/redis/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/apps/redis/Chart.yaml
- packages/apps/versions_map
🧰 Additional context used
🪛 yamllint (1.35.1)
packages/apps/redis/templates/redisfailover.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (4)
packages/apps/redis/templates/dashboard-resourcemap.yaml (1)
16-22: LGTM! Well-scoped RBAC permissions
The RBAC rule follows security best practices:
- Properly scoped to specific secret using Release.Name
- Read-only permissions (get, list, watch)
- Follows principle of least privilege
packages/apps/redis/values.schema.json (1)
24-28: Verify Redis configuration and secret handling
Please ensure:
- The Redis configuration properly uses the generated password
- Secrets are properly managed in case of updates/deletion
✅ Verification successful
Let me gather more information about the Redis configuration to ensure the password is properly used.
Let me check one more aspect to understand how Redis configuration is structured and how authentication is implemented.
Based on all the gathered information, I can now provide the final response:
Redis authentication is properly configured and secure
The implementation correctly handles Redis authentication:
- Password is securely generated using
randAlphaNumand stored as a base64-encoded secret - Existing passwords are preserved during updates through secret lookup
- Secret is properly managed in a Kubernetes Secret resource named
{Release.Name}-auth - Access to the auth secret is properly restricted through RBAC in the dashboard resource map
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Redis password is properly configured in templates
rg -l "requirepass|masterauth" packages/apps/redis/templates/
# Check for potential password/secret exposure in logs or env vars
rg -i "password|secret" packages/apps/redis/templates/ -A 3
Length of output: 1731
Script:
#!/bin/bash
# Check Redis configuration settings in templates
rg -A 5 "redis.conf|redisConfiguration" packages/apps/redis/templates/
# Check how the auth secret is used in Redis configuration
rg -A 5 "auth.*secret|secret.*auth" packages/apps/redis/templates/
# Look for Redis configuration map or similar resources
fd -e yaml -e yml . packages/apps/redis/templates/ --exec grep -l "ConfigMap\|redis.conf"
Length of output: 385
Script:
#!/bin/bash
# Check Redis failover configuration and auth settings
rg -A 10 "spec:" packages/apps/redis/templates/redisfailover.yaml
# Look for Redis configuration settings in the entire template directory
rg -A 5 "redis:" packages/apps/redis/templates/
# Check for any environment variables or configuration related to Redis auth
rg -A 5 "env:|command:|args:" packages/apps/redis/templates/redisfailover.yaml
Length of output: 1259
packages/apps/redis/templates/redisfailover.yaml (2)
2-6: Add validation for existing password format
When reusing an existing secret, we should validate that the password is properly base64 encoded and meets minimum length requirements.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
Line range hint 1-76: Verify Redis client compatibility
The addition of authentication requires all Redis clients to support authentication. We should verify that all internal services using Redis can handle authenticated connections.
✅ Verification successful
Based on the search results, I can see that there is one Redis client usage in the codebase at packages/system/dashboard/charts/kubeapps/templates/kubeappsapis/deployment.yaml which already properly handles authentication by retrieving the password from a secret:
- name: REDIS_PASSWORD
valueFrom:
secretKeyRef:
key: redis-password
name: {{ include "kubeapps.redis.secretName" . }}Let me do one final check to ensure we haven't missed any Redis client usage:
Based on the comprehensive analysis of the codebase, I can now provide a final response:
Redis authentication is properly handled in all client connections
The only Redis client usage found in the codebase is in packages/system/dashboard/charts/kubeapps/templates/kubeappsapis/deployment.yaml, which already correctly handles authentication by retrieving the password from the Redis secret. The client is configured with:
- Proper secret reference for authentication
- Correct connection details including host and port
- Appropriate environment variable setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Redis client usage in the codebase
rg -l 'redis\.(Redis|StrictRedis|from_url|createClient)' | \
while read -r file; do
echo "Checking $file for auth handling..."
rg -A 5 -B 5 'redis\.(Redis|StrictRedis|from_url|createClient)' "$file"
done
Length of output: 73
Script:
#!/bin/bash
# Search for Redis client usage with broader patterns
rg -l -g '!*.yaml' -g '!*.yml' 'redis\..*connect|redis\..*client|createClient|Redis\.create|new Redis|redis\.connect' | \
while read -r file; do
echo "=== $file ==="
rg -A 5 -B 5 'redis\..*connect|redis\..*client|createClient|Redis\.create|new Redis|redis\.connect' "$file"
done
# Also check for Redis connection strings/URLs
rg -l 'redis://' | \
while read -r file; do
echo "=== $file ==="
rg -A 2 -B 2 'redis://' "$file"
done
Length of output: 740
Script:
#!/bin/bash
# Let's try searching for Redis-related configuration or connection setup
rg -l -g '!*.yaml' -g '!*.yml' 'redis.*config|redis.*options|redis.*connection|REDIS_|redis.*password|redis.*auth' | \
while read -r file; do
echo "=== $file ==="
rg -A 5 -B 5 'redis.*config|redis.*options|redis.*connection|REDIS_|redis.*password|redis.*auth' "$file"
done
# Also search for Redis-related environment variables in deployment files
rg -l -g '*.yaml' -g '*.yml' 'REDIS_' | \
while read -r file; do
echo "=== $file ==="
rg -A 5 -B 5 'REDIS_' "$file"
done
Length of output: 70049
Script:
#!/bin/bash
# Search for Redis connection strings or client initialization patterns in all files
rg -g '!*.yaml' -g '!*.yml' -g '!*.md' 'redis.*connect|redis.*client|createClient|Redis\.create|new Redis|redis\.connect|redis://' | \
while read -r file; do
echo "=== $file ==="
rg -A 5 -B 5 'redis.*connect|redis.*client|createClient|Redis\.create|new Redis|redis\.connect|redis://' "$file"
done
Length of output: 23870
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
| @@ -19,5 +19,6 @@ Service utilizes the Spotahome Redis Operator for efficient management and orche | |||
| | `size` | Persistent Volume size | `1Gi` | | |||
| | `replicas` | Number of Redis replicas | `2` | | |||
| | `storageClass` | StorageClass used to store the data | `""` | | |||
| | `authEnabled` | Enable password generation | `true` | | |||
There was a problem hiding this comment.
Warning: Breaking change for existing deployments
Setting authEnabled: true by default will require all existing Redis clients to be updated with authentication credentials. Consider:
- Making it opt-in (
false) by default for backward compatibility - Adding a migration guide for existing deployments
| {{- if .Values.authEnabled }} | ||
| {{- $existingPassword := lookup "v1" "Secret" .Release.Namespace (printf "%s-auth" .Release.Name) }} | ||
| {{- $password := randAlphaNum 16 | b64enc }} | ||
| {{- if $existingPassword }} | ||
| {{- $password = index $existingPassword.data "password" }} | ||
| {{- else }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ .Release.Name }}-auth | ||
| data: | ||
| password: {{ $password }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix template indentation issues
The template has inconsistent indentation which could cause YAML parsing issues. The static analysis tool correctly identified this.
Apply this fix:
{{- if .Values.authEnabled }}
{{- $existingPassword := lookup "v1" "Secret" .Release.Namespace (printf "%s-auth" .Release.Name) }}
{{- $password := randAlphaNum 16 | b64enc }}
{{- if $existingPassword }}
{{- $password = index $existingPassword.data "password" }}
{{- else }}
---
apiVersion: v1
kind: Secret
metadata:
name: {{ .Release.Name }}-auth
data:
password: {{ $password }}
{{- end }}
{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if .Values.authEnabled }} | |
| {{- $existingPassword := lookup "v1" "Secret" .Release.Namespace (printf "%s-auth" .Release.Name) }} | |
| {{- $password := randAlphaNum 16 | b64enc }} | |
| {{- if $existingPassword }} | |
| {{- $password = index $existingPassword.data "password" }} | |
| {{- else }} | |
| --- | |
| apiVersion: v1 | |
| kind: Secret | |
| metadata: | |
| name: {{ .Release.Name }}-auth | |
| data: | |
| password: {{ $password }} | |
| {{- end }} | |
| {{- end }} | |
| {{- if .Values.authEnabled }} | |
| {{- $existingPassword := lookup "v1" "Secret" .Release.Namespace (printf "%s-auth" .Release.Name) }} | |
| {{- $password := randAlphaNum 16 | b64enc }} | |
| {{- if $existingPassword }} | |
| {{- $password = index $existingPassword.data "password" }} | |
| {{- else }} | |
| --- | |
| apiVersion: v1 | |
| kind: Secret | |
| metadata: | |
| name: {{ .Release.Name }}-auth | |
| data: | |
| password: {{ $password }} | |
| {{- end }} | |
| {{- end }} |
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/apps/redis/templates/redisfailover.yaml (2)
8-13: Enhance Secret metadata for better managementConsider adding standard labels and annotations for better secret management:
metadata: name: {{ .Release.Name }}-auth + labels: + app.kubernetes.io/instance: {{ .Release.Name }} + app.kubernetes.io/managed-by: {{ .Release.Service }} + annotations: + "helm.sh/hook": "pre-install,pre-upgrade" + "helm.sh/hook-weight": "-5"
Line range hint
1-75: Consider comprehensive Redis security configurationWhile adding password authentication is good, consider these additional security aspects:
- Sentinel authentication configuration might be needed
- Verify that Redis exporter can still access metrics with auth enabled
- Consider adding TLS configuration for encrypted communication
Would you like assistance in implementing any of these additional security measures?
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/apps/redis/templates/redisfailover.yaml(2 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
packages/apps/redis/templates/redisfailover.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (3)
packages/apps/redis/templates/redisfailover.yaml (3)
1-6: Fix template indentation issues
The template has inconsistent indentation which could cause YAML parsing issues.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
2-5: Consider adding validation for existing password
When retrieving an existing password, there's no validation of its format or encoding. Consider adding checks to ensure the retrieved password meets the required format.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 3-3: wrong indentation: expected 0 but found 2
(indentation)
[warning] 4-4: wrong indentation: expected 0 but found 2
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 4
(indentation)
72-75: Verify Redis operator compatibility with auth configuration
Please ensure that the Redis operator (databases.spotahome.com/v1) supports the auth configuration as implemented. The operator needs to properly handle the secret and configure Redis authentication.
✅ Verification successful
Let me generate another verification script to specifically check the Redis operator CRD for auth configuration support.
Redis operator supports the auth configuration as implemented
The Redis operator CRD (databases.spotahome.com_redisfailovers.yaml) explicitly defines the auth configuration with secretPath property under the auth field, which matches exactly with the implementation in the template. The CRD documentation confirms that "AuthSettings contains settings about auth", validating that the authentication configuration is properly supported.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for documentation or examples of auth configuration
rg -A 5 'auth:|authentication' --type yaml
# Check for any existing Redis auth configurations
ast-grep --pattern 'auth:
secretPath: $_'
Length of output: 65626
Script:
#!/bin/bash
# Search for auth configuration in Redis operator CRD
rg -A 10 'auth:' packages/system/redis-operator/charts/redis-operator/crds/
Length of output: 1615
Summary by CodeRabbit
New Features
authEnabledparameter for enabling password generation in Redis.Bug Fixes
0.3.1to0.4.0.Documentation
authEnabledparameter description.Chores