[DO NOT MERGE] sandboxed-containers-operator: add Azure disconnected workflow#80922
[DO NOT MERGE] sandboxed-containers-operator: add Azure disconnected workflow#80922balintTobik wants to merge 3 commits into
Conversation
Add e2e workflow for testing OSC on a disconnected ARO cluster. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Balint Tobik <btobik@redhat.com>
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new ChangesARO Disconnected E2E Testing Pipeline
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: balintTobik The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![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
`@ci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-commands.sh`:
- Line 107: The oc apply and oc patch commands at lines 107, 123, and 159 are
using || true to suppress errors, which masks critical cluster configuration
failures and allows the step to report success even when the cluster is only
partially configured. Remove the || true error suppression from all three oc
apply and oc patch command invocations to allow the script to fail appropriately
when these critical operations encounter errors, ensuring the cluster is
properly configured before the step completes.
- Line 6: Remove the global xtrace setting by replacing the `set -x` on line 6
with the default strict mode `set -euo pipefail`, which enables error handling
and variable expansion without command tracing. Additionally, refactor the
registry login commands at lines 35-37 to use the `--password-stdin` method
instead of passing the password directly with the `-p` flag, which pipes the
password through stdin instead of exposing it as a command-line argument that
could be captured in traces or process listings.
In
`@ci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-ref.yaml`:
- Around line 15-16: The environment variable declared with the name
OPERTORS_TO_MIRROR contains a typo (missing the "A" in OPERATORS). Rename the
variable declaration from OPERTORS_TO_MIRROR to OPERATORS_TO_MIRROR in the YAML
file at the name field. Additionally, add backward-compatible fallback logic in
the associated command script to check for the old misspelled variable name
OPERTORS_TO_MIRROR and use it if OPERATORS_TO_MIRROR is not set, ensuring
existing callers using the misspelled name continue to work while new code uses
the correct spelling.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0d6c86a0-0e95-4d47-a407-c355b3f87023
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (12)
ci-operator/config/openshift/sandboxed-containers-operator/openshift-sandboxed-containers-operator-devel__downstream-candidate419.yamlci-operator/step-registry/sandboxed-containers-operator/e2e/aro-disconnected/OWNERSci-operator/step-registry/sandboxed-containers-operator/e2e/aro-disconnected/sandboxed-containers-operator-e2e-aro-disconnected-workflow.metadata.jsonci-operator/step-registry/sandboxed-containers-operator/e2e/aro-disconnected/sandboxed-containers-operator-e2e-aro-disconnected-workflow.yamlci-operator/step-registry/sandboxed-containers-operator/env-cm/sandboxed-containers-operator-env-cm-commands.shci-operator/step-registry/sandboxed-containers-operator/gather-must-gather/sandboxed-containers-operator-gather-must-gather-commands.shci-operator/step-registry/sandboxed-containers-operator/get-kata-rpm/sandboxed-containers-operator-get-kata-rpm-commands.shci-operator/step-registry/sandboxed-containers-operator/mirror-operator/OWNERSci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-commands.shci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-ref.metadata.jsonci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-ref.yamlci-operator/step-registry/sandboxed-containers-operator/record-metadata/sandboxed-containers-operator-record-metadata-commands.sh
| set -o nounset | ||
| set -o errexit | ||
| set -o pipefail | ||
| set -x |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Stop tracing and argv-passing registry credentials.
Line 6 enables global xtrace, and Lines 35-37 pass passwords with -p; together this can expose credential material in traces/process args. Keep default strict mode without -x, and use --password-stdin for registry logins.
As per coding guidelines, "Use default set -euo pipefail (without -x) in step registry command scripts; only enable -x when actively debugging" and "Protect sensitive information... never echo or print passwords, tokens, API keys."
Suggested patch
-set -x
+# Keep default strict mode without global xtrace.
@@
-echo "Logging into registries..."
-skopeo login "${MIRROR_REGISTRY_HOST}" -u "${mirror_registry_user}" -p "${mirror_registry_password}" --tls-verify=false
-skopeo login registry.redhat.io -u "${redhat_auth_user}" -p "${redhat_auth_password}"
+echo "Logging into registries..."
+printf '%s' "${mirror_registry_password}" | \
+ skopeo login "${MIRROR_REGISTRY_HOST}" -u "${mirror_registry_user}" --password-stdin --tls-verify=false
+printf '%s' "${redhat_auth_password}" | \
+ skopeo login registry.redhat.io -u "${redhat_auth_user}" --password-stdinAlso applies to: 35-37
🤖 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
`@ci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-commands.sh`
at line 6, Remove the global xtrace setting by replacing the `set -x` on line 6
with the default strict mode `set -euo pipefail`, which enables error handling
and variable expansion without command tracing. Additionally, refactor the
registry login commands at lines 35-37 to use the `--password-stdin` method
instead of passing the password directly with the `-p` flag, which pipes the
password through stdin instead of exposing it as a command-line argument that
could be captured in traces or process listings.
Source: Coding guidelines
| if [[ -f "$f" ]]; then | ||
| echo "=== Applying: $f ===" | ||
| cat "$f" | ||
| oc apply -f "$f" || true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not suppress critical cluster-configuration failures.
Lines 107, 123, and 159 ignore oc apply/patch errors with || true, which can leave the cluster partially configured while this step reports success.
Suggested patch
- oc apply -f "$f" || true
+ oc apply -f "$f"
@@
- cat <<EOFITMS | oc apply -f - || true
+ cat <<EOFITMS | oc apply -f -
@@
-oc patch operatorhub cluster --type=merge -p '{"spec":{"disableAllDefaultSources":true}}' || true
+oc patch operatorhub cluster --type=merge -p '{"spec":{"disableAllDefaultSources":true}}'Also applies to: 123-123, 159-159
🤖 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
`@ci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-commands.sh`
at line 107, The oc apply and oc patch commands at lines 107, 123, and 159 are
using || true to suppress errors, which masks critical cluster configuration
failures and allows the step to report success even when the cluster is only
partially configured. Remove the || true error suppression from all three oc
apply and oc patch command invocations to allow the script to fail appropriately
when these critical operations encounter errors, ensuring the cluster is
properly configured before the step completes.
| - name: OPERTORS_TO_MIRROR | ||
| default: "sandboxed-containers-operator" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the operator env var name typo before this interface spreads.
Line 15 declares OPERTORS_TO_MIRROR (misspelled). This makes intuitive callers of OPERATORS_TO_MIRROR silently ineffective and can produce unexpected defaults at runtime. Rename the declared variable and keep a temporary backward-compatible fallback in the command script.
🤖 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
`@ci-operator/step-registry/sandboxed-containers-operator/mirror-operator/sandboxed-containers-operator-mirror-operator-ref.yaml`
around lines 15 - 16, The environment variable declared with the name
OPERTORS_TO_MIRROR contains a typo (missing the "A" in OPERATORS). Rename the
variable declaration from OPERTORS_TO_MIRROR to OPERATORS_TO_MIRROR in the YAML
file at the name field. Additionally, add backward-compatible fallback logic in
the associated command script to check for the old misspelled variable name
OPERTORS_TO_MIRROR and use it if OPERATORS_TO_MIRROR is not set, ensuring
existing callers using the misspelled name continue to work while new code uses
the correct spelling.
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-downstream-candidate419-aro-ipi-kata-disconnected |
|
@balintTobik: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@balintTobik: job(s): periodic-ci-openshift-sandboxed-containers-operator-devel-downstream-candidate419-aro-ipi-kata-disconnected either don't exist or were not found to be affected, and cannot be rehearsed |
add aro-ipi-kata-disconnected job to the 4.19 downstream candidate config using the new disconnected workflow. Co-Authored-By: Claude noreply@anthropic.com Signed-off-by: Balint Tobik <btobik@redhat.com>
Run make jobs && make registry-metadata to generate prow job configs and step registry metadata. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Balint Tobik <btobik@redhat.com>
balintTobik
force-pushed
the
osc-disconnected-aro
branch
from
18cf38f to
50bff37
Compare
|
[REHEARSALNOTIFIER]
A total of 50 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-downstream-candidate419-aro-ipi-kata-disconnected |
1 similar comment
|
/pj-rehearse periodic-ci-openshift-sandboxed-containers-operator-devel-downstream-candidate419-aro-ipi-kata-disconnected |
|
@balintTobik: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@balintTobik: 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. |
1 participant
sandboxed-containers-operator: add Azure disconnected workflow
Add e2e workflow for testing OSC on a disconnected ARO cluster.
Summary by CodeRabbit
This PR expands the OpenShift CI infrastructure to run end-to-end (e2e) Sandboxed Containers Operator tests on a disconnected (air-gapped) Azure Red Hat OpenShift (ARO) cluster, including private image/registry mirroring and proxy-aware execution. It’s intentionally blocked from merging into main (marked “[DO NOT MERGE]” with a
/hold), and includes a rehearsal trigger (/pj-rehearse) for validation of the new downstream periodic workflow.Key changes:
New e2e workflow for disconnected ARO (
sandboxed-containers-operator-e2e-aro-disconnected)Adds a dedicated workflow that sets up a private ARO environment (Azure resource group/VNet/bastion/proxy), mirrors required catalogs/images for disconnected operation, performs operator pre-setup, runs OpenShift extended tests, runs must-gather-related post-steps, and then tears down resources. The workflow is designed for disconnected/private visibility scenarios and includes the full step ordering and configuration needed for isolated testing.
New periodic downstream CI job (
aro-ipi-kata-disconnected)Adds a
testsjob to the 4.19 downstream candidate CI config to run the disconnected ARO e2e workflow with:ENABLE_MUST_GATHER: "false"SLEEP_DURATION: 3hTEST_FILTERS: ~Disruptive&TEST_PARALLEL: "1"(Notably, the new job definition omits a
restrict_network_accessfield.)New
mirror-operatorstep to support disconnected catalogs/imagesIntroduces a new mirroring command implementation that logs into the mirror registry and Red Hat operator registries, generates and runs
oc-mirrorv2 to mirror operator catalogs, applies generated disconnectedcluster-resources, and optionally mirrors extra images (including must-gather/hello-openshift defaults). It also:CatalogSource(when discovered/recorded) to become READY,Proxy + disconnected catalog source awareness across supporting steps
Updates multiple step scripts to be proxy-aware in disconnected environments by conditionally sourcing a shared
proxy-conf.shwhen present:sandboxed-containers-operator-env-cm-commands.sh(also reads adisconnected_catalog_source_namefile fromSHARED_DIRand setsCATALOG_SOURCE_NAMEaccordingly)gather-must-gather-commands.shget-kata-rpm-commands.shrecord-metadata-commands.shGovernance/metadata for the new workflow and step
Adds/updates OWNERS and workflow metadata for both:
e2e/aro-disconnected)mirror-operator)to define consistent approvers/reviewers (
ldoktor,tbuskey,vvoronko,wainersm).