Skip to content

feat(pipelines): require shellIdentity on Shell steps in the schema (AROSLSRE-1380)#262

Merged
janboll merged 1 commit into
Azure:mainfrom
raelga:raelga/aroslsre-1380-require-shellidentity
Jul 3, 2026
Merged

feat(pipelines): require shellIdentity on Shell steps in the schema (AROSLSRE-1380)#262
janboll merged 1 commit into
Azure:mainfrom
raelga:raelga/aroslsre-1380-require-shellidentity

Conversation

@raelga

@raelga raelga commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Problem

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

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

But shellStepBase in pipeline.schema.v1.json only marks command as required, so consumers that validate against the schema (e.g. ARO-HCP make validate-config-pipelines) without running EV2RA generation accept Shell steps that omit an identity. They only fail later, at bump time — which just caused an ARO-HCP revert/reland (Azure/ARO-HCP#5834#5888 / #5889).

What changes

Add shellIdentity to shellStepBase.required:

"required": [
  "command",
  "shellIdentity"
]

This also covers shellValidationStep, which embeds the same base. The Go type (shell.go) already documented the field as required. Existing valid-shell test fixtures are updated to declare shellIdentity.

Validation

  • go test ./pipelines/types/... passes (schema + fixtures). The pipelines/graph failures in this environment are only missing graphviz/dot, unrelated to this change.

Follow-ups

  • Un-hold once ARO-HCP is compliant (reland #5889 + hardening #5890).
  • Optional: improve the runtime error to name the step and say shellIdentity is required.

A Shell step's shellIdentity is effectively mandatory: EV2RA manifest
generation validates it unconditionally and aborts if it is unset. But
shellStepBase only marked 'command' as required, so consumers that
validate against the schema (e.g. ARO-HCP 'make validate-config-pipelines')
without running EV2RA generation accepted Shell steps missing an identity.
Those only failed later, at bump time.

Add shellIdentity to shellStepBase.required so schema validation rejects
such steps at authoring/PR time. This also covers shellValidationStep,
which embeds the same base. The Go type already documented the field as
required.

Existing valid-shell test fixtures are updated to declare shellIdentity.

NOTE: breaking schema tightening. Must not merge until all consumers
declare shellIdentity on every Shell step (ARO-HCP tracked by
AROSLSRE-1380).

AROSLSRE-1380

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 aligns the pipeline.schema.v1 JSON schema with existing runtime expectations by making shellIdentity required for all Shell steps, preventing configs from passing schema validation and only failing later during EV2RA manifest generation.

Changes:

  • Updated shellStepBase.required in pipeline.schema.v1.json to include shellIdentity alongside command.
  • Updated schema-validation unit tests to include shellIdentity in valid Shell step examples.
  • Updated subscription provisioning schema validation tests to include shellIdentity in embedded Shell steps.

Reviewed changes

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

File Description
pipelines/types/validation_test.go Updates “valid shell” schema-validation test cases to include shellIdentity.
pipelines/types/pipeline.schema.v1.json Makes shellIdentity a required property for Shell steps via shellStepBase.required.
pipelines/types/pipeline_test.go Updates YAML test cases to include shellIdentity in Shell steps so they remain schema-compliant.

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

@janboll janboll merged commit 4612291 into Azure:main Jul 3, 2026
2 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants