Helm chart improvements to support custom pod labels, annotations, node selectors and adding sidecar containers to gateway runtime pod#3324
Conversation
Co-authored-by: Copilot <copilot@github.com>
📝 WalkthroughWalkthroughThe changes update Helm chart templates and values files to support additional Kubernetes pod metadata and scheduling configuration. Specifically, the modifications add optional 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR improves the APK Helm charts’ deployment configurability by allowing users to set pod-level metadata (labels/annotations) and scheduling constraints (nodeSelectors) across key components, and by enabling injection of sidecar containers/volumes into the gateway runtime pod.
Changes:
- Added
deployment.pod.labelsanddeployment.pod.annotationsvalue hooks and rendered them into the relevant Deployment pod templates. - Added
deployment.nodeSelectorvalues for components where it wasn’t present in the defaultvalues.yaml, and rendered nodeSelectors in the relevant templates. - Added
gatewayRuntime.deployment.extraContainersandgatewayRuntime.deployment.extraVolumessupport and documented the new values in the chart README.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| helm-charts/values.yaml.template | Adds pod labels/annotations defaults (and gateway runtime extraContainers/extraVolumes defaults) for templated values. |
| helm-charts/values.yaml | Adds nodeSelector + pod labels/annotations defaults for affected components; adds gateway runtime extraContainers/extraVolumes defaults. |
| helm-charts/templates/idp/idp-ui/idp-ui-deployment.yaml | Renders pod.labels, optional pod.annotations, and optional nodeSelector for idp-ui pods. |
| helm-charts/templates/idp/idp-ds/idp-ds-deployment.yaml | Renders pod.labels, merges pod.annotations into existing annotations, and supports nodeSelector for idp-ds pods. |
| helm-charts/templates/data-plane/ratelimiter/ratelimiter-deployment.yaml | Renders pod.labels, optional pod.annotations, and supports nodeSelector for ratelimiter pods. |
| helm-charts/templates/data-plane/gateway-components/gateway-runtime/gateway-runtime-deployment.yaml | Renders pod.labels/pod.annotations, and adds support for extraContainers and extraVolumes in gateway runtime pods. |
| helm-charts/templates/data-plane/gateway-components/common-controller/common-controller-deployment.yaml | Renders pod.labels and merges pod.annotations into existing annotations for common-controller pods. |
| helm-charts/templates/data-plane/gateway-components/adapter/adapter-deployment.yaml | Renders pod.labels and merges pod.annotations into existing annotations for adapter pods. |
| helm-charts/templates/data-plane/config-deployer/config-ds-deployment.yaml | Renders pod.labels and merges pod.annotations into existing annotations for config-deployer pods. |
| helm-charts/README.md | Documents the newly added values (pod labels/annotations, nodeSelector, gateway runtime extraContainers/extraVolumes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
helm-charts/templates/data-plane/ratelimiter/ratelimiter-deployment.yaml (1)
34-40: 💤 Low valueFunctional implementation is correct; optional consistency improvement for annotations block.
The conditional
annotations:block (Lines 37–40) is the right approach here since ratelimiter has no mandatorychecksum/configannotation to anchor the block. Functionally sound.For internal consistency with the labels block (Lines 34–36), the annotations could use
{{- with }}and drop the redundant full value path:♻️ Optional refactor for consistency
-{{- if .Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations }} - annotations: -{{ toYaml .Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations | indent 8 }} -{{- end }} +{{- with .Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations }} + annotations: +{{ toYaml . | indent 8 }} +{{- end }}🤖 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 `@helm-charts/templates/data-plane/ratelimiter/ratelimiter-deployment.yaml` around lines 34 - 40, The annotations block is functionally fine but should mirror the labels block for consistency: replace the current conditional using .Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations with a `{{- with .Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations }}` block and then render `{{ toYaml . | indent 8 }}` inside it (dropping the redundant full value path) so labels and annotations use the same pattern; update the template around the annotations area to use the same `with` flow as the labels block.helm-charts/templates/data-plane/gateway-components/gateway-runtime/gateway-runtime-deployment.yaml (1)
424-426: ⚡ Quick winConsider documenting security context expectations for
extraContainers.The pod-level
seccompProfile: RuntimeDefault(Line 432) applies to all containers including injected sidecars, but the container-level hardening present onenforcerandrouter(allowPrivilegeEscalation: false,capabilities.drop: ["ALL"],readOnlyRootFilesystem: true,runAsNonRoot: true) does not propagate automatically. Users providingextraContainersmust specify these constraints explicitly in their container definitions.Adding a comment to the
extraContainerskey invalues.yaml.template(or the README) noting that sidecars should define their ownsecurityContextaligned with the pod security policy would help avoid misconfiguration.Also applies to: 502-504
🤖 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 `@helm-charts/templates/data-plane/gateway-components/gateway-runtime/gateway-runtime-deployment.yaml` around lines 424 - 426, Add documentation near the values key for extraContainers (the template variable .Values.wso2.apk.dp.gatewayRuntime.deployment.extraContainers) to warn users that pod-level seccompProfile: RuntimeDefault does not apply container-level hardening and that any injected sidecars must explicitly set a securityContext mirroring the hardening used for enforcer and router (e.g., allowPrivilegeEscalation: false, capabilities.drop: ["ALL"], readOnlyRootFilesystem: true, runAsNonRoot: true). Update the values.yaml.template (and README if present) to include this note and an example securityContext snippet so users know to add those fields to their extraContainers definitions. Ensure the comment references seccompProfile: RuntimeDefault and the enforcer/router settings so it’s clear which constraints must be replicated.
🤖 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 `@helm-charts/templates/idp/idp-ui/idp-ui-deployment.yaml`:
- Around line 34-36: The current template prints user pod labels
(.Values.idp.idpui.deployment.pod.labels) after the chart's selector labels
which allows users to override selector keys; change the template so user labels
are rendered first and the selector labels (the Deployment's
spec.selector.matchLabels keys) are rendered last so selector keys remain
authoritative. Locate the block that uses "{{- with
.Values.idp.idpui.deployment.pod.labels }} {{ toYaml . | indent 8 }} {{- end }}"
and move/merge it so it appears before the selector labels block (or ensure
selector labels are rendered after it), preserving indenting (indent 8) and YAML
formatting. Ensure no duplicate keys remain after reordering so
spec.selector.matchLabels and pod.metadata.labels stay consistent.
---
Nitpick comments:
In
`@helm-charts/templates/data-plane/gateway-components/gateway-runtime/gateway-runtime-deployment.yaml`:
- Around line 424-426: Add documentation near the values key for extraContainers
(the template variable
.Values.wso2.apk.dp.gatewayRuntime.deployment.extraContainers) to warn users
that pod-level seccompProfile: RuntimeDefault does not apply container-level
hardening and that any injected sidecars must explicitly set a securityContext
mirroring the hardening used for enforcer and router (e.g.,
allowPrivilegeEscalation: false, capabilities.drop: ["ALL"],
readOnlyRootFilesystem: true, runAsNonRoot: true). Update the
values.yaml.template (and README if present) to include this note and an example
securityContext snippet so users know to add those fields to their
extraContainers definitions. Ensure the comment references seccompProfile:
RuntimeDefault and the enforcer/router settings so it’s clear which constraints
must be replicated.
In `@helm-charts/templates/data-plane/ratelimiter/ratelimiter-deployment.yaml`:
- Around line 34-40: The annotations block is functionally fine but should
mirror the labels block for consistency: replace the current conditional using
.Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations with a `{{- with
.Values.wso2.apk.dp.ratelimiter.deployment.pod.annotations }}` block and then
render `{{ toYaml . | indent 8 }}` inside it (dropping the redundant full value
path) so labels and annotations use the same pattern; update the template around
the annotations area to use the same `with` flow as the labels block.
🪄 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: ddd0dee0-ecd2-4c41-b9c0-5d8688d4ec6f
📒 Files selected for processing (10)
helm-charts/README.mdhelm-charts/templates/data-plane/config-deployer/config-ds-deployment.yamlhelm-charts/templates/data-plane/gateway-components/adapter/adapter-deployment.yamlhelm-charts/templates/data-plane/gateway-components/common-controller/common-controller-deployment.yamlhelm-charts/templates/data-plane/gateway-components/gateway-runtime/gateway-runtime-deployment.yamlhelm-charts/templates/data-plane/ratelimiter/ratelimiter-deployment.yamlhelm-charts/templates/idp/idp-ds/idp-ds-deployment.yamlhelm-charts/templates/idp/idp-ui/idp-ui-deployment.yamlhelm-charts/values.yamlhelm-charts/values.yaml.template
| {{- with .Values.idp.idpui.deployment.pod.labels }} | ||
| {{ toYaml . | indent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Prevent user labels from overriding selector labels.
On Line 34–Line 36, custom pod labels are appended after chart selector labels. If overlapping keys are provided, template labels can diverge from spec.selector.matchLabels, which can break Deployment behavior. Render user labels first and selector labels last so selector keys remain authoritative.
Suggested change
metadata:
labels:
-{{ include "apk-helm.pod.selectorLabels" (dict "root" . "app" "idp-ui" ) | indent 8}}
{{- with .Values.idp.idpui.deployment.pod.labels }}
{{ toYaml . | indent 8 }}
{{- end }}
+{{ include "apk-helm.pod.selectorLabels" (dict "root" . "app" "idp-ui" ) | indent 8}}🤖 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 `@helm-charts/templates/idp/idp-ui/idp-ui-deployment.yaml` around lines 34 -
36, The current template prints user pod labels
(.Values.idp.idpui.deployment.pod.labels) after the chart's selector labels
which allows users to override selector keys; change the template so user labels
are rendered first and the selector labels (the Deployment's
spec.selector.matchLabels keys) are rendered last so selector keys remain
authoritative. Locate the block that uses "{{- with
.Values.idp.idpui.deployment.pod.labels }} {{ toYaml . | indent 8 }} {{- end }}"
and move/merge it so it appears before the selector labels block (or ensure
selector labels are rendered after it), preserving indenting (indent 8) and YAML
formatting. Ensure no duplicate keys remain after reordering so
spec.selector.matchLabels and pod.metadata.labels stay consistent.
This pull request enhances the configurability of the Helm charts for APK components by introducing new options for pod-level customization and deployment flexibility. The main changes add support for specifying custom pod annotations, labels, and node selectors for various components. Additionally, the gateway runtime deployment now supports injecting extra containers and volumes.
Related to -
#3321
#3323
Doc PR - wso2/docs-apk#798
Pod customization enhancements:
Added support for specifying
pod.annotationsandpod.labelsfor the following components:configdeployer,adapter,commonController,ratelimiter,gatewayRuntime,idpds, andidpuiin bothvalues.yamland their respective deployment templates, allowing for more granular pod-level metadata configuration. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20]Introduced
nodeSelectorconfiguration for the same set of components, enabling targeted scheduling of pods to specific nodes. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]Gateway runtime extensibility:
extraContainersandextraVolumesin thegatewayRuntimedeployment, allowing users to inject additional sidecar containers and volumes into the pod for advanced use cases. [1] [2] [3] [4]