Skip to content

revert: Swift VNet managed-identity container step breaks EV2 manifest generation (ARO-28045)#5888

Merged
raelga merged 1 commit into
Azure:mainfrom
raelga:rael/revert-5834-swift-vnet-ev2-ARO-28045
Jul 2, 2026
Merged

revert: Swift VNet managed-identity container step breaks EV2 manifest generation (ARO-28045)#5888
raelga merged 1 commit into
Azure:mainfrom
raelga:rael/revert-5834-swift-vnet-ev2-ARO-28045

Conversation

@raelga

@raelga raelga commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

ARO-28045 (epic ARO-28038)

Reverts #5834.

What

Reverts the Swift VNet migration from #5834, restoring the previous deploymentScripts-based create/tag in modules/network/vnet.bicep and removing the swift-vnet / swift-vnet-permissions pipeline steps, scripts/swift-vnet.sh, and the vnet-rbac.bicep module.

Why

The swift-vnet Shell step added by #5834 does not declare a shellIdentity. sdp-pipelines EV2RA manifest generation validates every Shell step's shellIdentity unconditionally (tooling/pkg/types/pipeline/shell.goValidateVariable(&s.ShellIdentity)), so the ARO-HCP bump fails at manifest resolution:

step Management.Infra.management.swift-vnet: error validating step:
invalid shell identity: variable must specify config ref, value, or input chain

ARO-HCP CI does not run the sdp-pipelines EV2RA generation, so this was not caught pre-merge — same class of issue as #5887 (a step field accepted by templatize's local validation but rejected by EV2RA). Reverting unblocks the bump; a follow-up PR relands #5834 with a shellIdentity (globalMSI) on the swift-vnet step in both mgmt-pipeline.yaml and mgmt-solo-pipeline.yaml.

Note: this temporarily restores the Azure-Policy-banned deploymentScripts path (SFI-ID4.2.1). That is the state currently deployed everywhere (nothing from #5834 has rolled out, since the bump fails before deploy), so the revert does not introduce new runtime breakage — it only restores the last bump-passing state while the fixed version is prepared.

Testing

Special notes for your reviewer

Emergency revert to unblock the EV2 bump. The proper fix (add shellIdentity: globalMSIId to the swift-vnet step) is small and will land as a follow-up reland PR referencing this one.

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented

…t generation (ARO-28045)

Reverts Azure#5834.

The swift-vnet Shell step introduced by Azure#5834 omits the required
shellIdentity field. sdp-pipelines EV2RA manifest generation validates
every Shell step's shellIdentity (tooling/pkg/types/pipeline/shell.go),
so the bump fails at manifest resolution with:

  step Management.Infra.management.swift-vnet: error validating step:
  invalid shell identity: variable must specify config ref, value, or input chain

Reverting to unblock the bump. A follow-up PR relands the change with a
shellIdentity (globalMSIId) on the swift-vnet step in both mgmt-pipeline.yaml
and mgmt-solo-pipeline.yaml.
Copilot AI review requested due to automatic review settings July 2, 2026 09:55
@openshift-ci openshift-ci Bot added the approved label Jul 2, 2026
@openshift-ci openshift-ci Bot requested review from avollmer-redhat and mmazur July 2, 2026 09:55
@geoberle

geoberle commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

/lgtm

@openshift-ci

openshift-ci Bot commented Jul 2, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geoberle, raelga

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:

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reverts the Swift VNet migration introduced in #5834 to unblock EV2 manifest generation failures caused by a Shell step missing shellIdentity, restoring the prior deploymentScripts-based Swift VNet create/tag flow.

Changes:

  • Removes the swift-vnet / swift-vnet-permissions pipeline steps and deletes the associated script/template/bicepparam artifacts.
  • Restores Swift VNet creation/tagging via Microsoft.Resources/deploymentScripts in modules/network/vnet.bicep.
  • Updates vnet.bicep callers to pass deploymentMsiId.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
dev-infrastructure/templates/swift-vnet-permissions.bicep Deleted (revert removes the dedicated Swift RBAC template).
dev-infrastructure/templates/svc-cluster.bicep Passes deploymentMsiId into the VNet module call.
dev-infrastructure/templates/opstool-cluster.bicep Passes deploymentMsiId into the VNet module call.
dev-infrastructure/templates/mgmt-cluster.bicep Passes deploymentMsiId into the VNet module call.
dev-infrastructure/scripts/swift-vnet.sh Deleted (revert removes the container-based Swift VNet logic).
dev-infrastructure/modules/network/vnet.bicep Reintroduces deployment script path for Swift VNet tagging/creation and RBAC setup.
dev-infrastructure/modules/network/vnet-rbac.bicep Deleted (revert inlines/relocates Swift-related RBAC).
dev-infrastructure/mgmt-solo-pipeline.yaml Removes Swift VNet pipeline steps (and related subscription-output wiring).
dev-infrastructure/mgmt-pipeline.yaml Removes Swift VNet pipeline steps.
dev-infrastructure/configurations/swift-vnet-permissions.tmpl.bicepparam Deleted (no longer needed after removing Swift RBAC step).

Comment thread dev-infrastructure/modules/network/vnet.bicep
Comment thread dev-infrastructure/modules/network/vnet.bicep
@raelga

raelga commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

Force-merge justification

This is a clean revert force-merged to unblock the int/stg/prod rollout. It meets all four criteria from the alert-latch thread:

1. int/stg/prod is blocked by the problem.
The Global bump (sdp-pipelines!16310885, build 170683084) fails at EV2 manifest resolution — Resolve Step Manifests for Microsoft.Azure.ARO.HCP.Global aborts with:

failed to generate manifests for step: step Management.Infra.management.swift-vnet:
error validating step: invalid shell identity:
variable must specify config ref, value, or input chain

No ARO-HCP change can reach int until the bump succeeds, so the rollout is blocked. The merge queue itself is not materially disrupted — the block is downstream in the rollout pipeline.

2. High confidence the revert fixes the problem.
Root cause is the reverted PR #5834: its swift-vnet Shell step omits the shellIdentity field, which sdp-pipelines EV2RA manifest generation validates unconditionally. ARO-HCP CI never runs EV2RA generation, so it passed all PR checks and only broke at bump time. Reverting removes the offending step entirely, which restores manifest generation.

3. The revert builds and passes all fast testing.
Clean revert; all fast checks are green (lint, mega-linter, bicep-lint, verify, test-unit, integration, images, config-change-detection, CodeQL, CLA). Only the slow ci/prow/e2e-parallel (and tide) remain — which is exactly the gate this FM bypasses.

4. The revert is clean AND done by the original author.
Straight revert of #5834, authored by @raelga — the same author as #5834.

The durable fix relands #5834 with shellIdentity declared (#5889), plus repo-wide hardening (#5890) and a schema change to make shellIdentity required (Azure/ARO-Tools#262, AROSLSRE-1380). Those go through the queue normally.

@raelga raelga merged commit 3a49793 into Azure:main Jul 2, 2026
15 of 17 checks passed
raelga added a commit to raelga/ARO-HCP that referenced this pull request Jul 2, 2026
…ellIdentity (ARO-28045)

Relands Azure#5834 (reverted in Azure#5888) with the missing shellIdentity so EV2
manifest generation succeeds.

Azure#5834 created/tagged the Swift management VNet via a managed-identity ACI
launched from a synchronous `swift-vnet` Shell step, replacing the
Azure-Policy-banned deploymentScripts. It was reverted because that Shell
step omitted `shellIdentity`, which sdp-pipelines EV2RA manifest
generation validates unconditionally:

    step Management.Infra.management.swift-vnet: error validating step:
    invalid shell identity: variable must specify config ref, value, or input chain

ARO-HCP CI does not run EV2RA generation, so it passed all PR checks and
only broke the Global bump after merge.

Declare shellIdentity: globalMSIId on the Shell steps in both
mgmt-pipeline.yaml and mgmt-solo-pipeline.yaml (swift-vnet in both, plus
the pre-existing `fixes` step in mgmt-solo-pipeline.yaml which had the
same omission). In EV2 the outer shell runs as globalMSI (harmless: the
container still performs the create/tag); templatize's local executor
ignores shellIdentity, so dev/CI behaviour is unchanged.

AROSLSRE-1380 makes shellIdentity required in the ARO-Tools pipeline
schema so this class of bug fails at PR time.
raelga added a commit to raelga/ARO-HCP that referenced this pull request Jul 2, 2026
Relands Azure#5834 (reverted in Azure#5888) unchanged.

Creates and tags the Swift management VNet via a managed-identity ACI
launched from a synchronous `swift-vnet` Shell step, replacing the
Azure-Policy-banned deploymentScripts approach.

The follow-up commit adds the shellIdentity that was missing from the
Shell step and caused the original revert.
raelga added a commit to raelga/ARO-HCP that referenced this pull request Jul 2, 2026
A Shell step's shellIdentity is validated unconditionally by sdp-pipelines
EV2RA manifest generation:

    step Management.Infra.management.swift-vnet: error validating step:
    invalid shell identity: variable must specify config ref, value, or input chain

ARO-HCP CI does not run EV2RA generation, so the omission in Azure#5834 passed
all PR checks and only broke the Global bump after merge (the reason for
the Azure#5888 revert).

Declare shellIdentity: globalMSIId on the Shell steps in both
mgmt-pipeline.yaml and mgmt-solo-pipeline.yaml (swift-vnet in both, plus
the pre-existing `fixes` step in mgmt-solo-pipeline.yaml which had the
same omission). In EV2 the outer shell runs as globalMSI (harmless: the
container still performs the create/tag); templatize's local executor
ignores shellIdentity, so dev/CI behaviour is unchanged.

AROSLSRE-1380 makes shellIdentity required in the ARO-Tools pipeline
schema so this class of bug fails at PR time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants