-
Notifications
You must be signed in to change notification settings - Fork 33
feat(gateway): update admin password handling and add secret for cont… #629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdmin password management is restructured across Helm charts. The hardcoded gateway controller password is removed from inline configuration, moved to a Kubernetes Secret resource that is created conditionally, and injected into the deployment via environment variables using the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @kubernetes/helm/gateway-helm-chart/templates/gateway/controller/secret.yaml:
- Around line 1-11: The Secret creation is conditional on
.Values.gateway.controller.auth.adminPassword in the Secret template (template
generating name via include "gateway-operator.fullname"), but the Deployment's
extraEnvFrom unconditionally references that secret which will break if
adminPassword is empty; update the Deployment template where extraEnvFrom is
rendered (the block that iterates over extraEnvFrom and emits secretRef with tpl
.secretRef.name) to either add optional: true to the secretRef entries or wrap
the entire extraEnvFrom block in the same condition
(.Values.gateway.controller.auth.adminPassword) so the secret is only referenced
when created; ensure you update the envFrom rendering logic that handles
.secretRef to include optional: true or to be guarded by the adminPassword
check.
🧹 Nitpick comments (1)
kubernetes/helm/gateway-helm-chart/values.yaml (1)
408-410: Verify default extraEnvFrom doesn't cause issues when customizing.The default
extraEnvFromincludes the controller-auth secretRef. If users want to add their ownextraEnvFromentries while keeping the secret injection, they need to include both. Consider documenting this behavior, or providing a separate dedicated field for the auth secret reference to avoid accidental override.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/operator-integration-test.ymlkubernetes/helm/gateway-helm-chart/templates/gateway/controller/deployment.yamlkubernetes/helm/gateway-helm-chart/templates/gateway/controller/secret.yamlkubernetes/helm/gateway-helm-chart/values.yamlkubernetes/helm/operator-helm-chart/values.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tharindu1st
Repo: wso2/api-platform PR: 514
File: gateway/gateway-controller/config/config.yaml:0-0
Timestamp: 2025-12-19T06:57:38.504Z
Learning: In gateway/gateway-controller/config/config.yaml, the default admin user with plaintext password "admin" is intentionally provided for testing purposes only and must be changed to secure hashed passwords for production deployments.
📚 Learning: 2025-12-19T06:57:38.504Z
Learnt from: tharindu1st
Repo: wso2/api-platform PR: 514
File: gateway/gateway-controller/config/config.yaml:0-0
Timestamp: 2025-12-19T06:57:38.504Z
Learning: In gateway/gateway-controller/config/config.yaml, the default admin user with plaintext password "admin" is intentionally provided for testing purposes only and must be changed to secure hashed passwords for production deployments.
Applied to files:
kubernetes/helm/gateway-helm-chart/templates/gateway/controller/secret.yaml.github/workflows/operator-integration-test.ymlkubernetes/helm/operator-helm-chart/values.yamlkubernetes/helm/gateway-helm-chart/values.yaml
🪛 YAMLlint (1.37.1)
kubernetes/helm/gateway-helm-chart/templates/gateway/controller/secret.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (6)
kubernetes/helm/operator-helm-chart/values.yaml (2)
371-372: Acknowledge default password for testing purposes.The default
adminPassword: "admin"aligns with the documented testing convention. Based on learnings, this default is intentionally provided for testing and must be changed to a secure hashed password for production deployments. Consider adding a comment here to make this explicit for users.
463-468: extraEnvFrom configuration looks good with tpl templating.The secretRef uses a Helm template expression that will be evaluated via
tplin the deployment template. This correctly enables dynamic secret name resolution..github/workflows/operator-integration-test.yml (1)
286-288: Test configuration correctly mirrors helm chart changes.The integration test ConfigMap properly reflects the new password handling pattern: empty password in config (line 286), adminPassword field for secret creation (lines 594-595), and extraEnvFrom for secret injection (lines 686-688).
Also applies to: 594-595, 686-688
kubernetes/helm/gateway-helm-chart/templates/gateway/controller/deployment.yaml (1)
95-98: Good use oftplfor dynamic template evaluation.Using
tpl (toYaml .) $correctly enables Helm template expressions (like{{ include "gateway-operator.fullname" . }}) within theextraEnvFromvalues to be evaluated. The root context$is properly passed to ensure template functions have access to the full chart context.kubernetes/helm/gateway-helm-chart/values.yaml (2)
31-33: Password field cleared as expected.The password is now set to empty string in the config, with the actual value to be injected via the
GATEWAY_GATEWAY__CONTROLLER_AUTH_BASIC_USERS_0_PASSWORDenvironment variable from the controller-auth secret.
313-314: Default admin password for testing.Based on learnings, this default "admin" password is intentionally provided for testing purposes. Users must change this to a secure value (preferably hashed) for production deployments.
| {{- if .Values.gateway.controller.auth.adminPassword }} | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ include "gateway-operator.fullname" . }}-controller-auth | ||
| labels: | ||
| {{- include "gateway-operator.componentLabels" (list . "controller" (dict)) | nindent 4 }} | ||
| type: Opaque | ||
| stringData: | ||
| GATEWAY_GATEWAY__CONTROLLER_AUTH_BASIC_USERS_0_PASSWORD: {{ .Values.gateway.controller.auth.adminPassword | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential deployment failure when adminPassword is empty or unset.
The Secret is created conditionally ({{- if .Values.gateway.controller.auth.adminPassword }}), but extraEnvFrom in values.yaml unconditionally references this secret. If a user sets adminPassword to empty or removes it, the deployment will fail because Kubernetes cannot find the referenced secret.
Consider one of these fixes:
- Make the
secretRefoptional in the deployment template - Also make
extraEnvFromconditional onadminPasswordin the deployment template - Document that
adminPasswordmust be set (even if to a custom value)
🔎 Proposed fix: Make secretRef optional
In deployment.yaml, modify the envFrom rendering to add optional: true:
{{- with $deployment.extraEnvFrom }}
envFrom:
{{- range . }}
{{- if .secretRef }}
- secretRef:
name: {{ tpl .secretRef.name $ }}
optional: true
{{- else }}
- {{- tpl (toYaml .) $ | nindent 12 }}
{{- end }}
{{- end }}
{{- end }}Alternatively, wrap the extraEnvFrom block with the same condition:
+{{- if .Values.gateway.controller.auth.adminPassword }}
{{- with $deployment.extraEnvFrom }}
envFrom:
{{- tpl (toYaml .) $ | nindent 12 }}
{{- end }}
+{{- end }}Note: The YAMLlint syntax error is a false positive—Helm template syntax {{- is valid and will be processed correctly.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In @kubernetes/helm/gateway-helm-chart/templates/gateway/controller/secret.yaml
around lines 1-11, The Secret creation is conditional on
.Values.gateway.controller.auth.adminPassword in the Secret template (template
generating name via include "gateway-operator.fullname"), but the Deployment's
extraEnvFrom unconditionally references that secret which will break if
adminPassword is empty; update the Deployment template where extraEnvFrom is
rendered (the block that iterates over extraEnvFrom and emits secretRef with tpl
.secretRef.name) to either add optional: true to the secretRef entries or wrap
the entire extraEnvFrom block in the same condition
(.Values.gateway.controller.auth.adminPassword) so the secret is only referenced
when created; ensure you update the envFrom rendering logic that handles
.secretRef to include optional: true or to be guarded by the adminPassword
check.
…roller auth
Purpose
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.