Conversation
WalkthroughAdds OpenShift-gated volumeMounts and matching emptyDir volumes for a security directory across deployments, and refactors entrypoint scripts to replace in-place permission tweaks with a guarded copy-and-sync (swap) approach using SRC_SECURITY and CONFIG_SECURITY. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
distributed/key-manager/templates/key-manager/wso2am-km-conf-entrypoint.yaml (1)
56-65: Same misleading comment as control-plane entrypoint.The comment on line 58 mentions "symlink" but no symlink is created. The code performs a copy-remove-copy pattern instead. Update the comment for accuracy:
📝 Suggested comment fix
# For permission configuration on OpenShift. echo "Openshift deployment detected." - # Copy security dir to writable emptyDir and symlink + # Copy security dir to writable emptyDir and replace original SRC_SECURITY="${WSO2_SERVER_HOME}/repository/resources/security"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@distributed/key-manager/templates/key-manager/wso2am-km-conf-entrypoint.yaml` around lines 56 - 65, The comment saying "Copy security dir to writable emptyDir and symlink" is inaccurate because the script uses a copy-remove-copy pattern, not a symlink; update the comment in wso2am-km-conf-entrypoint.yaml to accurately describe the behavior (e.g., "Copy security directory into writable emptyDir (uses copy-remove-copy to replace original)"), and reference the variables SRC_SECURITY and CONFIG_SECURITY so readers know which paths are being copied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@all-in-one/templates/am/instance-1/wso2am-deployment.yaml`:
- Around line 280-283: The template mounts a volume named "wso2am-security-dir"
in the container (in the wso2am deployment template) but never declares a
matching volume in the Pod spec, causing manifest validation to fail; add a
volume entry named "wso2am-security-dir" under the Deployment's
spec.template.spec.volumes (use an appropriate volume type such as emptyDir for
temporary security files or a PVC if persistence is required) and wrap that
volume declaration with the same condition
(.Values.kubernetes.openshift.enabled) so the name matches the volumeMount in
the container spec.
In `@all-in-one/templates/am/instance-2/wso2am-deployment.yaml`:
- Around line 287-290: The Deployment mounts a volume named
"wso2am-security-dir" when .Values.kubernetes.openshift.enabled is true but no
matching volume is declared, causing Pod creation to fail; add a corresponding
volume entry under spec.template.spec.volumes (conditioned on the same
.Values.kubernetes.openshift.enabled) with the same name "wso2am-security-dir"
(e.g., an emptyDir or appropriate volume type for your environment) so the
mountPath /home/wso2carbon/tmp-security has a declared backing volume.
In `@all-in-one/templates/am/wso2am-conf-entrypoint.yaml`:
- Around line 62-64: The current snippet copies from "${SRC_SECURITY}" into
"${CONFIG_SECURITY}" without deleting pre-existing files, which can reintroduce
stale certs/keystores; modify the block that references SRC_SECURITY and
CONFIG_SECURITY to purge or atomically replace CONFIG_SECURITY contents before
copying (for example, remove all files under "${CONFIG_SECURITY}" safely or use
a sync/copy with delete semantics), ensuring you guard destructive deletes (use
the CONFIG_SECURITY variable check) to avoid accidental removal of non-intended
paths and then perform the copy so only up-to-date security artifacts are
present.
In
`@distributed/control-plane/templates/control-plane/instance-2/wso2am-cp-deployment.yaml`:
- Around line 281-284: The template adds a container volumeMount named
wso2am-security-dir (mountPath /home/wso2carbon/tmp-security) when
.Values.kubernetes.openshift.enabled is true but never defines a matching
volume, causing pod creation to fail; update the Pod/Deployment spec's volumes
list to include a volume with name "wso2am-security-dir" (e.g., an emptyDir or
appropriate hostPath) wrapped with the same conditional
(.Values.kubernetes.openshift.enabled) so the volume exists whenever the
volumeMount in the container template is added; locate the volumes section near
the Pod/deployment spec for this container and add the named volume entry to
match the volumeMount.
In
`@distributed/control-plane/templates/control-plane/wso2am-cp-conf-entrypoint.yaml`:
- Around line 63-72: The comment mentioning a symlink is misleading—update the
comment above the security copy logic to reflect the actual behavior: it copies
the security directory into the writable emptyDir and restores it (no symlink is
created). Edit the comment that references SRC_SECURITY and CONFIG_SECURITY to
something like "Copy security dir to writable emptyDir and restore" so it
accurately describes the cp -dR / rm -rf / cp -dR sequence operating on
${SRC_SECURITY} and ${CONFIG_SECURITY}.
In `@distributed/gateway/templates/gateway/wso2am-gateway-conf-entrypoint.yaml`:
- Around line 55-57: The current sync block using SRC_SECURITY and
CONFIG_SECURITY can reintroduce stale files from an emptyDir across restarts;
modify the script inside the if branch to explicitly clear the target temporary
directory before copying back to avoid resurrecting old data — e.g., remove or
truncate contents of CONFIG_SECURITY (use safe rm -rf on "${CONFIG_SECURITY}/."
or an equivalent safe purge) then recreate it if needed, copy the authoritative
security files from SRC_SECURITY into CONFIG_SECURITY, and only then replace
SRC_SECURITY with the fresh content (preserving the existing guard that checks
SRC_SECURITY, CONFIG_SECURITY, and ! -L "${SRC_SECURITY}").
In
`@distributed/traffic-manager/templates/traffic-manager/instance-1/wso2am-tm-deployment.yaml`:
- Around line 264-267: The manifest conditionally adds a volumeMount named
"wso2am-security-dir" when .Values.kubernetes.openshift.enabled is true but does
not define a matching volume under spec.template.spec.volumes, causing invalid
Deployment errors; add a conditional volume entry (e.g., a
projected/emptyDir/secret depending on intent) to spec.template.spec.volumes
when .Values.kubernetes.openshift.enabled is true so the volumeMount
"wso2am-security-dir" in the container spec has a corresponding volume; update
the template near spec.template.spec.volumes to include a volume named
"wso2am-security-dir" guarded by the same .Values.kubernetes.openshift.enabled
check.
In
`@distributed/traffic-manager/templates/traffic-manager/instance-2/wso2am-tm-deployment.yaml`:
- Around line 265-268: The new OpenShift-only volumeMount 'wso2am-security-dir'
was added under the conditional block controlled by
.Values.kubernetes.openshift.enabled but no corresponding volumes[] entry
exists; add a matching volumes[] item (name: wso2am-security-dir) under the Pod
template spec (spec.template.spec.volumes) and conditionally render it with the
same .Values.kubernetes.openshift.enabled guard, using an emptyDir volume type
so the Pod spec validates when the mount is present.
In
`@distributed/traffic-manager/templates/traffic-manager/wso2am-tm-conf-entrypoint.yaml`:
- Around line 55-57: The copy-back step using SRC_SECURITY and CONFIG_SECURITY
can reintroduce stale files; before running cp -dR "${CONFIG_SECURITY}/."
"${SRC_SECURITY}/" ensure CONFIG_SECURITY is emptied (e.g., remove or truncate
its contents) so stale artifacts are not copied back; modify the block around
the cp/rm commands (referencing SRC_SECURITY and CONFIG_SECURITY) to delete or
clean CONFIG_SECURITY (for example rm -rf "${CONFIG_SECURITY}/." or remove
specific entries) immediately before the final cp -dR so only fresh files are
synchronized.
---
Nitpick comments:
In
`@distributed/key-manager/templates/key-manager/wso2am-km-conf-entrypoint.yaml`:
- Around line 56-65: The comment saying "Copy security dir to writable emptyDir
and symlink" is inaccurate because the script uses a copy-remove-copy pattern,
not a symlink; update the comment in wso2am-km-conf-entrypoint.yaml to
accurately describe the behavior (e.g., "Copy security directory into writable
emptyDir (uses copy-remove-copy to replace original)"), and reference the
variables SRC_SECURITY and CONFIG_SECURITY so readers know which paths are being
copied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 797c0636-d71a-4e2e-bac4-db207653bc3b
📒 Files selected for processing (13)
all-in-one/templates/am/instance-1/wso2am-deployment.yamlall-in-one/templates/am/instance-2/wso2am-deployment.yamlall-in-one/templates/am/wso2am-conf-entrypoint.yamldistributed/control-plane/templates/control-plane/instance-1/wso2am-cp-deployment.yamldistributed/control-plane/templates/control-plane/instance-2/wso2am-cp-deployment.yamldistributed/control-plane/templates/control-plane/wso2am-cp-conf-entrypoint.yamldistributed/gateway/templates/gateway/wso2am-gateway-conf-entrypoint.yamldistributed/gateway/templates/gateway/wso2am-gateway-deployment.yamldistributed/key-manager/templates/key-manager/wso2am-km-conf-entrypoint.yamldistributed/key-manager/templates/key-manager/wso2am-km-deployment.yamldistributed/traffic-manager/templates/traffic-manager/instance-1/wso2am-tm-deployment.yamldistributed/traffic-manager/templates/traffic-manager/instance-2/wso2am-tm-deployment.yamldistributed/traffic-manager/templates/traffic-manager/wso2am-tm-conf-entrypoint.yaml
kavindasr
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Summary by CodeRabbit