[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 7 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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a disconnected ARO CI job and workflow for sandboxed-containers-operator, a mirror-operator step for registry mirroring, and script updates that load shared proxy settings and disconnected catalog-source state. ChangesDisconnected ARO pipeline
🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 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
|
/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. |
Remove REGISTER_MIRROR_REGISTRY_DNS parameter which is not needed for ARO deployments. DNS resolution for the mirror registry is handled by bastion-dnsmasq instead of Azure private DNS zones. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
/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. |
Add REGISTER_MIRROR_REGISTRY_DNS and BASE_DOMAIN to enable proper mirror registry URL creation. The azure-provision-bastionhost step requires BASE_DOMAIN when REGISTER_MIRROR_REGISTRY_DNS is enabled to create the mirror_registry_url file needed by mirror-images-by-oc-adm. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
balintTobik
force-pushed
the
osc-disconnected-aro
branch
from
37f5673 to
716257b
Compare
|
/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. |
…vision Move sandboxed-containers-operator-mirror-operator step after aro-provision-cluster since it needs to connect to the cluster API to get the OCP version. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
/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. |
Remove bastion-dnsmasq step and ENALBE_DNSMASQ_METHOD env var as they are not needed for ARO. ARO manages its own DNS, and the mirror registry DNS is already handled by azure-provision-bastionhost when REGISTER_MIRROR_REGISTRY_DNS is enabled. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
[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 |
|
@balintTobik: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
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 adds an end-to-end disconnected (air-gapped) Azure Red Hat OpenShift (ARO) test workflow for the Sandboxed Containers Operator (OSC) to OpenShift CI infrastructure, including private/disconnected catalog & image mirroring and proxy-aware execution. The PR is intentionally kept on hold (
[DO NOT MERGE]with/hold) and is being rehearsed via a downstream disconnected ARO periodic CI job.Key changes:
New downstream periodic job for disconnected ARO (
aro-ipi-kata-disconnected)Adds a new
testsjob in the 4.19 downstream candidate configuration to run the disconnected Azure ARO OSC e2e workflow on theazure-qecluster profile. It disables must-gather (ENABLE_MUST_GATHER: "false"), setsSLEEP_DURATION: 3h, narrows tests to non-disruptive viaTEST_FILTERS: ~Disruptive&, forcesTEST_PARALLEL: "1", includes reporter configuration, and sets a24h0m0stimeout. (restrict_network_accessis not set on the new job entry.)New disconnected ARO e2e workflow (
sandboxed-containers-operator-e2e-aro-disconnected)Introduces a workflow that provisions a disconnected/private ARO environment (resource group/VNet/bastion/proxy and network setup), configures required mirroring/disconnected parameters, runs the operator pre-chain, executes
openshift-extended-test, then runs the operator post-chain and deprovisions resources. The workflow supports disconnected routing/mirroring controls (including mirror registry DNS/base domain inputs) and configures must-gather behavior as failure-only (MUST_GATHER_ON_FAILURE_ONLY: "true"whileENABLE_MUST_GATHERstays off).New
mirror-operatormirroring step/implementationAdds a mirroring command script that authenticates to the target mirror registry and Red Hat operator registry, generates/runs
oc-mirrorv2 to mirror operator catalogs, applies generated disconnectedcluster-resources, optionally mirrors extra images, configures trust for the mirror registry host (trusted CA), disables default OperatorHub catalog sources, waits for required MCP/catalog readiness, and can record the discovered disconnectedCatalogSourcename for downstream use.Proxy-aware + disconnected catalog propagation across step scripts
Several step scripts now conditionally source
${SHARED_DIR}/proxy-conf.sh(when present and non-empty) to ensure proxy settings are applied early. Additionally, the env-cm commands script can read a disconnected catalog source name from a shared file and setCATALOG_SOURCE_NAMEaccordingly; this value is then used by other steps (must-gather, Kata RPM fetching, and record-metadata).Owners/metadata for the new disconnected workflow and mirroring step
Updates
OWNERSand adds/fills workflow/step metadata JSON for the new disconnected ARO workflow and the new mirroring reference, with consistent approver/reviewer lists across the added components.