chore(CI): initial step to refactor the CI #3671

gustavolira · 2025-11-10T20:49:35Z

Description

Please explain the changes you made here.

Which issue(s) does this PR fix

Fixes #?

PR acceptance criteria

Please make sure that the following steps are complete:

GitHub Actions are completed and successful
Unit Tests are updated and passing
E2E Tests are updated and passing
Documentation is updated if necessary (requirement for new features)
Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

openshift-ci · 2025-11-10T20:49:42Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign pataknight for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

github-actions · 2025-11-10T23:21:07Z

The image is available at:

zdrapela · 2025-11-11T08:38:08Z

/review

rhdh-qodo-merge · 2025-11-11T08:38:38Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🔒 Security concerns Sensitive information handling: Several functions read and propagate secret/token data (e.g., `OCM_CLUSTER_TOKEN`, PostgreSQL credentials, GitHub/Keycloak secrets in env.sh). Ensure secrets are not logged via log_info/log_debug and avoid writing them to shared artifacts. Additionally, in k8s.sh `create_secret_dockerconfigjson` writes provided dockerconfigjson directly; validate inputs are base64-encoded and avoid echoing their values.
⚡ Recommended focus areas for review Possible Issue In perform_helm_install, when the values file is missing the error message references an undefined variable `values_path`. This will print an empty/incorrect path and can mislead debugging. Use the actual `original_values` variable in the error message. local original_values="${PIPELINES_ROOT}/config/helm-values/${value_file}" if [[ ! -f "${original_values}" ]]; then log_error "Values file not found: ${values_path}" return 1 fi Portability Concern apply_yaml_files uses `base64 -w 0` for encoding `dh_target_url` which is not available on macOS. Elsewhere you implemented a cross-platform base64 wrapper; consider reusing it to avoid failures on non-GNU systems. local dh_target_url=$(echo -n "test-backstage-customization-provider-${project}.${K8S_CLUSTER_ROUTER_BASE}" \| base64 -w 0) local rhdh_base_url_encoded=$(echo -n "${rhdh_base_url}" \| base64 \| tr -d '\n') local rhdh_base_url_http=$(echo -n "${rhdh_base_url/https/http}" \| base64 \| tr -d '\n') export DH_TARGET_URL="${dh_target_url}" export RHDH_BASE_URL="${rhdh_base_url_encoded}" export RHDH_BASE_URL_HTTP="${rhdh_base_url_http}" # Apply service account and secrets
📚 Focus areas based on broader codebase context Plugin Merge Risk The code attempts to strip orchestrator plugins using yq path .global.dynamicPlugins...", but similar helm values use .global.dynamic.plugins. This mismatch likely leaves orchestrator plugins unmodified when INSTALL_ORCHESTRATOR_PLUGINS=false. Align the yq path to the actual schema and add a test to verify removal. (Ref 1) cp "${in_file}" "${out_file}" if [[ "${INSTALL_ORCHESTRATOR_PLUGINS:-true}" != "true" ]]; then log_info "Removing orchestrator plugins from Helm values (INSTALL_ORCHESTRATOR_PLUGINS=false)" if command -v yq &>/dev/null; then # delete any element in dynamicPlugins array where package contains 'orchestrator' yq -i 'del(.global.dynamicPlugins.""[] \| select(.package \| test("orchestrator")))' "${out_file}" else log_warning "yq not found – cannot strip orchestrator plugins." fi fi Reference reasoning: The referenced helm values show dynamic plugins under global.dynamic.plugins. Using a different key path in preprocessing will not affect those entries, leading to unintended plugin retention.
📄 References redhat-developer/rhdh-chart/charts/backstage/ci/with-orchestrator-values.yaml [1-21] redhat-developer/rhdh-chart/charts/orchestrator-software-templates-infra/ci/upstream-values.yaml [1-27] redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [219-243] redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [197-218] redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [155-171] redhat-developer/rhdh-chart/charts/orchestrator-software-templates/templates/tekton/tekton-tasks.yaml [172-196] redhat-developer/rhdh-operator/config/profile/rhdh/plugin-deps/tekton.yaml [172-196] redhat-developer/rhdh-operator/config/profile/rhdh/plugin-deps/tekton.yaml [321-342]

github-actions · 2025-11-11T13:23:58Z

The image is available at:

- Create new modular pipeline structure in pipelines/ directory - Add platform abstraction layer (OpenShift, AKS, EKS, GKE) - Implement core utilities (env, k8s, logging, reporting) - Add deployment modules (Helm, PostgreSQL, orchestrator workflows) - Create job handlers for different CI scenarios (pull, nightly, upgrade) - Extract PostgreSQL TLS certificates to temporary directory (security) - Add orchestrator feature flags (INSTALL_ORCHESTRATOR_INFRA/PLUGINS) - Fix code review findings (Qodo analysis): - Fix undefined variable in error message - Replace base64 -w 0 with portable version for macOS - Correct yq path for orchestrator plugin removal - Update .gitignore for pipeline artifacts This refactoring improves code organization, portability, and security while maintaining compatibility with existing CI workflows.

github-actions · 2025-11-11T15:02:21Z

The image is available at:

gustavolira · 2025-11-14T14:00:25Z

@zdrapela Any comments here?

schultzp2020 · 2025-11-14T18:02:48Z

@gustavolira can you please update the description and add a linked issue? 🙏

zdrapela · 2025-11-19T14:28:03Z

Can you please remove the AKS, GKE, and EKS completely? We want to only focus on OCP here for now. Also, remove ocp-upgrade, we can add that later too. There are still many files in pipelines/modules/platform/*

Can we also set up a job in OpenShift CI that would allow us to verify it runs here?

Sorry for the delay, but it still is a scary PR to review 😅

zdrapela

First round of comments. It would be good to do a second round after we have it configured in OpenShift CI

zdrapela · 2025-11-19T14:34:46Z

pipelines/main.sh

+case "${JOB_NAME}" in
+  *pull*ocp*helm*)


Why not get rid ot this detection by JOB_NAME and move it to openshift/release repo? You already have the Makefile, so let's use it. The entrypoint for ocp-helm pull is here: https://github.com/openshift/release/blob/3951d20775795d07b42e9f10d4df22d61af55182/ci-operator/step-registry/redhat-developer/rhdh/ocp/helm/redhat-developer-rhdh-ocp-helm-commands.sh#L180-L181 and it's similar for others.

zdrapela · 2025-11-19T14:38:44Z

pipelines/modules/platform/ocp.sh

+
+# Detect OpenShift platform (wrapper for common function)
+# Usage: detect_ocp
+detect_ocp() {


The detection was not reliable, and it's now hardcoded, see: https://github.com/openshift/release/blob/3951d20775795d07b42e9f10d4df22d61af55182/ci-operator/step-registry/redhat-developer/rhdh/ocp/helm/redhat-developer-rhdh-ocp-helm-commands.sh#L51
Please delete this and the detect_platform_type function

zdrapela · 2025-11-19T14:44:43Z

pipelines/modules/platform/ocp.sh

+set -euo pipefail  # Exit on error, undefined variables, and pipe failures
+
+# Source core modules
+# shellcheck source=../../core/k8s.sh


Can you tell me what's your ShellCheck config? I cannot get my ShellCheck to follow the sourcing if it wasn't absolute :(

zdrapela · 2025-11-19T14:46:00Z

pipelines/modules/platform/eks.sh

@@ -0,0 +1,479 @@
+#!/bin/bash


Remove the file

zdrapela · 2025-11-19T14:46:06Z

pipelines/modules/platform/aks.sh

@@ -0,0 +1,277 @@
+#!/bin/bash


Remove the file

zdrapela · 2025-11-19T14:47:59Z

pipelines/modules/deployment/helm.sh

+  for file in "${files[@]}"; do
+    if [[ -f "${file}" ]]; then
+      sed_inplace "s/namespace:.*/namespace: ${project}/g" "${file}"
+    fi
+  done


Replacing namespace in the files is redundant when you apply the file to a specific namespace instead (which is already happening). What's more, it is making changes in files that you don't want to commit.

zdrapela · 2025-11-19T14:54:25Z

pipelines/modules/deployment/helm.sh

+  local postgres_password=$(oc get secret/postgress-external-db-pguser-janus-idp \
+    -n "${NAME_SPACE_POSTGRES_DB}" -o jsonpath='{.data.password}')
+  sed_inplace "s|POSTGRES_PASSWORD:.*|POSTGRES_PASSWORD: ${postgres_password}|g" \
+    "${resources_dir}/postgres-cred.yaml"
+
+  local postgres_host=$(echo -n "postgress-external-db-primary.${NAME_SPACE_POSTGRES_DB}.svc.cluster.local" | \
+    base64 | tr -d '\n')
+  sed_inplace "s|POSTGRES_HOST:.*|POSTGRES_HOST: ${postgres_host}|g" \
+    "${resources_dir}/postgres-cred.yaml"
+
+  oc apply -f "${resources_dir}/postgres-cred.yaml" --namespace="${project}"


Let's use envsubst < file.yaml | oc apply --namespace="${project}" -f - anywhere possible. It's substantially better than using sed

zdrapela · 2025-11-19T14:58:21Z

pipelines/core/env.sh

+    echo ""
+  fi


We should be notified is secret is not populated. This will hide a missing env var, which is hard to debug

github-actions · 2025-11-27T02:09:21Z

This PR is stale because it has been open 7 days with no activity. Remove stale label or comment or this will be closed in 21 days.

openshift-ci bot requested review from josephca and schultzp2020 November 10, 2025 20:49

gustavolira temporarily deployed to internal November 10, 2025 20:49 — with GitHub Actions Inactive

gustavolira changed the title ~~Initial step to refactor the CI~~ chore(CI): Initial step to refactor the CI Nov 10, 2025

gustavolira changed the title ~~chore(CI): Initial step to refactor the CI~~ chore(CI): initial step to refactor the CI Nov 10, 2025

gustavolira force-pushed the main-refactor-sh-new branch from 316bf59 to 55d9e3f Compare November 10, 2025 20:50

gustavolira temporarily deployed to internal November 10, 2025 20:51 — with GitHub Actions Inactive

gustavolira force-pushed the main-refactor-sh-new branch from 91b31fe to be8a683 Compare November 10, 2025 21:00

gustavolira temporarily deployed to internal November 10, 2025 21:02 — with GitHub Actions Inactive

gustavolira temporarily deployed to internal November 10, 2025 21:49 — with GitHub Actions Inactive

gustavolira temporarily deployed to internal November 10, 2025 22:26 — with GitHub Actions Inactive

rhdh-qodo-merge bot added Review effort 4/5 Possible security concern labels Nov 11, 2025

gustavolira force-pushed the main-refactor-sh-new branch from 4bb8f69 to e6b846e Compare November 11, 2025 12:29

gustavolira temporarily deployed to internal November 11, 2025 12:29 — with GitHub Actions Inactive

gustavolira added 2 commits November 11, 2025 11:07

Initial step to refactor the CI

ea330e6

gustavolira force-pushed the main-refactor-sh-new branch from e6b846e to eb6404d Compare November 11, 2025 14:07

gustavolira temporarily deployed to internal November 11, 2025 14:07 — with GitHub Actions Inactive

gustavolira requested review from albarbaro and zdrapela November 12, 2025 17:23

zdrapela suggested changes Nov 19, 2025

View reviewed changes

github-actions bot added the Stale label Nov 27, 2025

zdrapela mentioned this pull request Dec 2, 2025

feat: centralize namespace configuration for RHDH CI/CD jobs #3776

Merged

5 tasks

github-actions bot removed the Stale label Dec 3, 2025

gustavolira closed this Dec 3, 2025

chore(CI): initial step to refactor the CI #3671

chore(CI): initial step to refactor the CI #3671

Uh oh!

Conversation

gustavolira commented Nov 10, 2025

Description

Which issue(s) does this PR fix

PR acceptance criteria

How to test changes / Special notes to the reviewer

Uh oh!

openshift-ci bot commented Nov 10, 2025

Uh oh!

github-actions bot commented Nov 10, 2025

Uh oh!

zdrapela commented Nov 11, 2025

Uh oh!

rhdh-qodo-merge bot commented Nov 11, 2025

PR Reviewer Guide 🔍

Uh oh!

github-actions bot commented Nov 11, 2025

Uh oh!

github-actions bot commented Nov 11, 2025

Uh oh!

gustavolira commented Nov 14, 2025

Uh oh!

schultzp2020 commented Nov 14, 2025

Uh oh!

zdrapela commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zdrapela left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zdrapela commented Nov 19, 2025 •

edited

Loading