Skip to content

fix(pipelines): declare shellIdentity on dev-ci ci-bot-secret Shell steps (AROSLSRE-1380)#5890

Merged
openshift-merge-bot[bot] merged 1 commit into
Azure:mainfrom
raelga:rael/harden-shellidentity-AROSLSRE-1380
Jul 2, 2026
Merged

fix(pipelines): declare shellIdentity on dev-ci ci-bot-secret Shell steps (AROSLSRE-1380)#5890
openshift-merge-bot[bot] merged 1 commit into
Azure:mainfrom
raelga:rael/harden-shellidentity-AROSLSRE-1380

Conversation

@raelga

@raelga raelga commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

AROSLSRE-1380

What

Declare shellIdentity on the dev-ci Shell steps that omit it:

  • ci-bot-secret-{int,stg,prod} in dev-infrastructure/dev-ci/e2e-subscription-rbac/pipeline.yaml → reference a new outputOnly global-identity step (using the already-present output-opstool-global-identity.bicep + its bicepparam) to expose globalMSIId.

The swift-vnet and mgmt-solo-pipeline.yaml fixes Shell steps are handled in the reland PR #5889 (which already edits both mgmt pipeline files). Together with this PR, every Shell step in the repo declares shellIdentity.

Why

A Shell step's shellIdentity is effectively mandatory — sdp-pipelines EV2RA manifest generation validates it unconditionally:

step ...: error validating step:
invalid shell identity: variable must specify config ref, value, or input chain

ARO-HCP CI never runs that generation, so a Shell step missing shellIdentity passes all checks and only breaks the bump after merge (as happened with #5834, ARO-28045). This PR makes the dev-ci pipeline compliant so AROSLSRE-1380 can make shellIdentity required in the ARO-Tools pipeline schema and catch this class of bug at PR time.

shellIdentity is consumed only by EV2; templatize's local executor ignores it, so dev/CI runtime behaviour is unchanged.

Testing

  • yamllint -c .yamllint.yml (clean).
  • templatize pipeline validate against topology-dev-ci.yaml — passes, confirming the new global-identity output step and the shellIdentity input chains resolve.

Special notes for your reviewer

global-identity is outputOnly: true — it deploys nothing, it only surfaces the existing global MSI's resource ID (via existing lookup) so the dev-ci Shell steps can reference it.

PR Checklist

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

Copilot AI review requested due to automatic review settings July 2, 2026 10:19
@openshift-ci openshift-ci Bot added the approved label Jul 2, 2026

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 makes ARO-HCP pipeline YAMLs schema-compliant by ensuring all remaining Shell steps declare a shellIdentity, preventing EV2RA manifest-generation failures caused by missing shell identity variables.

Changes:

  • Add shellIdentity to the fixes Shell step in mgmt-solo-pipeline.yaml, sourcing globalMSIId from the existing global/output step.
  • Add a new outputOnly global-identity ARM step in the dev-ci E2E subscription RBAC pipeline to surface globalMSIId.
  • Wire shellIdentity for the ci-bot-secret-{int,stg,prod} Shell steps to the new global-identity step output.

Reviewed changes

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

File Description
dev-infrastructure/mgmt-solo-pipeline.yaml Adds missing shellIdentity to the remaining Shell step (fixes) using the existing globalMSIId output chain.
dev-infrastructure/dev-ci/e2e-subscription-rbac/pipeline.yaml Introduces an output-only identity lookup step and updates Shell steps to reference it for shellIdentity.

…teps

Every Shell step's shellIdentity is effectively mandatory: sdp-pipelines
EV2RA manifest generation validates it unconditionally. ARO-HCP CI does
not run that generation, so Shell steps missing shellIdentity slip
through and only fail the bump after merge (as happened in Azure#5834).

Declare shellIdentity on the dev-ci offenders so the repo is fully
compliant ahead of making shellIdentity required in the ARO-Tools
pipeline schema (AROSLSRE-1380):

- dev-ci/e2e-subscription-rbac 'ci-bot-secret-{int,stg,prod}': reference
  a new outputOnly 'global-identity' step (output-opstool-global-identity.bicep,
  which already existed with its bicepparam) to expose globalMSIId.

shellIdentity is only consumed by EV2; templatize's local executor
ignores it, so dev/CI behaviour is unchanged.

The swift-vnet and mgmt-solo 'fixes' Shell steps are handled in the
reland PR Azure#5889, which already edits both mgmt pipeline files.

Validated with 'templatize pipeline validate' against topology-dev-ci.yaml.

AROSLSRE-1380
@raelga raelga force-pushed the rael/harden-shellidentity-AROSLSRE-1380 branch from 74fae53 to b0cd5ef Compare July 2, 2026 10:29
@raelga raelga changed the title fix(pipelines): declare shellIdentity on remaining Shell steps (AROSLSRE-1380) fix(pipelines): declare shellIdentity on dev-ci ci-bot-secret Shell steps (AROSLSRE-1380) Jul 2, 2026
@raelga raelga requested a review from Copilot July 2, 2026 10:30

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@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

@openshift-merge-bot openshift-merge-bot Bot merged commit 73cd26f into Azure:main Jul 2, 2026
17 checks passed
raelga added a commit to raelga/ARO-HCP that referenced this pull request Jul 3, 2026
…SLSRE-1380)

Bumps the github.com/Azure/ARO-Tools modules from 2277df76598b (2026-06-17)
to 4612291d5420 (2026-07-03), which makes shellIdentity a required field on
shellStepBase in the pipeline schema (Azure/ARO-Tools#262).

This lands the enforcement so that a Shell step missing shellIdentity now
fails ARO-HCP 'make validate-config-pipelines' at PR time, instead of only
breaking the sdp-pipelines EV2 bump after merge (the regression that hit
ARO-28045 / Azure#5834).

All in-repo pipelines validate cleanly against the stricter schema; the known
offenders were already fixed in Azure#5889 and Azure#5890.
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