Remove swft deploy environment by bennerv · Pull Request #4874 · Azure/ARO-HCP

bennerv · 2026-04-14T13:20:58Z

What

Remove the dedicated swft deploy environment and make the demo script unconditionally create the Swift subnet and delegation.

Why

All pers clusters now have Swift V2 enabled by default, making the separate swft environment redundant.

Testing

cd config; make materialize passes validation and rendering
Helm fixture tests pass
No remaining references to DEPLOY_ENV=swft in the codebase

Special notes for your reviewer

Changes:

Removed swft environment definition from tooling/templatize/settings.yaml
Removed swft environment config section from config/config.yaml
Removed rendered config config/rendered/dev/swft/uksouth.yaml
Updated demo/02-customer-infra.sh to always create the Swift subnet (removed conditional swift argument)
Updated dev-infrastructure/swift/README.md to reflect that pers environments have Swift V2 enabled by default

openshift-ci · 2026-04-14T13:21:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bennerv

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~config/OWNERS~~ [bennerv]
~~demo/OWNERS~~ [bennerv]
~~dev-infrastructure/OWNERS~~ [bennerv]
~~tooling/templatize/OWNERS~~ [bennerv]

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

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Removes the dedicated swft deploy environment and updates demo/dev-infra tooling so Swift V2 resources (integration subnet + delegation) are created unconditionally in personal environments.

Changes:

Removed swft environment configuration and its rendered output.
Updated demo scripts and Bicep templates to always provision a Swift integration subnet, supporting private Key Vault and explicit cluster/nodepool versions.
Updated Swift dev-infrastructure docs/env-vars to reflect the new default behavior in pers.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tooling/templatize/settings.yaml	Removes `swft` environment definition from templatize settings.
config/config.yaml	Removes the `swft` environment config block.
config/rendered/dev/swft/uksouth.yaml	Deletes rendered config for the removed `swft` environment.
demo/02-customer-infra.sh	Makes Swift subnet creation + delegation unconditional.
demo/env_vars	Renames Swift subnet env var and bumps API version used by demo.
demo/deploy-hcp-bicep.sh	Passes new parameters for integration subnet, KV visibility, and versions.
demo/bicep/customer-infra.bicep	Adds integration subnet + KV private endpoint and outputs.
demo/bicep/cluster.bicep	Adds integration subnet + KV visibility + cluster version; updates API version.
demo/bicep/nodepool.bicep	Adds node pool version parameter; updates API version.
dev-infrastructure/swift/README.md	Removes `swft` env instructions; updates demo steps.
dev-infrastructure/swift/swift_env_vars	Switches subnet env var to the integration subnet name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

demo/02-customer-infra.sh

demo/deploy-hcp-bicep.sh

demo/bicep/customer-infra.bicep

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

demo/02-customer-infra.sh

Copilot · 2026-04-14T13:57:07Z

demo/bicep/customer-infra.bicep

