Skip to content

Conversation

@rm3l
Copy link
Member

@rm3l rm3l commented Jan 15, 2026

Description of the change

  • ci: ensure stability by testing PR using the latest stable/RC version of RHDH
  • ci: add nightly workflow for testing against the unstable 'next' tag of RHDH
  • ci: refactor chart testing into reusable composite action

Which issue(s) does this PR fix or relate to

Fixes https://issues.redhat.com/browse/RHDHBUGS-2513

How to test changes / Special notes to the reviewer

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Use pre-commit run -a to apply changes. The pre-commit Workflow will do this automatically for you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Jan 15, 2026

PR Reviewer Guide 🔍

(Review updated until commit 3091e22)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 Security concerns

Supply chain / remote script execution:
The workflow downloads and executes install.sh for OLM via curl and then runs it, and also applies remote Kubernetes manifests from a GitHub release URL. Even with version pinning, executing remote scripts/manifests without checksum/signature verification can be risky. Consider verifying checksums/signatures (or using a pinned commit artifact / trusted action) before execution, and prefer kubectl apply with pinned digests or vendored manifests where feasible.

⚡ Recommended focus areas for review

Possible Issue

The helm install invocation for ingress-nginx appears to have arguments in the wrong order (chart reference used where the release name is expected). This may cause the step to fail at runtime; validate the command syntax.

- name: Install Ingress Controller
  if: steps.list-changed.outputs.changed == 'true'
  shell: bash
  run: |
    helm install ingress-nginx/ingress-nginx --generate-name \
      --set controller.service.type='NodePort' \
      --set controller.admissionWebhooks.enabled=false
CI Stability

The disk cleanup step sets rm_cmd to rmz, which is likely not a valid command on GitHub runners and could make the workflow fail before tests run. Confirm the intended value or remove the override.

- name: Remove unnecessary files to free up disk space
  if: steps.list-changed.outputs.changed == 'true'
  uses: endersonmenezes/free-disk-space@e6ed9b02e683a3b55ed0252f1ee469ce3b39a885 # v3
  with:
    remove_android: true
    remove_dotnet: true
    remove_haskell: true
    rm_cmd: "rmz"
📚 Focus areas based on broader codebase context

CI Values

The composite action hard-codes a minimal set of Helm overrides for KinD (e.g., route.enabled=false) but does not apply the other CI workarounds used elsewhere for KinD (e.g., disabling PVC-backed Postgres persistence and/or disabling the Helm test pod / using a known-good test pod image). This can reintroduce CI instability in KinD where Routes/PVCs are not available or where the default test image/tag is unreliable. Consider aligning the ct install overrides with the existing charts/backstage/ci/*-values.yaml patterns. (Ref 4, Ref 5)

EXTRA_ARGS=(
  "--set route.enabled=false"
  "--set upstream.ingress.enabled=true"
  "--set global.host=rhdh.127.0.0.1.sslip.io"
)

Reference reasoning: The existing CI-specific values files for the Backstage chart explicitly disable Routes and PVC persistence and also provide patterns to disable the test pod or pin a custom test image/tag for CI. The new workflow only mirrors the Route workaround, so reusing or matching those established values would keep CI behavior consistent with the repo’s prior KinD-focused configuration.

📄 References
  1. redhat-developer/rhdh-chart/ct.yaml [1-10]
  2. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.yaml [37-41]
  3. redhat-developer/rhdh-chart/charts/backstage/values.yaml [351-369]
  4. redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12]
  5. redhat-developer/rhdh-chart/charts/backstage/ci/with-custom-image-for-test-pod-values.yaml [1-15]
  6. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27]
  7. redhat-developer/rhdh-chart/charts/orchestrator-software-templates/ci/upstream-values.yaml [1-16]
  8. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/values.yaml [141-163]

@rhdh-qodo-merge rhdh-qodo-merge bot added the enhancement New feature or request label Jan 15, 2026
@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Jan 15, 2026

PR Type

(Describe updated until commit e275176)

Enhancement


Description

  • Refactor chart testing logic into reusable composite action for code reuse

  • Pin PR testing to stable RHDH image with configurable repository and tag

  • Add nightly workflow to test against unstable 'next' RHDH image tag

  • Simplify PR workflow by removing obsolete branch patterns


File Walkthrough

Relevant files
Enhancement
action.yml
New composite action for chart testing                                     

.github/actions/test-charts/action.yml

  • Created new composite action encapsulating all chart testing setup and
    execution steps
  • Accepts configurable inputs for target branch, extra Helm arguments,
    and chart selection
  • Includes environment setup (Helm, Python, chart-testing), cluster
    creation (KIND), and prerequisite installations (OLM, Ingress
    Controller, CRDs)
  • Implements conditional logic to detect changed charts and install
    Knative/SonataFlow CRDs when backstage chart is modified
+185/-0 
nightly.yaml
New nightly chart testing workflow                                             

.github/workflows/nightly.yaml

  • New workflow scheduled to run nightly at 21:38 UTC with manual trigger
    support
  • Tests all charts against unstable 'next' RHDH image tag from
    rhdh-community repository
  • Uses the new reusable test-charts composite action with appropriate
    configuration
+29/-0   
test.yaml
Refactor PR workflow to use composite action                         

.github/workflows/test.yaml

  • Removed obsolete branch patterns (rhdh-1.x and 1.x.x branches) from PR
    trigger
  • Added environment variables for configurable RHDH image repository and
    tag with sensible defaults
  • Replaced 120+ lines of inline testing logic with call to new
    test-charts composite action
  • Pinned PR testing to stable RHDH image on main branch while preserving
    release branch behavior
+11/-133

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Jan 15, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix unsafe parsing of Helm arguments

To avoid unsafe parsing of Helm arguments, refactor the run step to use string
concatenation for EXTRA_ARGS instead of creating a bash array with read -ra.

.github/actions/test-charts/action.yml [131-148]

 - name: Run chart-testing (install)
   shell: bash
   run: |
-    EXTRA_ARGS=(
-      "--set route.enabled=false"
-      "--set upstream.ingress.enabled=true"
-      "--set global.host=rhdh.127.0.0.1.sslip.io"
-    )
+    EXTRA_ARGS="--set route.enabled=false --set upstream.ingress.enabled=true --set global.host=rhdh.127.0.0.1.sslip.io"
     if [[ -n "${{ inputs.extra_helm_args }}" ]]; then
-      IFS=' ' read -ra ADDITIONAL_ARGS <<< "${{ inputs.extra_helm_args }}"
-      EXTRA_ARGS+=("${ADDITIONAL_ARGS[@]}")
+      EXTRA_ARGS="$EXTRA_ARGS ${{ inputs.extra_helm_args }}"
     fi
     ct install \
       --debug \
       --config ct-install.yaml \
       --upgrade \
       --target-branch "${{ inputs.target_branch }}" \
-      --helm-extra-set-args="${EXTRA_ARGS[*]}"
+      --helm-extra-set-args="$EXTRA_ARGS"
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that parsing arguments with read -ra is fragile and proposes a more robust string concatenation approach, which simplifies the code and improves reliability.

Medium
General
Quote dynamic Helm args

Wrap the expression for extra_helm_args in quotes to ensure it always resolves
to a string, preventing potential errors with Helm arguments.

.github/workflows/test.yaml [49]

-extra_helm_args: ${{ github.event.pull_request.base.ref == 'main' && '--set upstream.backstage.image.repository=rhdh/rhdh-hub-rhel9 --set upstream.backstage.image.tag=latest' || '' }}
+extra_helm_args: "${{ github.event.pull_request.base.ref == 'main' && '--set upstream.backstage.image.repository=rhdh/rhdh-hub-rhel9 --set upstream.backstage.image.tag=latest' || '' }}"
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential issue where a boolean false could be passed as an argument, and wrapping the expression in quotes ensures it is always treated as a string, improving the workflow's reliability.

Medium
Use kubectl apply for idempotency

Replace kubectl create -f with kubectl apply -f to make the resource creation
step idempotent and prevent failures on workflow re-runs.

.github/actions/test-charts/action.yml [120-129]

 - name: Install Knative and SonataFlow CRDs
   if: inputs.install_orchestrator_prereqs == 'true'
   shell: bash
   env:
     SONATAFLOW_OPERATOR_VERSION: "10.1.0"
   run: |
     for crdDir in charts/orchestrator-infra/crds/*; do
-      kubectl create -f "${crdDir}"
+      kubectl apply -f "${crdDir}"
     done
-    kubectl create -f "https://github.com/apache/incubator-kie-tools/releases/download/${SONATAFLOW_OPERATOR_VERSION}/apache-kie-${SONATAFLOW_OPERATOR_VERSION}-incubating-sonataflow-operator.yaml"
+    kubectl apply -f "https://github.com/apache/incubator-kie-tools/releases/download/${SONATAFLOW_OPERATOR_VERSION}/apache-kie-${SONATAFLOW_OPERATOR_VERSION}-incubating-sonataflow-operator.yaml"
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that kubectl apply is idempotent and more suitable for CI/CD, which improves the robustness of the workflow against re-runs.

Low
High-level
Pin external dependencies in CI

To improve security and ensure reproducible builds, pin the OLM installation
script dependency. Instead of using a mutable tag, verify the downloaded script
against a known checksum.

Examples:

.github/actions/test-charts/action.yml [105-113]
    - name: Install Operator Lifecycle Manager (OLM)
      # In case we need to install additional Operators
      shell: bash
      env:
        OLM_VERSION: "v0.31.0"
      run: |
        curl -L "https://github.com/operator-framework/operator-lifecycle-manager/releases/download/${OLM_VERSION}/install.sh" -o install-olm.sh
        chmod +x install-olm.sh
        ./install-olm.sh "${OLM_VERSION}"

Solution Walkthrough:

Before:

- name: Install Operator Lifecycle Manager (OLM)
  env:
    OLM_VERSION: "v0.31.0"
  run: |
    curl -L "https://github.com/operator-framework/operator-lifecycle-manager/releases/download/${OLM_VERSION}/install.sh" -o install-olm.sh
    chmod +x install-olm.sh
    ./install-olm.sh "${OLM_VERSION}"

After:

- name: Install Operator Lifecycle Manager (OLM)
  env:
    OLM_VERSION: "v0.31.0"
    INSTALL_SH_SHA256: "known_checksum_for_v0.31.0_script"
  run: |
    curl -L "https://github.com/operator-framework/operator-lifecycle-manager/releases/download/${OLM_VERSION}/install.sh" -o install-olm.sh
    echo "${INSTALL_SH_SHA256} install-olm.sh" | sha256sum --check
    chmod +x install-olm.sh
    ./install-olm.sh "${OLM_VERSION}"
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies an unpinned dependency (install.sh) in the new composite action, which is a valid security and reproducibility concern, although the proposed fix isn't perfectly applicable to release assets.

Low
  • Update

@rm3l rm3l changed the title stabilize ci ci: stabilize CI Jan 15, 2026
@rm3l
Copy link
Member Author

rm3l commented Jan 15, 2026

/review

@rhdh-qodo-merge
Copy link

Persistent review updated to latest commit 3091e22

@rm3l rm3l changed the title ci: stabilize CI ci: stabilize CI [RHDHBUGS-2513] Jan 15, 2026
@rm3l rm3l changed the title ci: stabilize CI [RHDHBUGS-2513] ci: Stabilize CI by pinning to specific RHDH tag on PRs and testing next on a nightly basis [RHDHBUGS-2513] Jan 15, 2026
@sonarqubecloud
Copy link

@rm3l rm3l marked this pull request as ready for review January 15, 2026 11:20
@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHDHBUGS-2513 - Partially compliant

Compliant requirements:

  • Adjust CI so PR validation remains stable even when next is broken.
  • Still keep coverage for testing the unstable next tag (e.g., on a schedule/nightly) to detect when it breaks or recovers.

Non-compliant requirements:

  • Fix CI failures related to the next tag of RHDH failing to start.

Requires further human verification:

  • Confirm PR workflow now passes reliably on main PRs with the pinned stable/RC image configuration.
  • Confirm nightly workflow actually runs and reports failures when next is broken (and passes when it recovers).
  • Confirm repository variables RHDH_IMAGE_REPOSITORY / RHDH_IMAGE_TAG are set as intended (or that defaults are acceptable).
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 Security concerns

Supply chain risk:
The workflow downloads and executes install.sh for OLM from GitHub releases (curl ... -o install-olm.sh then chmod +x and runs it). Even with a pinned OLM_VERSION, this is still executing remote code without checksum/signature verification. Consider verifying checksums/signatures for the downloaded script/artifacts or using a pinned, trusted action/container image for OLM installation instead.

⚡ Recommended focus areas for review

CI Breakage

The composite action sets up Python with python-version: 3.14. If GitHub Actions toolcache or actions/setup-python does not support this version, the workflow may fail during environment setup. Consider using a currently supported Python version (or pinning to a known-good minor) for ct.

- name: Set up Python
  uses: actions/setup-python@83679a892e2d95755f2dac6acb0bfd1e9ac5d548 # v6
  with:
    python-version: 3.14

- name: Set up chart-testing
  uses: helm/chart-testing-action@6ec842c01de15ebb84c8627d2744a0c2f2755c9f # v2.8.0
  with:
    version: '3.14.0'
    yamllint_version: '1.37.1'
Argument Parsing

extra_helm_args is split on spaces (IFS=' ' read -ra ...), which can break flags that require quoting or contain spaces, and may not preserve intended argument boundaries. Consider passing extra args as multiline, JSON array, or using a more robust parsing strategy to avoid subtle CI-only failures.

if [[ -n "$INPUT_EXTRA_HELM_ARGS" ]]; then
  IFS=' ' read -ra ADDITIONAL_ARGS <<< "$INPUT_EXTRA_HELM_ARGS"
  EXTRA_ARGS+=("${ADDITIONAL_ARGS[@]}")
fi
📄 References
  1. redhat-developer/rhdh-chart/ct.yaml [1-10]
  2. redhat-developer/rhdh-chart/ct-install.yaml [1-11]
  3. redhat-developer/rhdh-chart/charts/backstage/values.yaml [351-369]
  4. redhat-developer/rhdh-chart/charts/backstage/ci/with-test-pod-disabled-values.yaml [1-12]
  5. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-and-dynamic-plugins-npmrc-values.yaml [1-24]
  6. redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-values.yaml [1-21]
  7. redhat-developer/rhdh-chart/charts/orchestrator-infra/values.yaml [37-41]
  8. redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/values.yaml [141-163]

@rm3l rm3l merged commit 3fa9cdc into redhat-developer:main Jan 15, 2026
6 of 7 checks passed
@rm3l rm3l deleted the stabilize_ci branch January 15, 2026 11:21
@rhdh-qodo-merge
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct invalid rm_cmd flag

Correct the invalid rm_cmd value from "rmz" to a valid command like "rm -rf" to
ensure the disk space cleanup step functions correctly.

.github/actions/test-charts/action.yml [64]

-rm_cmd: "rmz"
+rm_cmd: "rm -rf"
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that rmz is not a standard command and would cause the disk space cleanup step to fail, which is a critical bug in the workflow.

High
General
Simplify conditional expression for clarity

Simplify the extra_helm_args expression by removing the redundant || '' and
improve readability by using a multiline block scalar.

.github/workflows/test.yaml [37]

-extra_helm_args: ${{ github.event.pull_request.base.ref == 'main' && format('--set upstream.backstage.image.repository={0} --set upstream.backstage.image.tag={1}', env.RHDH_IMAGE_REPOSITORY, env.RHDH_IMAGE_TAG) || '' }}
+extra_helm_args: |
+  ${{ github.event.pull_request.base.ref == 'main' && format('--set upstream.backstage.image.repository={0} --set upstream.backstage.image.tag={1}', env.RHDH_IMAGE_REPOSITORY, env.RHDH_IMAGE_TAG) }}
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly points out that || '' is redundant and improves readability by using a multiline block scalar, which is a good practice for long strings in YAML.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant