helm: enhance test to verify readiness (includes DB connectivity)#308
helm: enhance test to verify readiness (includes DB connectivity)#308
Conversation
45895ee to
c657c91
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45895ee146
ℹ️ 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".
| {{- range .Values.dbInit.databases }} | ||
| DO $$ BEGIN | ||
| IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = '{{ .user }}') THEN | ||
| CREATE ROLE {{ .user }} WITH LOGIN PASSWORD '{{ .password }}'; |
There was a problem hiding this comment.
Escape db-init password before embedding it in SQL
The pre-install db-init hook interpolates {{ .password }} directly inside a SQL string literal ('...'). If any configured password contains a single quote, the SQL becomes syntactically invalid and the hook job fails, which blocks installs/upgrades when dbInit.enabled=true. This also creates a SQL-injection footgun for values-driven provisioning; use proper literal escaping (or psql variables with quote_literal) instead of raw interpolation.
Useful? React with 👍 / 👎.
deploy/helm/codex-lb/values.yaml
Outdated
| # @param global.nodeSelector Node selector labels applied to ALL pods (deployment, jobs, tests) | ||
| nodeSelector: {} |
There was a problem hiding this comment.
Wire global nodeSelector to templates or remove the value
This commit adds global.nodeSelector and documents it as applying to all pods, but the chart templates continue reading .Values.nodeSelector (deployment, test pod, and db-init job) and never reference .Values.global.nodeSelector. Users who follow the new value contract will see no scheduling effect, which is especially problematic for ARM pinning scenarios described in the change.
Useful? React with 👍 / 👎.
Summary
/healthand/health/readyendpointsMotivation
Current test only checks
/healthwhich passes even if the database connection is broken. The/health/readyendpoint includes DB connectivity verification, catching misconfigured external database URLs.Changes
tests/test-connection.yaml: Add readiness check + nodeSelector