feat: get password from existing secret#238
Conversation
fallback to default values update README.md
📝 WalkthroughWalkthroughThe Helm chart gains a ChangesDatabase existingSecret support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![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
🤖 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 `@deployments/helm-chart/README.md`:
- Around line 55-56: Update the default values in the README table for the
existingSecret fields to match the actual defaults applied by the helm template.
Change the default for database.existingSecret.secretName from "" to
"<fullname>-secrets" and the default for database.existingSecret.passwordKey
from "" to "database-password" to accurately reflect the effective behavior when
these values are unset in the template.
In `@deployments/helm-chart/templates/deployment.yaml`:
- Around line 66-70: The DATABASE__PASSWORD secret reference is inconsistent
across the deployment specifications: the anchor location (lines 66-70)
correctly uses the fallback format `printf "%s-secret"` with the
database.existingSecret values, but the sibling location (lines 204-208) uses a
hardcoded secret name that ignores the database.existingSecret configuration and
uses a different naming pattern. Update the sibling deployment secret reference
to match the anchor location by replacing the hardcoded secret name with the
same template logic that uses the printf "%s-secret" format and respects the
Values.database.existingSecret.secretName and
database.existingSecret.passwordKey values, ensuring both deployments follow the
same backward-compatible secret resolution behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e01d88be-b3bc-4a9c-90f7-542bd1cce729
📒 Files selected for processing (3)
deployments/helm-chart/README.mddeployments/helm-chart/templates/deployment.yamldeployments/helm-chart/values.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (2)
deployments/helm-chart/templates/deployment.yaml (2)
66-70:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFallback secret name is inconsistent with worker deployment.
Line 69 uses
"%s-secret"(singular) as the fallback, but the worker deployment at line 207 references"%s-secrets"(plural). Whendatabase.existingSecret.secretNameis not set, the main deployment will look for a different secret than the worker, causing one or both pods to fail to start.The fallback should use
"%s-secrets"(plural) to maintain backward compatibility with the existing hardcoded reference.🔧 Proposed fix
- name: DATABASE__PASSWORD valueFrom: secretKeyRef: - name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secret" (include "sms-gateway.fullname" .)) }} + name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secrets" (include "sms-gateway.fullname" .)) }} key: {{ .Values.database.existingSecret.passwordKey | default "database-password" }}🤖 Prompt for 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. In `@deployments/helm-chart/templates/deployment.yaml` around lines 66 - 70, The fallback secret name in the database password valueFrom secretKeyRef uses the singular format "%s-secret" but this is inconsistent with the worker deployment which references "%s-secrets" (plural). To fix this, change the fallback value in the printf template from "%s-secret" to "%s-secrets" to match the worker deployment and maintain backward compatibility when database.existingSecret.secretName is not explicitly set.
204-208:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winWorker deployment ignores
database.existingSecretconfiguration.Lines 204-208 use a hardcoded secret reference that doesn't respect the
database.existingSecretconfiguration. When users configuredatabase.existingSecret.secretName, the main deployment will successfully connect to the database, but the worker deployment will fail because it's still looking for the hardcoded secret.Apply the same template logic used in the main deployment (lines 66-70) to ensure both deployments respect the
database.existingSecretconfiguration.🔧 Proposed fix
- name: DATABASE__PASSWORD valueFrom: secretKeyRef: - name: {{ include "sms-gateway.fullname" . }}-secrets - key: database-password + name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secrets" (include "sms-gateway.fullname" .)) }} + key: {{ .Values.database.existingSecret.passwordKey | default "database-password" }}🤖 Prompt for 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. In `@deployments/helm-chart/templates/deployment.yaml` around lines 204 - 208, The DATABASE__PASSWORD environment variable definition in the worker deployment uses a hardcoded secret reference that ignores the database.existingSecret configuration. Update the secretKeyRef block in the worker deployment to apply the same conditional template logic used in the main deployment: check if database.existingSecret is configured and use either the provided existing secret name or the generated secret name (sms-gateway.fullname combined with -secrets suffix). This ensures both deployments respect user-provided database secret configurations consistently.
🤖 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.
Duplicate comments:
In `@deployments/helm-chart/templates/deployment.yaml`:
- Around line 66-70: The fallback secret name in the database password valueFrom
secretKeyRef uses the singular format "%s-secret" but this is inconsistent with
the worker deployment which references "%s-secrets" (plural). To fix this,
change the fallback value in the printf template from "%s-secret" to
"%s-secrets" to match the worker deployment and maintain backward compatibility
when database.existingSecret.secretName is not explicitly set.
- Around line 204-208: The DATABASE__PASSWORD environment variable definition in
the worker deployment uses a hardcoded secret reference that ignores the
database.existingSecret configuration. Update the secretKeyRef block in the
worker deployment to apply the same conditional template logic used in the main
deployment: check if database.existingSecret is configured and use either the
provided existing secret name or the generated secret name (sms-gateway.fullname
combined with -secrets suffix). This ensures both deployments respect
user-provided database secret configurations consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef157068-be00-4d56-9bf7-849c16aae238
📒 Files selected for processing (1)
deployments/helm-chart/templates/deployment.yaml
eiabea
force-pushed
the
allow-existing-secret-for-external-mariadb
branch
from
623cc4e to
2b4a93d
Compare
|
Hello! Thank you for your contribution! If the Please squash commits after you finish. Best Regards, |
Thank you for the hint! The latest version of the PR now also uses the existing secret for the internal database. Please check again and then i will squash everything together |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
deployments/helm-chart/templates/deployment.yaml (1)
69-70:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUse the correct fallback Secret name for both Deployments.
Line 69 and Line 207 default to
{{ include "sms-gateway.fullname" . }}-secret, but the chart-managed secret is{{ include "sms-gateway.fullname" . }}-secrets(seedeployments/helm-chart/templates/secrets.yaml, Line 4). With default values, both pods can fail with missing secret refs.Suggested fix
- name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secret" (include "sms-gateway.fullname" .)) }} + name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secrets" (include "sms-gateway.fullname" .)) }} ... - name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secret" (include "sms-gateway.fullname" .)) }} + name: {{ .Values.database.existingSecret.secretName | default (printf "%s-secrets" (include "sms-gateway.fullname" .)) }}Also applies to: 207-208
🤖 Prompt for 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. In `@deployments/helm-chart/templates/deployment.yaml` around lines 69 - 70, The deployment references to the default Secret name use an incorrect fallback suffix. In deployments/helm-chart/templates/deployment.yaml at lines 69-70 and 207-208, the code defaults the secretName to include "sms-gateway.fullname" followed by "-secret", but the actual managed secret created in secrets.yaml uses "-secrets" (plural). Update both occurrences to change the fallback from "-secret" to "-secrets" in the printf template to match the actual secret name that gets generated.
🤖 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 `@deployments/helm-chart/templates/database.yaml`:
- Around line 95-97: The guard condition at
deployments/helm-chart/templates/database.yaml line 95 incorrectly checks the
truthiness of the entire `existingSecret` map, while the fallback logic
elsewhere only checks the `secretName` field. This creates a mismatch where an
empty map or one with an empty secretName would skip password generation but
still fail later. Change the condition from `if not
.Values.database.existingSecret` to `if not
.Values.database.existingSecret.secretName` at line 95. Apply the same fix to
deployments/helm-chart/templates/secrets.yaml line 9 to ensure consistent
behavior across both templates when determining whether to generate default
passwords.
---
Duplicate comments:
In `@deployments/helm-chart/templates/deployment.yaml`:
- Around line 69-70: The deployment references to the default Secret name use an
incorrect fallback suffix. In deployments/helm-chart/templates/deployment.yaml
at lines 69-70 and 207-208, the code defaults the secretName to include
"sms-gateway.fullname" followed by "-secret", but the actual managed secret
created in secrets.yaml uses "-secrets" (plural). Update both occurrences to
change the fallback from "-secret" to "-secrets" in the printf template to match
the actual secret name that gets generated.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1bb81bd5-31a2-4fb2-9d28-1c385d87cc77
📒 Files selected for processing (3)
deployments/helm-chart/templates/database.yamldeployments/helm-chart/templates/deployment.yamldeployments/helm-chart/templates/secrets.yaml
capcom6
left a comment
capcom6
left a comment
There was a problem hiding this comment.
Please squash the commits and I will merge PR into the master.
2 participants
fallback to default values
update README.md
Context: We use mariadb-operator to create new MariaDB instance in our cluster, which expects an already deployed secret. In order to stick to our naming conventions we would like to override the name of the secret which is used by the sms-gate
Summary by CodeRabbit
New Features
database.existingSecret.secretNameanddatabase.existingSecret.passwordKey.Documentation
Bug Fixes
Chores