Conversation
Greptile SummaryThis PR adds the full
Confidence Score: 3/5Not safe to merge — multiple P1 issues would prevent a working deployment out of the box. Five distinct P1 defects: invalid cron schedules (half the cron jobs broken), session-destroying secret regeneration, invalid Kubernetes probe specs on both Postgres clusters, a silently broken email integration, and a mismatched toleration guard. charts/plural/values.yaml, charts/plural/templates/secrets.yaml, charts/plural/templates/postgres.yaml, charts/plural/templates/hydra-postgres.yaml
|
| Filename | Overview |
|---|---|
| charts/plural/values.yaml | 5 of 10 cron entries use cronTab (capital T) instead of crontab, causing those CronJobs to render with an empty schedule and fail Kubernetes validation. |
| charts/plural/templates/secrets.yaml | Two bugs: SECRET_KEY_BASE falls back to randAlphaNum 24 which regenerates on every upgrade invalidating sessions; email provider env var key has a typo (SENGRID_API_KEY missing a D). |
| charts/plural/templates/postgres.yaml | Sidecar readinessProbe contains a stray livenessProbe: key (copy-paste error), making the probe spec invalid and causing Kubernetes to reject the postgresql resource. |
| charts/plural/templates/hydra-postgres.yaml | Same readinessProbe / livenessProbe copy-paste error as postgres.yaml, producing an invalid sidecar probe spec. |
| charts/plural/templates/registry.yaml | Toleration guard checks .Values.tolerations (top-level) but renders .Values.registry.tolerations; mismatched condition can emit a null tolerations block. |
| charts/plural/templates/deployment.yaml | Adds five Deployments (api, worker, rtc, www, mcp); structure is sound with checksum annotations and topology spread support. |
| charts/plural/templates/hpa.yaml | Uses deprecated autoscaling/v2beta2 API which was removed in Kubernetes 1.26+; should be autoscaling/v2. |
| charts/plural/templates/migration.yaml | Defines migration, scan-packages, create-hydra-db, and hydra-migration jobs; logic looks correct with appropriate backoffLimits and restartPolicy. |
| charts/plural/templates/ingress.yaml | Routes API, RTC, WWW, and registry traffic via nginx ingress with TLS; paths and backends look correct. |
| charts/plural/templates/rbac.yaml | Creates Role, ClusterRole, ServiceAccount, and bindings; ClusterRole grants wildcard verbs on pods/exec which is intentional for shell access. |
Reviews (1): Last reviewed commit: "add charts to action" | Re-trigger Greptile
| - cronName: plrl-metering | ||
| cronModule: Task.Metering | ||
| cronTab: "30 1 * * *" | ||
| envVars: [] | ||
| - cronName: plrl-digest | ||
| cronModule: Digest.Pending | ||
| cronTab: "0 12 * * 1" | ||
| envVars: [] | ||
| - cronName: plrl-prune-cloud-instances | ||
| cronModule: Prune.Cloud | ||
| cronTab: "45 1 * * *" | ||
| envVars: [] | ||
|
|
||
| hydraSecrets: | ||
| dsn: memory | ||
| system: dummy | ||
| cookie: dummy |
There was a problem hiding this comment.
cronTab case mismatch breaks 5 cron schedules
The cron template uses .crontab (all lowercase) in schedule: {{ .crontab | quote }}, but five entries here use cronTab (capital T). Helm key lookups are case-sensitive, so .crontab resolves to an empty string for these entries, producing schedule: "" — an invalid cron expression that Kubernetes will reject. Affected crons: plrl-prune-trials, plrl-prune-notifs, plrl-metering, plrl-digest, plrl-prune-cloud-instances.
| DKR_DNS: {{ .Values.ingress.dkr_dns | b64enc | quote }} | ||
| JWT_SECRET: {{ .Values.secrets.jwt | b64enc | quote }} | ||
| SECRET_KEY_BASE: {{ .Values.secrets.key_base | default (randAlphaNum 24) | b64enc | quote }} | ||
| ERLANG_COOKIE: {{ .Values.secrets.erlang | b64enc | quote }} |
There was a problem hiding this comment.
SECRET_KEY_BASE regenerated on every Helm upgrade
randAlphaNum 24 is evaluated fresh on each helm upgrade, so SECRET_KEY_BASE changes whenever .Values.secrets.key_base is not explicitly set. This invalidates all active user sessions on every upgrade. The value must be stable across deployments and should be set explicitly in values or generated once and stored.
| labels: | ||
| spilo-role: master | ||
| {{ include "plural.labels" . | nindent 4 }} | ||
| spec: | ||
| type: ClusterIP | ||
| ports: | ||
| - name: postgres | ||
| port: 5432 | ||
| targetPort: 5432 | ||
| - name: http-metrics | ||
| port: 9187 | ||
| targetPort: http-metrics |
There was a problem hiding this comment.
Malformed
readinessProbe — livenessProbe key is a copy-paste artifact
The readinessProbe block contains a stray livenessProbe: key (null value) as a child field. Kubernetes will reject the sidecar spec because livenessProbe is not a valid field of a probe object. The same issue is present in hydra-postgres.yaml.
| scheme: HTTP | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 10 | ||
| successThreshold: 1 | ||
| timeoutSeconds: 5 | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: hydra-master | ||
| labels: |
| {{- end }} | ||
| {{- with .Values.registry.topologySpreadConstraints }} | ||
| topologySpreadConstraints: | ||
| {{- toYaml . | nindent 8 }} |
There was a problem hiding this comment.
Toleration guard checks the wrong value
The condition is {{- if .Values.tolerations }} (top-level tolerations), but the rendered value is .Values.registry.tolerations. Because .Values.tolerations is always non-empty in values.yaml, the block is unconditionally emitted. If .Values.registry.tolerations is empty/nil, this renders tolerations: with a null body, producing invalid YAML.
| {{ end }} | ||
| {{ if .Values.secrets.zoom_client_id }} | ||
| ZOOM_CLIENT_ID: {{ .Values.secrets.zoom_client_id | b64enc | quote }} |
There was a problem hiding this comment.
| apiVersion: autoscaling/v2beta2 | ||
| kind: HorizontalPodAutoscaler | ||
| metadata: |
Test Plan
Checklist
Plural Flow: console