From 0975c17c820dfeca3b9a2275e88d27525a24ee9f Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 2 Jul 2026 10:23:17 +0000 Subject: [PATCH] feat(pipelines): require shellIdentity on Shell steps in the schema 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 --- pipelines/types/pipeline.schema.v1.json | 3 ++- pipelines/types/pipeline_test.go | 12 ++++++++++++ pipelines/types/validation_test.go | 6 ++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pipelines/types/pipeline.schema.v1.json b/pipelines/types/pipeline.schema.v1.json index 3569cb28..cc6af36a 100644 --- a/pipelines/types/pipeline.schema.v1.json +++ b/pipelines/types/pipeline.schema.v1.json @@ -451,7 +451,8 @@ } }, "required": [ - "command" + "command", + "shellIdentity" ] }, "shellStep": { diff --git a/pipelines/types/pipeline_test.go b/pipelines/types/pipeline_test.go index 057c47aa..81c012c3 100644 --- a/pipelines/types/pipeline_test.go +++ b/pipelines/types/pipeline_test.go @@ -79,6 +79,8 @@ resourceGroups: - name: deploy action: Shell command: echo hello + shellIdentity: + value: existing-msi `, expectError: false, description: "backfillSubscriptionId alone should be valid", @@ -101,6 +103,8 @@ resourceGroups: - name: deploy action: Shell command: echo hello + shellIdentity: + value: existing-msi `, expectError: false, description: "displayName and roleAssignment alone should be valid", @@ -124,6 +128,8 @@ resourceGroups: - name: deploy action: Shell command: echo hello + shellIdentity: + value: existing-msi `, expectError: false, description: "backfillSubscriptionId with displayName should be valid (new behavior)", @@ -152,6 +158,8 @@ resourceGroups: - name: deploy action: Shell command: echo hello + shellIdentity: + value: existing-msi `, expectError: false, description: "backfillSubscriptionId with all other fields should be valid (new behavior)", @@ -173,6 +181,8 @@ resourceGroups: - name: deploy action: Shell command: echo hello + shellIdentity: + value: existing-msi `, expectError: true, description: "missing both required patterns should fail validation", @@ -194,6 +204,8 @@ resourceGroups: - name: deploy action: Shell command: echo hello + shellIdentity: + value: existing-msi `, expectError: true, description: "displayName without roleAssignment should fail validation", diff --git a/pipelines/types/validation_test.go b/pipelines/types/validation_test.go index b860c693..4ac6a206 100644 --- a/pipelines/types/validation_test.go +++ b/pipelines/types/validation_test.go @@ -303,6 +303,9 @@ func TestValidatePipelineSchema(t *testing.T) { "name": "step", "action": "Shell", "command": "echo hello", + "shellIdentity": map[string]interface{}{ + "value": "test-msi", + }, }, }, }, @@ -325,6 +328,9 @@ func TestValidatePipelineSchema(t *testing.T) { "action": "Shell", "command": "echo hello", "timeout": "75m", + "shellIdentity": map[string]interface{}{ + "value": "test-msi", + }, }, }, },