Repository navigation
Conversation
The Helm and Operator disconnected nightlies have not passed for over five weeks. Three independent gaps kept them red: - Backend egress to the mirror registry. The chart and the Operator both default-deny backend egress except DNS, HTTPS, Redis, and PostgreSQL, while the CI mirror serves plain HTTP on port 5000. install-dynamic-plugins timed out pulling the catalog index from the bastion. - Operator catalog. A filtered oc-mirror run against the published Red Hat catalog rejects it since rhdh-operator#3822 removed the default `fast` channel, and that catalog carries no 2.x bundles anyway. CI runs now mirror the RHDH CI catalog (quay.io/rhdh/iib:<stream>-v<ocp>-x86_64); its registry.redhat.io/rhdh relatedImages are redirected to quay.io/rhdh with CONTAINERS_REGISTRIES_CONF for the prepare-restricted-environment.sh call. - Smoke plugin set. On the 2.x New Frontend System hub the sign-in page, sidebar, SCM integrations, and guest auth provider are dynamic plugins, so the homepage-only set could not render a login. Mirror the minimal digest pinned set and configure the NFS extensions and signInPage in the app configs. The 2.x hub image and the new plugin set were verified on a live OCP cluster: guest sign-in, sidebar, and the "Welcome back!" homepage heading all render. Assisted-by: OpenCode
PR Summary by QodoUnblock disconnected OCP Helm and Operator nightlies
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-2.1 #5710 +/- ##
===============================================
- Coverage 75.70% 69.00% -6.70%
===============================================
Files 65 58 -7
Lines 708 555 -153
Branches 100 83 -17
===============================================
- Hits 536 383 -153
Misses 171 171
Partials 1 1
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Code Review by Qodo
View medium (1)🟠 **Medium** 1. Backend pods can reach unrelated services
|
Comment on lines
+159
to
+162
| egress: | ||
| - ports: | ||
| - port: ${registry_port} | ||
| protocol: TCP |
There was a problem hiding this comment.
🟠 Medium
1. Backend pods can reach unrelated services 🐞 Bug ⛨ Security
allow_mirror_registry_egress limits the new NetworkPolicies by TCP port but omits an egress destination. When the mirror uses port 5000, the selected backend pods can connect to any reachable service on that port, not just the mirror registry.
Agent Prompt
## Issue description
The new mirror-registry policies permit traffic to every destination on the mirror's TCP port, weakening the backend's default-deny egress policy.
## Fix Focus Areas
- .ci/pipelines/lib/disconnected/namespace.sh[130-186]
## Recommended Fix
Constrain both egress rules to the mirror destination as well as its port. Resolve and validate a stable mirror address or CIDR for an `ipBlock`, or use an appropriate selector when the mirror is inside the cluster.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+12
to
+18
| readonly -a DISCONNECTED_SMOKE_PLUGIN_NAMES=( | ||
| "red-hat-developer-hub-backstage-plugin-app-auth" | ||
| "red-hat-developer-hub-backstage-plugin-app-defaults" | ||
| "red-hat-developer-hub-backstage-plugin-app-integrations" | ||
| "red-hat-developer-hub-backstage-plugin-homepage" | ||
| "backstage-plugin-auth-backend-module-guest-provider" | ||
| ) |
There was a problem hiding this comment.
🟡 Low
2. Smoke plugin list is empty inside child shells 🐞 Bug ☼ Reliability
The single exported DISCONNECTED_HOMEPAGE_PLUGIN_NAME constant is replaced by `readonly -a DISCONNECTED_SMOKE_PLUGIN_NAMES, and bash cannot export arrays. Yet write_smoke_plugin_list` and set_smoke_plugin_packages_from_list are still exported with export -f so they can run in child shells (timeout bash -c). In such a child shell the array is empty, so write_digest_plugin_list gets an empty filter and keeps every digest-pinned plugin in the catalog. The export loop then sets no *_PLUGIN_PACKAGE variables and still returns success.
Agent Prompt
## Issue description
`DISCONNECTED_SMOKE_PLUGIN_NAMES` is a bash array. Arrays cannot be exported, so the exported functions `write_smoke_plugin_list` and `set_smoke_plugin_packages_from_list` see an empty list when they run in a child shell. The old homepage constant was exported for exactly this case.
## Fix Focus Areas
- .ci/pipelines/lib/disconnected/plugins.sh[12-18]
- .ci/pipelines/lib/disconnected/plugins.sh[203-241]
## Recommended Fix
Store the names in an exported space-separated string, e.g. `export DISCONNECTED_SMOKE_PLUGIN_NAMES="a b c ..."`. Loop over it with `for name in ${DISCONNECTED_SMOKE_PLUGIN_NAMES}`, or split it into a local array with `read -ra`. Also make `set_smoke_plugin_packages_from_list` fail when it exports zero packages.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- Pick the operator catalog from the chart build type instead of the image registry alone: a CI chart (x.y-N-CI) mirrors the RHDH CI catalog, a released chart (x.y.z) keeps the published catalog. A pinned RC/GA verification on main (TAG_NAME and CHART_VERSION 1.9.4) was handed a `next` catalog for a 1.9 channel filter. - Export the smoke plugin list as a space-separated scalar. bash cannot export arrays, so the `export -f` helpers saw an empty list in a child shell and would have mirrored every digest-pinned plugin. Fail when no package variable was exported. - Document why the mirror egress rule is scoped by port and not by destination: the bastion address is assigned per run and reachable by DNS name only, so an ipBlock would break whenever the bastion is replaced. Assisted-by: OpenCode
The chart mounts <fullname>-dynamic-plugins-registry-auth, and fullname is <release>-<nameOverride> with nameOverride pinned to developer-hub. The job created rhdh-dynamic-plugins-registry-auth instead, so the optional volume was never mounted, install-dynamic-plugins ran without auth.json, and skopeo failed the mirror pull with "authentication required". Pin nameOverride in the values and create the secret under the chart's name. Verified on a live OCP 4.21 cluster (LOCAL_DISCONNECTED): the init container now pulls the catalog index and all five smoke plugins through the mirror. Assisted-by: OpenCode
Contributor
Helm: after the IDMS/ITMS apply, `oc wait machineconfigpool --for=condition=Updated=True` returned while the MCO had not rendered the new config yet. The nodes were drained a few seconds later, while the hub and PostgreSQL pods were starting, so they were evicted mid-startup. The hub recovery path then ran `oc rollout restart`, starting a second backend next to the first against a freshly initialised database; the two raced on the knex migration lock and the losing backend stayed up with readiness 503 forever. - snapshot each pool's rendered config before the IDMS/ITMS and additionalTrustedCA changes, and wait for the MCO to render before waiting for convergence (poll Updated + updatedMachineCount; warn and continue when nothing is rolling) - start the hub restart grace period only once the backend container has started, and replace the rollout restart with scale to 0, wait for the pods to be gone, scale to 1, so two backends never overlap Operator: OLM v1 installs the bundle with the rhdh-operator-installer ServiceAccount, which cannot grant RBAC it does not hold itself. The installer ClusterRole rendered by prepare-restricted-environment.sh lags behind config/rbac/role.yaml (pods, apiservers/ingresses, devhubplugincatalogs), so the operator ClusterRole creation is refused and the operator ends up running with no permissions. The half-applied release still creates the CRD, so the CRD-only wait passed and the job failed later with a misleading message. - require ClusterExtension Installed=True as well as the CRD, log the install condition at info level, and dump OLM v1 status when the Backstage CR produces no deployment - TEMPORARY: grant the installer the operator's own ClusterRole rules from the same release branch before prepare runs, so the first install attempt succeeds. Remove once redhat-developer/rhdh-operator ships the fix. Verified the RBAC workaround on a live OCP cluster: the same ClusterRole creation is refused before the grant and succeeds after it. Assisted-by: OpenCode
Point the temporary OLM v1 installer workaround at redhat-developer/rhdh-operator#3923 (backport redhat-developer#3924) and RHDHBUGS-4056, so the patch is easy to find and remove once the fix is released. Assisted-by: OpenCode
Contributor
|
|
@zdrapela: The following test failed, say
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 branch has not been deployed
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
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.
Cherry-pick of #5709 to release-2.1. The disconnected job/lib/value/resource files are byte-identical between main and release-2.1.
The disconnected OCP Helm and Operator nightlies have not passed for over five weeks. Three gaps were blocking them:
disconnected::allow_mirror_registry_egressapplies a targeted rule for the mirror's port (Helm + Operator pod selectors); skipped on 443 (LOCAL_DISCONNECTED).fastchannel filtered out), and it has no 2.x bundles. CI runs now usequay.io/rhdh/iib:2.1-v<ocp>-x86_64with theregistry.redhat.io/rhdhrelatedImages redirected toquay.io/rhdhviaCONTAINERS_REGISTRIES_CONF.app.extensionsconfig andsignInPage: guest.Verified on a live OCP 4.21 cluster with the 2.x hub image: guest sign-in, sidebar and the
Welcome back!homepage heading render; plugin list/package export exercised against the real:2.1catalog.The nightly jobs on this branch additionally need the
upi-installerbase_image fix in openshift/release (tracked separately).Ref: #5709