You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.69%. Comparing base (10310a5) to head (5906de7). ⚠️ Report is 1 commits behind head on main.
The following are alternative approaches to this PR:
1. Declarative Azure IaC
➕ Provides idempotent plans, dependency tracking, and clearer teardown semantics.
➕ Enables policy validation and reusable environment modules.
➖ Adds Terraform or Bicep state and tooling overhead for a manual-only workflow.
➖ May diverge from the command-oriented upstream ARO disconnected installation guide.
Recommendation: The shell-based approach is appropriate for migrating a manual, guide-aligned workflow with limited CI integration. If provisioning becomes frequent or shared across environments, migrate the Azure resource layer to Bicep or Terraform while retaining the bastion setup and verification scripts.
Files changed (9) +856 / -1
Tests (2) +126 / -0
test-firewall-rules.shTest firewall rule iteration and replacement+100/-0
Test firewall rule iteration and replacement
• Uses a mocked Azure CLI to verify every configured rule is created, collection action and priority are applied once, and confirmed replacement deletes then recreates the collection.
README.mdDocument the manual disconnected ARO workflow+94/-0
Document the manual disconnected ARO workflow
• Explains prerequisites, configuration, provisioning, firewall updates, bastion setup, security expectations, upstream guidance, and resource-group cleanup. It also warns that restricted outbound traffic can conflict with ARO support policy.
• Adds a local Azure CLI workflow that validates prerequisites and the target firewall, prompts before replacing an existing collection, and iterates synchronized rule names and FQDN lists. Collection action and priority are set only when creating the first rule.
install-aro-disconnected.shProvision a secured disconnected ARO environment+312/-0
Provision a secured disconnected ARO environment
• Creates the resource group, segmented VNet, Azure Firewall, routes, private ARO cluster, restricted bastion NSG, and bastion VM. It validates configuration, removes temporary installer egress, stores SSH keys under an ignored local directory, and writes credentials to a [mode-0600](https://redhat.atlassian.net/browse/mode-0600) access file.
setup-bastion.shInstall verified cluster tooling on the bastion+133/-0
Install verified cluster tooling on the bastion
• Installs pinned oc, Helm, opm, and umoci artifacts after SHA-256 verification, plus Podman and Skopeo from system packages. It logs into ARO, checks outbound connectivity, enables the internal registry, and expands bastion logical volumes.
1. Default cluster name invalid 🐞 Bug≡ Correctness
Description
The example configuration derives CLUSTER as aro-disconnected_6, but ARO cluster names do not
permit underscores, so the shipped workflow fails at az aro create after provisioning the network
and firewall. The installer validates the bastion name but never rejects the invalid cluster name
before creating resources.
Invalid default cluster naming is a deterministic provisioning failure; accepted history favors
fixing concrete configuration defects.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The example combines BASE_NAME and SUFFIX with _, producing aro-disconnected_6; that value
is passed unchanged as the ARO name after all network and firewall resources have already been
created.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The default `CLUSTER` value contains an underscore and is not a valid ARO cluster name, causing the installer to fail after creating prerequisite Azure resources.
## Issue Context
Use a valid separator in the example and validate `CLUSTER` before any Azure resources are created.
## Fix Focus Areas
- .ci/manual/aro-disconnected/.env.example[5-10]
- .ci/manual/aro-disconnected/install-aro-disconnected.sh[55-77]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The bastion source check accepts impossible values such as 999.999.999.999/99 because it validates
only digit shape, not IPv4 octet or CIDR ranges. Such input reaches the NSG rule only after the ARO
cluster has been created, where Azure rejects it and the script exits without creating the bastion.
+if [[ ! "$BASTION_SOURCE_ADDRESS_PREFIX" =~ ^([0-9]{1,3}\.){3}[0-9]{1,3}(/[0-9]{1,2})?$ ]]; then+ printf 'BASTION_SOURCE_ADDRESS_PREFIX must be an IPv4 address or CIDR: %s\n' \+ "$BASTION_SOURCE_ADDRESS_PREFIX" >&2+ exit 1
Relevance
●●● Strong
Regex-only IP validation admits invalid Azure NSG inputs; this is a concrete correctness defect with
an obvious local fix.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The regular expression permits any one-to-three-digit octet and any one-to-two-digit prefix; the
unchecked value is later supplied to az network nsg rule create, after az aro create completes.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`BASTION_SOURCE_ADDRESS_PREFIX` validation accepts out-of-range IPv4 octets and CIDR lengths, allowing a late installer failure.
## Issue Context
Reject octets above 255 and prefix lengths above 32 before any Azure resources are created.
## Fix Focus Areas
- .ci/manual/aro-disconnected/install-aro-disconnected.sh[67-71]
- .ci/manual/aro-disconnected/test-install-configuration.sh[7-25]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Password exposed in argv 🐞 Bug⛨ Security
Description
setup-bastion.sh expands the kubeadmin password into oc login's command-line arguments, making
the cluster-admin credential visible through process inspection while the command runs. This defeats
the PR's credential-protection goal on a bastion with any additional local user or process
visibility.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The README instructs the operator to export KUBEADMIN_PASSWORD, and the setup script then
interpolates that secret directly after the --password option.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The kubeadmin password is passed directly in `oc login` process arguments and can be observed by other local processes while login runs.
## Issue Context
Authenticate without placing the secret in argv, for example by using a protected temporary request/config mechanism supported by the workflow, and securely remove temporary credential material.
## Fix Focus Areas
- .ci/manual/aro-disconnected/setup-bastion.sh[88-90]
- .ci/manual/aro-disconnected/README.md[71-79]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
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.
zdrapela
changed the title
Summary
.ci/manual/aro-disconnected/.Testing
bash .ci/manual/aro-disconnected/test-firewall-rules.shbash .ci/manual/aro-disconnected/test-install-configuration.shbash -n .ci/manual/aro-disconnected/*.shyarn shellcheckandyarn prettier:checkfrom.ci