Add namespace creation Job to prevent deletion on ClusterPackage removal#501
Add namespace creation Job to prevent deletion on ClusterPackage removal#501nephomaniac wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nephomaniac 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 |
|
/hold |
WalkthroughAdded a new Kubernetes/OpenShift manifest that idempotently creates and verifies the operator namespace via a Job, plus a ServiceAccount and cluster-scoped RBAC to permit namespace creation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy_pko/Create-Namespace-Job.yaml`:
- Around line 72-77: The job's command uses sh with a script that relies on the
bash-only "pipefail" option; update the command invocation and script shebang so
bash is used explicitly: change the command from "sh -c" to "bash -c" (or
equivalent) and update the script shebang from "#!/bin/sh" to "#!/bin/bash" (or
remove the shebang and rely on the invoked "bash -c"), ensuring the "set -euxo
pipefail" line runs under bash; locate the command block and inline script in
Create-Namespace-Job.yaml (the command: / - sh / - -c / - | block) and make
these substitutions.
- Around line 68-94: The container "create-namespace" currently has no
container-level securityContext; add an explicit securityContext block on that
container to harden it — set runAsNonRoot: true, runAsUser to a non-root UID
(e.g. 1000), ensure privileged: false, set allowPrivilegeEscalation: false, drop
all capabilities (capabilities.drop: ["ALL"]), enable readOnlyRootFilesystem:
true, and set seccompProfile.type to "RuntimeDefault" (and optionally set
procMount/selinuxOptions if required by environment) so the pod-level SCC is
complemented by container-level defense-in-depth.
🪄 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: b3cab60b-3c14-493a-b783-22c9b8e9035b
📒 Files selected for processing (1)
deploy_pko/Create-Namespace-Job.yaml
deploy_pko/Create-Namespace-Job.yaml
Outdated
| containers: | ||
| - name: create-namespace | ||
| image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest | ||
| imagePullPolicy: Always | ||
| command: | ||
| - sh | ||
| - -c | ||
| - | | ||
| #!/bin/sh | ||
| set -euxo pipefail | ||
|
|
||
| NS="openshift-route-monitor-operator" | ||
|
|
||
| # Create namespace if it doesn't exist (idempotent) | ||
| cat <<EOF | oc apply -f - | ||
| apiVersion: v1 | ||
| kind: Namespace | ||
| metadata: | ||
| name: ${NS} | ||
| labels: | ||
| openshift.io/cluster-monitoring: "true" | ||
| pod-security.kubernetes.io/enforce: restricted | ||
| pod-security.kubernetes.io/audit: restricted | ||
| pod-security.kubernetes.io/warn: restricted | ||
| EOF | ||
|
|
||
| echo "Namespace ${NS} created or already exists" |
There was a problem hiding this comment.
Add explicit security context to address static analysis findings.
The container lacks an explicit securityContext. While the restricted-v2 SCC annotation on the pod helps, container-level settings provide defense-in-depth and make the security posture explicit.
🛡️ Proposed fix
containers:
- name: create-namespace
image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest
imagePullPolicy: Always
+ securityContext:
+ allowPrivilegeEscalation: false
+ runAsNonRoot: true
+ capabilities:
+ drop:
+ - ALL
command:📝 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.
| containers: | |
| - name: create-namespace | |
| image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest | |
| imagePullPolicy: Always | |
| command: | |
| - sh | |
| - -c | |
| - | | |
| #!/bin/sh | |
| set -euxo pipefail | |
| NS="openshift-route-monitor-operator" | |
| # Create namespace if it doesn't exist (idempotent) | |
| cat <<EOF | oc apply -f - | |
| apiVersion: v1 | |
| kind: Namespace | |
| metadata: | |
| name: ${NS} | |
| labels: | |
| openshift.io/cluster-monitoring: "true" | |
| pod-security.kubernetes.io/enforce: restricted | |
| pod-security.kubernetes.io/audit: restricted | |
| pod-security.kubernetes.io/warn: restricted | |
| EOF | |
| echo "Namespace ${NS} created or already exists" | |
| containers: | |
| - name: create-namespace | |
| image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest | |
| imagePullPolicy: Always | |
| securityContext: | |
| allowPrivilegeEscalation: false | |
| runAsNonRoot: true | |
| capabilities: | |
| drop: | |
| - ALL | |
| command: | |
| - sh | |
| - -c | |
| - | | |
| #!/bin/sh | |
| set -euxo pipefail | |
| NS="openshift-route-monitor-operator" | |
| # Create namespace if it doesn't exist (idempotent) | |
| cat <<EOF | oc apply -f - | |
| apiVersion: v1 | |
| kind: Namespace | |
| metadata: | |
| name: ${NS} | |
| labels: | |
| openshift.io/cluster-monitoring: "true" | |
| pod-security.kubernetes.io/enforce: restricted | |
| pod-security.kubernetes.io/audit: restricted | |
| pod-security.kubernetes.io/warn: restricted | |
| EOF | |
| echo "Namespace ${NS} created or already exists" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy_pko/Create-Namespace-Job.yaml` around lines 68 - 94, The container
"create-namespace" currently has no container-level securityContext; add an
explicit securityContext block on that container to harden it — set
runAsNonRoot: true, runAsUser to a non-root UID (e.g. 1000), ensure privileged:
false, set allowPrivilegeEscalation: false, drop all capabilities
(capabilities.drop: ["ALL"]), enable readOnlyRootFilesystem: true, and set
seccompProfile.type to "RuntimeDefault" (and optionally set
procMount/selinuxOptions if required by environment) so the pod-level SCC is
complemented by container-level defense-in-depth.
| command: | ||
| - sh | ||
| - -c | ||
| - | | ||
| #!/bin/sh | ||
| set -euxo pipefail |
There was a problem hiding this comment.
Use bash instead of sh for pipefail compatibility.
The script uses set -euxo pipefail, but pipefail is a bash feature, not POSIX sh. While RHEL-based images typically symlink sh to bash, this is implicit. Using bash explicitly ensures the script works as intended.
🔧 Proposed fix
command:
- - sh
+ - /bin/bash
- -c
- |
- #!/bin/sh
+ #!/bin/bash
set -euxo pipefail📝 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.
| command: | |
| - sh | |
| - -c | |
| - | | |
| #!/bin/sh | |
| set -euxo pipefail | |
| command: | |
| - /bin/bash | |
| - -c | |
| - | | |
| #!/bin/bash | |
| set -euxo pipefail |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy_pko/Create-Namespace-Job.yaml` around lines 72 - 77, The job's command
uses sh with a script that relies on the bash-only "pipefail" option; update the
command invocation and script shebang so bash is used explicitly: change the
command from "sh -c" to "bash -c" (or equivalent) and update the script shebang
from "#!/bin/sh" to "#!/bin/bash" (or remove the shebang and rely on the invoked
"bash -c"), ensuring the "set -euxo pipefail" line runs under bash; locate the
command block and inline script in Create-Namespace-Job.yaml (the command: / -
sh / - -c / - | block) and make these substitutions.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #501 +/- ##
=======================================
Coverage 56.26% 56.26%
=======================================
Files 31 31
Lines 2851 2851
=======================================
Hits 1604 1604
Misses 1137 1137
Partials 110 110 🚀 New features to boost your workflow:
|
dab48ee to
91c4686
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
deploy_pko/Create-Namespace-Job.yaml (2)
72-117:⚠️ Potential issue | 🟠 MajorAdd explicit container
securityContextfor least privilege.The container currently relies only on SCC. Add explicit container-level hardening to prevent privilege escalation/root admission drift and address the CKV findings.
Proposed fix
containers: - name: create-namespace image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest imagePullPolicy: Always + securityContext: + allowPrivilegeEscalation: false + privileged: false + runAsNonRoot: true + capabilities: + drop: + - ALL command:As per coding guidelines "**: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy_pko/Create-Namespace-Job.yaml` around lines 72 - 117, The create-namespace container lacks an explicit container-level securityContext; add one under the container spec for the container named "create-namespace" to enforce least privilege: set runAsNonRoot: true and a non-root runAsUser (e.g. 1000), set allowPrivilegeEscalation: false, set readOnlyRootFilesystem: true, drop all capabilities (capabilities: drop: ["ALL"]), and add a seccompProfile (type: RuntimeDefault) to harden the runtime; place this securityContext block beneath the container definition for "create-namespace" so it applies to the job container regardless of cluster SCCs.
76-81:⚠️ Potential issue | 🟠 MajorUse
bashexplicitly when enablingpipefail.Line [77] runs
sh -c, but Line [81] usesset -euxo pipefail(not guaranteed in POSIXsh). This can make the Job fail before doing any namespace work.Proposed fix
command: - - sh + - /bin/bash - -c - | - #!/bin/sh + #!/bin/bash set -euxo pipefailAs per coding guidelines "**: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy_pko/Create-Namespace-Job.yaml` around lines 76 - 81, The job's command uses sh (- sh - -c) but the script sets "set -euxo pipefail", which is not portable to POSIX sh; change the command to invoke bash (use - bash - -c) and update the shebang line in the multi-line script to #!/usr/bin/env bash so that the pipefail option is supported and the set invocation in the command block (the script started under the command: section and its shebang) runs reliably.
🧹 Nitpick comments (1)
deploy_pko/Create-Namespace-Job.yaml (1)
74-75: Pin the CLI image to an immutable digest.Using
:latestwithimagePullPolicy: Alwaysmakes rollout behavior non-deterministic and increases supply-chain drift risk. Prefer a pinned digest.Proposed fix
- image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest - imagePullPolicy: Always + image: image-registry.openshift-image-registry.svc:5000/openshift/cli@sha256:<approved-digest> + imagePullPolicy: IfNotPresentAs per coding guidelines "**: -Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy_pko/Create-Namespace-Job.yaml` around lines 74 - 75, The image is using a mutable tag "image-registry.openshift-image-registry.svc:5000/openshift/cli:latest" with imagePullPolicy: Always; replace that tag with an immutable digest (sha256) to pin the CLI image and ensure deterministic rollouts. Update the "image" field to the same registry URL but with the resolved digest (e.g., `@sha256`:<digest>) and keep imagePullPolicy as Always (or change to IfNotPresent if desired), ensuring you locate and modify the exact "image: image-registry.openshift-image-registry.svc:5000/openshift/cli:latest" line and its accompanying "imagePullPolicy" entry in this manifest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@deploy_pko/Create-Namespace-Job.yaml`:
- Around line 72-117: The create-namespace container lacks an explicit
container-level securityContext; add one under the container spec for the
container named "create-namespace" to enforce least privilege: set runAsNonRoot:
true and a non-root runAsUser (e.g. 1000), set allowPrivilegeEscalation: false,
set readOnlyRootFilesystem: true, drop all capabilities (capabilities: drop:
["ALL"]), and add a seccompProfile (type: RuntimeDefault) to harden the runtime;
place this securityContext block beneath the container definition for
"create-namespace" so it applies to the job container regardless of cluster
SCCs.
- Around line 76-81: The job's command uses sh (- sh - -c) but the script sets
"set -euxo pipefail", which is not portable to POSIX sh; change the command to
invoke bash (use - bash - -c) and update the shebang line in the multi-line
script to #!/usr/bin/env bash so that the pipefail option is supported and the
set invocation in the command block (the script started under the command:
section and its shebang) runs reliably.
---
Nitpick comments:
In `@deploy_pko/Create-Namespace-Job.yaml`:
- Around line 74-75: The image is using a mutable tag
"image-registry.openshift-image-registry.svc:5000/openshift/cli:latest" with
imagePullPolicy: Always; replace that tag with an immutable digest (sha256) to
pin the CLI image and ensure deterministic rollouts. Update the "image" field to
the same registry URL but with the resolved digest (e.g., `@sha256`:<digest>) and
keep imagePullPolicy as Always (or change to IfNotPresent if desired), ensuring
you locate and modify the exact "image:
image-registry.openshift-image-registry.svc:5000/openshift/cli:latest" line and
its accompanying "imagePullPolicy" entry in this manifest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0d25046-79f9-4e6b-b37e-fc5b2e40b789
📒 Files selected for processing (1)
deploy_pko/Create-Namespace-Job.yaml
|
@nephomaniac: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This Job creates the openshift-route-monitor-operator namespace during the PKO namespace phase if it doesn't already exist. Using a Job (instead of a PKO-managed Namespace resource) ensures the namespace is created without owner references to the ClusterPackage, so it persists when the ClusterPackage is deleted. The Job runs in the 'default' namespace with cluster-level RBAC to create namespaces, following the same pattern as Cleanup-OLM-Job.yaml. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
91c4686 to
07ecca7
Compare
Summary
(POC) Adds a Job to create the operator namespace during PKO deployment, ensuring the namespace persists when the ClusterPackage is deleted.
Problem
When PKO manages a Namespace resource directly in
deploy_pko/, it creates an owner reference from the namespace to the ClusterPackage. This means when the ClusterPackage is deleted, Kubernetes garbage collection also deletes the namespace.This is problematic because:
openshift-route-monitor-operatorshould persistSolution
Create the namespace via a Job instead of a PKO-managed resource:
oc apply- idempotent, no error if existsImplementation Details
File:
deploy_pko/Create-Namespace-Job.yamldefaultnamespace (target namespace doesn't exist yet)oc applyto create namespacepackage-operator.run/phase: namespaceannotationIfNoControllercollision protectionFollows Established Pattern
This approach matches the pattern used by
Cleanup-OLM-Job.yaml:Test Plan
🤖 Generated with Claude Code