This repository was archived by the owner on Sep 9, 2026. It is now read-only.
OSAC-1564: Create AAP config-as-code secrets in Helm mode - #283
Merged
openshift-merge-bot[bot] merged 2 commits intoJun 17, 2026
Merged
openshift-merge-bot[bot] merged 2 commits into
openshift-merge-bot[bot] merged 2 commits into
Conversation
|
@eliorerz: This pull request references OSAC-1564 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Walkthrough
ChangesAAP Secret Auto-Creation for Helm Mode
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 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: 2
🤖 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 `@scripts/setup.sh`:
- Around line 347-358: The oc create secret command in the config-as-code-ig
secret creation (lines 353-358) uses double quotes around variable expansion
with --from-literal arguments, which allows shell command substitution if
environment variables contain malicious content. To fix this vulnerability, use
printf or a here-doc approach to pass literal values to the oc create secret
command without allowing shell expansion of the AAP_EE_IMAGE,
AAP_PROJECT_GIT_URI, AAP_PROJECT_GIT_BRANCH, and INSTALLER_NAMESPACE variables.
This prevents attackers from injecting arbitrary shell commands through these
environment variables.
- Around line 326-345: The AAP_LICENSE_FILE variable can be set to an arbitrary
path through environment control, enabling information disclosure of any file
readable by the script. Add validation before the file existence check to ensure
AAP_LICENSE_FILE points to a safe location within the expected repository
structure. Specifically, add a check that validates the resolved path does not
escape the overlays directory (e.g., using realpath to resolve symlinks and
normalize paths, then verify it stays within overlays/), and reject paths
containing traversal sequences like ../. This validation must occur before the
existing if [[ -f "${AAP_LICENSE_FILE}" ]] check to prevent unauthorized file
reads.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a1062eac-8371-4adb-8385-946d659b9840
📒 Files selected for processing (3)
charts/osac/values-example.yamldocs/helm-deployment-guide.mdscripts/setup.sh
Comment on lines
+326
to
+345
| # Create AAP license secret (required by the bootstrap job). | ||
| # In kustomize mode this is handled by secretGenerator; in Helm mode we | ||
| # must create it explicitly. The license.zip can be provided via: | ||
| # 1. AAP_LICENSE_FILE env var (absolute path) | ||
| # 2. overlays/<overlay>/files/license.zip (default convention) | ||
| AAP_LICENSE_FILE=${AAP_LICENSE_FILE:-"overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/license.zip"} | ||
| if [[ -f "${AAP_LICENSE_FILE}" ]]; then | ||
| echo "Creating config-as-code-manifest-ig secret from ${AAP_LICENSE_FILE}..." | ||
| oc create secret generic config-as-code-manifest-ig \ | ||
| --from-file=license.zip="${AAP_LICENSE_FILE}" \ | ||
| -n "${INSTALLER_NAMESPACE}" \ | ||
| --dry-run=client -o yaml | oc apply -f - | ||
| oc label secret config-as-code-manifest-ig \ | ||
| osac.openshift.io/project=osac-aap \ | ||
| -n "${INSTALLER_NAMESPACE}" --overwrite | ||
| else | ||
| echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}" | ||
| echo "The AAP bootstrap job will fail without it." | ||
| echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/" | ||
| fi |
There was a problem hiding this comment.
Path traversal risk in license file handling.
The AAP_LICENSE_FILE variable defaults to a path constructed from the user-controlled INSTALLER_KUSTOMIZE_OVERLAY environment variable. An attacker who controls this environment variable can set AAP_LICENSE_FILE to an arbitrary path (e.g., /etc/passwd or ../../../sensitive-file), causing the script to read that file's content and embed it in the config-as-code-manifest-ig secret. While oc create secret --from-file does not execute commands, this enables information disclosure of arbitrary files readable by the script's user.
Risk severity: Medium
Impact: Information disclosure; attacker can exfiltrate file contents from the deployment environment into a Kubernetes secret.
🛡️ Recommended mitigation
Validate that AAP_LICENSE_FILE points to an expected location before using it:
AAP_LICENSE_FILE=${AAP_LICENSE_FILE:-"overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/license.zip"}
+# Validate license file path to prevent path traversal
+if [[ "${AAP_LICENSE_FILE}" =~ \.\. ]] || [[ "${AAP_LICENSE_FILE}" == /* && ! "${AAP_LICENSE_FILE}" =~ ^/tmp/ ]]; then
+ echo "ERROR: AAP_LICENSE_FILE path is invalid or potentially unsafe: ${AAP_LICENSE_FILE}"
+ exit 1
+fi
if [[ -f "${AAP_LICENSE_FILE}" ]]; thenAlternatively, enforce that the file must be within the repository:
AAP_LICENSE_FILE=${AAP_LICENSE_FILE:-"overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/license.zip"}
+# Resolve to canonical path and ensure it's under the repo root
+CANONICAL_LICENSE=$(realpath -m "${AAP_LICENSE_FILE}" 2>/dev/null || echo "")
+REPO_ROOT=$(realpath "${SCRIPT_DIR}/..")
+if [[ -z "${CANONICAL_LICENSE}" ]] || [[ ! "${CANONICAL_LICENSE}" =~ ^"${REPO_ROOT}" ]]; then
+ echo "ERROR: AAP_LICENSE_FILE must be within the repository directory"
+ exit 1
+fi
+AAP_LICENSE_FILE="${CANONICAL_LICENSE}"
if [[ -f "${AAP_LICENSE_FILE}" ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Create AAP license secret (required by the bootstrap job). | |
| # In kustomize mode this is handled by secretGenerator; in Helm mode we | |
| # must create it explicitly. The license.zip can be provided via: | |
| # 1. AAP_LICENSE_FILE env var (absolute path) | |
| # 2. overlays/<overlay>/files/license.zip (default convention) | |
| AAP_LICENSE_FILE=${AAP_LICENSE_FILE:-"overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/license.zip"} | |
| if [[ -f "${AAP_LICENSE_FILE}" ]]; then | |
| echo "Creating config-as-code-manifest-ig secret from ${AAP_LICENSE_FILE}..." | |
| oc create secret generic config-as-code-manifest-ig \ | |
| --from-file=license.zip="${AAP_LICENSE_FILE}" \ | |
| -n "${INSTALLER_NAMESPACE}" \ | |
| --dry-run=client -o yaml | oc apply -f - | |
| oc label secret config-as-code-manifest-ig \ | |
| osac.openshift.io/project=osac-aap \ | |
| -n "${INSTALLER_NAMESPACE}" --overwrite | |
| else | |
| echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}" | |
| echo "The AAP bootstrap job will fail without it." | |
| echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/" | |
| fi | |
| # Create AAP license secret (required by the bootstrap job). | |
| # In kustomize mode this is handled by secretGenerator; in Helm mode we | |
| # must create it explicitly. The license.zip can be provided via: | |
| # 1. AAP_LICENSE_FILE env var (absolute path) | |
| # 2. overlays/<overlay>/files/license.zip (default convention) | |
| AAP_LICENSE_FILE=${AAP_LICENSE_FILE:-"overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/license.zip"} | |
| # Validate license file path to prevent path traversal | |
| if [[ "${AAP_LICENSE_FILE}" =~ \.\. ]] || [[ "${AAP_LICENSE_FILE}" == /* && ! "${AAP_LICENSE_FILE}" =~ ^/tmp/ ]]; then | |
| echo "ERROR: AAP_LICENSE_FILE path is invalid or potentially unsafe: ${AAP_LICENSE_FILE}" | |
| exit 1 | |
| fi | |
| if [[ -f "${AAP_LICENSE_FILE}" ]]; then | |
| echo "Creating config-as-code-manifest-ig secret from ${AAP_LICENSE_FILE}..." | |
| oc create secret generic config-as-code-manifest-ig \ | |
| --from-file=license.zip="${AAP_LICENSE_FILE}" \ | |
| -n "${INSTALLER_NAMESPACE}" \ | |
| --dry-run=client -o yaml | oc apply -f - | |
| oc label secret config-as-code-manifest-ig \ | |
| osac.openshift.io/project=osac-aap \ | |
| -n "${INSTALLER_NAMESPACE}" --overwrite | |
| else | |
| echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}" | |
| echo "The AAP bootstrap job will fail without it." | |
| echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/" | |
| fi |
🤖 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 `@scripts/setup.sh` around lines 326 - 345, The AAP_LICENSE_FILE variable can
be set to an arbitrary path through environment control, enabling information
disclosure of any file readable by the script. Add validation before the file
existence check to ensure AAP_LICENSE_FILE points to a safe location within the
expected repository structure. Specifically, add a check that validates the
resolved path does not escape the overlays directory (e.g., using realpath to
resolve symlinks and normalize paths, then verify it stays within overlays/),
and reject paths containing traversal sequences like ../. This validation must
occur before the existing if [[ -f "${AAP_LICENSE_FILE}" ]] check to prevent
unauthorized file reads.
setup.sh in Helm mode did not create the config-as-code-manifest-ig and config-as-code-ig secrets, causing the AAP bootstrap job to fail. These were only created by kustomize secretGenerator in the legacy path. Add explicit secret creation to the Helm block before helm upgrade --install, with env var overrides (AAP_LICENSE_FILE, AAP_EE_IMAGE, AAP_PROJECT_GIT_URI, AAP_PROJECT_GIT_BRANCH) and idempotent apply. Generated by agent-teams skill
eliorerz
force-pushed
the
fix/OSAC-1564-helm-aap-secrets
branch
from
June 16, 2026 15:09
ab7001a to
f513012
Compare
Use 'helm dependency update' instead of 'helm dependency build' so that Chart.lock is regenerated from Chart.yaml. 'build' fails when the lock file is out of sync with the dependencies (e.g. after submodule updates), while 'update' resolves and locks fresh versions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/setup.sh`:
- Around line 341-344: The warning message on lines 342-343 in the setup.sh
script incorrectly states that the AAP bootstrap job will always fail without
the AAP_LICENSE_FILE, but this is not true if the config-as-code-manifest-ig
secret already exists in the namespace from a prior run. Update the warning
messages to clarify that the job will only fail if the license file is missing
AND the secret does not already exist in the namespace, so users understand when
the warning is actually actionable versus when the bootstrap can proceed with an
existing secret.
🪄 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: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: f9e01fed-76e1-41d9-88e6-2dc88c49108b
📒 Files selected for processing (3)
charts/osac/values-example.yamldocs/helm-deployment-guide.mdscripts/setup.sh
Comment on lines
+341
to
+344
| else | ||
| echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}" | ||
| echo "The AAP bootstrap job will fail without it." | ||
| echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/" |
There was a problem hiding this comment.
Avoid a false failure warning when the secret already exists.
Line 342/343 states the bootstrap job will fail whenever AAP_LICENSE_FILE is missing, but that is incorrect if config-as-code-manifest-ig is already present in the namespace from a prior run.
Suggested fix
- else
- echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}"
- echo "The AAP bootstrap job will fail without it."
- echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/"
- fi
+ elif oc get secret config-as-code-manifest-ig -n "${INSTALLER_NAMESPACE}" &>/dev/null; then
+ echo "AAP license file not found at ${AAP_LICENSE_FILE}; using existing config-as-code-manifest-ig secret."
+ else
+ echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}"
+ echo "The AAP bootstrap job will fail without it."
+ echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| else | |
| echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}" | |
| echo "The AAP bootstrap job will fail without it." | |
| echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/" | |
| elif oc get secret config-as-code-manifest-ig -n "${INSTALLER_NAMESPACE}" &>/dev/null; then | |
| echo "AAP license file not found at ${AAP_LICENSE_FILE}; using existing config-as-code-manifest-ig secret." | |
| else | |
| echo "WARNING: AAP license file not found at ${AAP_LICENSE_FILE}" | |
| echo "The AAP bootstrap job will fail without it." | |
| echo "Set AAP_LICENSE_FILE or place license.zip in overlays/${INSTALLER_KUSTOMIZE_OVERLAY}/files/" | |
| fi |
🤖 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 `@scripts/setup.sh` around lines 341 - 344, The warning message on lines
342-343 in the setup.sh script incorrectly states that the AAP bootstrap job
will always fail without the AAP_LICENSE_FILE, but this is not true if the
config-as-code-manifest-ig secret already exists in the namespace from a prior
run. Update the warning messages to clarify that the job will only fail if the
license file is missing AND the secret does not already exist in the namespace,
so users understand when the warning is actually actionable versus when the
bootstrap can proceed with an existing secret.
ajamias
pushed a commit
to ajamias/osac-installer
that referenced
this pull request
Jun 17, 2026
…aap-secrets OSAC-1564: Create AAP config-as-code secrets in Helm mode
This was referenced Jun 22, 2026
Merged
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
setup.shin Helm mode (DEPLOY_MODE=helm) did not create theconfig-as-code-manifest-igandconfig-as-code-igsecrets, causing the AAP bootstrap job to failsecretGeneratorin the legacy pathoc create secretcommands to the Helm block insetup.sh, beforehelm upgrade --install, matching the manual steps documented in the Helm deployment guide (sections 2.4-2.5)Changes
scripts/setup.sh— Add license secret (config-as-code-manifest-ig) and config-as-code secret (config-as-code-ig) creation in Helm mode, with env var overrides (AAP_LICENSE_FILE,AAP_EE_IMAGE,AAP_PROJECT_GIT_URI,AAP_PROJECT_GIT_BRANCH) and idempotent applydocs/helm-deployment-guide.md— Note thatsetup.shnow handles these secrets automatically; manual commands kept for referencecharts/osac/values-example.yaml— Fix inaccurate comment (previously saidaap-configuration.shcreated these secrets, which it does not)Test plan
setup.shwithDEPLOY_MODE=helmand a validlicense.zip— verify both secrets are created in the namespace before Helm installsetup.shwithDEPLOY_MODE=helmwithoutlicense.zip— verify warning is emitted and script continuesDEPLOY_MODE=kustomizestill works (no regression)AAP_LICENSE_FILE,AAP_EE_IMAGE) workFixes: https://redhat.atlassian.net/browse/OSAC-1564
Generated by agent-teams skill
Summary by CodeRabbit
Documentation
New Features