+resource privateEndpoint 'Microsoft.Network/privateEndpoints@2023-09-01' = if (privateKeyVault) {
+  name: 'kv-private-endpoint'


The private endpoint, DNS zone group, and VNet link resource names are effectively constant. This can cause deployment conflicts when creating more than one Key Vault/private endpoint or linking multiple VNets in the same resource group (names collide). Consider making these names unique per deployment (e.g., include randomSuffix, customerKeyVaultName, and/or customerVnetName in the name/uniqueString inputs) to keep the demo idempotent and allow multiple runs/environments.

Copilot · 2026-04-14T13:57:07Z

demo/bicep/customer-infra.bicep

+resource privateEndpointDnsGroup 'Microsoft.Network/privateEndpoints/privateDnsZoneGroups@2023-09-01' = if (privateKeyVault) {
+  name: 'kv-private-ep-dns-group'


The private endpoint, DNS zone group, and VNet link resource names are effectively constant. This can cause deployment conflicts when creating more than one Key Vault/private endpoint or linking multiple VNets in the same resource group (names collide). Consider making these names unique per deployment (e.g., include randomSuffix, customerKeyVaultName, and/or customerVnetName in the name/uniqueString inputs) to keep the demo idempotent and allow multiple runs/environments.

demo/bicep/customer-infra.bicep

dev-infrastructure/swift/README.md

…bled The dedicated swft environment is no longer needed since pers already has Swift V2 enabled by default. The demo script now unconditionally creates the swift subnet and delegation. Remove demo scripts that leverage local RP, instead we should use test/e2e directory with makefile target to run a e2e test against one of our scenarios

dev-infrastructure/swift/README.md

Remove references to deleted demo scripts from the swift README and point to demo/README.md and test/ e2e tests instead. Add az resource commands for common cluster operations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T16:15:06Z

demo/env_vars

 export CUSTOMER_VNET_SUBNET1="customer-subnet-1"
-export CUSTOMER_VNET_PODNETWORK1="customer-podnetwork-1"
+export CUSTOMER_VNET_INTEGRATION_SUBNET="customer-vnet-integration-subnet"
 export CUSTOMER_NSG="customer-nsg"


DEMO_ENV_NAME is no longer set in this file (the logic that initialized it was removed), but it’s still used to build CUSTOMER_KV_NAME. This will expand to an empty string and can lead to surprising names/collisions. Fix by reintroducing a default for DEMO_ENV_NAME (e.g., derived from ${DEPLOY_ENV:-pers}) or by removing DEMO_ENV_NAME from the Key Vault name construction and using a stable suffix.

Suggested change

export CUSTOMER_NSG="customer-nsg"

export CUSTOMER_NSG="customer-nsg"

if [ -z "${DEMO_ENV_NAME:-}" ]; then

export DEMO_ENV_NAME="${DEPLOY_ENV:-pers}"

fi

Copilot · 2026-04-14T16:15:07Z

demo/deploy-hcp-bicep.sh

    clusterName="${CLUSTER_NAME}" \
    managedResourceGroupName="${MANAGED_RESOURCE_GROUP}" \
-    keyVaultName="${KEYVAULT_NAME}"
+    keyVaultName="${KEYVAULT_NAME}" \
+    privateKeyVault=${PRIVATE_KEYVAULT} \
+    clusterVersion="${CLUSTER_VERSION}"


KEYVAULT_NAME is referenced but (based on the updated demo/env_vars) the variable appears to be CUSTOMER_KV_NAME. As written, this will pass an empty keyVaultName unless users export KEYVAULT_NAME manually. Recommend switching to the expected variable name (or defining KEYVAULT_NAME in env_vars to avoid breaking users).

Copilot · 2026-04-14T16:15:07Z

demo/deploy-hcp-bicep.sh

 source env_vars
-# The ONLY supported region for ARO-HCP in INT is uksouth
 LOCATION=${LOCATION:-uksouth}
 SUBSCRIPTION=$(az account show --query 'name' -o tsv)


The script uses the subscription name from az account show for --subscription. Subscription names can contain spaces and are not guaranteed unique, which can make automation flaky. Prefer using the subscription id (az account show --query id -o tsv) to make the script deterministic.

Suggested change

SUBSCRIPTION=$(az account show --query 'name' -o tsv)

SUBSCRIPTION=$(az account show --query 'id' -o tsv)

Copilot · 2026-04-14T16:15:07Z

demo/deploy-hcp-bicep.sh

 az group create \
  --name "${CUSTOMER_RG_NAME}" \
  --subscription "${SUBSCRIPTION}" \


The script uses the subscription name from az account show for --subscription. Subscription names can contain spaces and are not guaranteed unique, which can make automation flaky. Prefer using the subscription id (az account show --query id -o tsv) to make the script deterministic.

Copilot · 2026-04-14T16:15:07Z

dev-infrastructure/swift/README.md

+Basic instructions to get swift working in our dev environment. For this you will need access to our mock 1P app. The `login_fpa.sh` script will collect the certificate and log you in as the 1P app, its used to create/delete the service association link on the customer subnet.

-NOTE: Yes when you start creating the SAL, PotNetwork etc you will get logged in and out of Azure becuase we're switching between the mock 1p and the dev sub
+The sal_env_vars script stores some basic configuration that is specific to the dev environment (subscription). This would need to be modified for other environments such as INT


Two documentation issues: (1) grammar: “its used” should be “it’s used”; (2) the text references sal_env_vars, but the file in this directory is named swift_env_vars in this PR—update the name in the README to match the actual script users should edit/source.

Copilot · 2026-04-14T16:15:08Z

demo/README.md

 ```bash
-CS_CLUSTER_ID=$(curl localhost:8001/api/aro_hcp/v1alpha1/clusters | jq .items[0].id -r)
-kubectl get manifestwork -n local-cluster | grep ^${CS_CLUSTER_ID}
+az resource invoke-action --ids "${CLUSTER_RESOURCE_ID}" --action requestAdminCredential


This command likely needs an explicit --api-version to work reliably with preview resources/actions (especially since the README states these commands use 2025-12-23-preview). Without it, az resource invoke-action may pick an API version that doesn’t expose requestAdminCredential. Recommend adding --api-version 2025-12-23-preview (or whatever version your Bicep/templates use) to the command.

Suggested change

az resource invoke-action --ids "${CLUSTER_RESOURCE_ID}" --action requestAdminCredential

az resource invoke-action --ids "${CLUSTER_RESOURCE_ID}" --action requestAdminCredential --api-version 2025-12-23-preview

Copilot AI review requested due to automatic review settings April 14, 2026 13:21

openshift-ci bot requested review from geoberle and janboll April 14, 2026 13:21

openshift-ci bot added the approved label Apr 14, 2026

Copilot AI reviewed Apr 14, 2026

View reviewed changes

demo/02-customer-infra.sh Outdated Show resolved Hide resolved

demo/deploy-hcp-bicep.sh Outdated Show resolved Hide resolved

demo/bicep/customer-infra.bicep Outdated Show resolved Hide resolved

Copilot started reviewing on behalf of bennerv April 14, 2026 13:35 View session

bennerv force-pushed the bvesel/remove-swift-environment branch 2 times, most recently from 169d35f to 5be2403 Compare April 14, 2026 13:49

Copilot AI review requested due to automatic review settings April 14, 2026 13:49

Copilot AI reviewed Apr 14, 2026

View reviewed changes

bennerv mentioned this pull request Apr 14, 2026

Switch demo cluster to swift #4833

Closed

Copilot started reviewing on behalf of bennerv April 14, 2026 14:08 View session

bennerv force-pushed the bvesel/remove-swift-environment branch from 5be2403 to b314ed2 Compare April 14, 2026 14:50

bennerv mentioned this pull request Apr 14, 2026

demo: add mandatory version information to nodepool and cluster resources #4369

Closed

tony-schndr reviewed Apr 14, 2026

View reviewed changes

dev-infrastructure/swift/README.md Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings April 14, 2026 16:08

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Copilot started reviewing on behalf of bennerv April 14, 2026 17:21 View session

		resource privateEndpoint 'Microsoft.Network/privateEndpoints@2023-09-01' = if (privateKeyVault) {
		name: 'kv-private-endpoint'

		resource privateEndpointDnsGroup 'Microsoft.Network/privateEndpoints/privateDnsZoneGroups@2023-09-01' = if (privateKeyVault) {
		name: 'kv-private-ep-dns-group'

	SUBSCRIPTION=$(az account show --query 'name' -o tsv)
	SUBSCRIPTION=$(az account show --query 'id' -o tsv)

	az resource invoke-action --ids "${CLUSTER_RESOURCE_ID}" --action requestAdminCredential
	az resource invoke-action --ids "${CLUSTER_RESOURCE_ID}" --action requestAdminCredential --api-version 2025-12-23-preview

Conversation

bennerv commented Apr 14, 2026

What

Why

Testing

Special notes for your reviewer

Uh oh!

openshift-ci bot commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants