From 37ab71200193ab9ea08dcea6918cec18583dd3e8 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Sat, 11 Jan 2025 12:00:19 +0100 Subject: [PATCH 1/3] fix: add validation for the `each` values Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/fair-rocks-dream.md | 5 +++++ .../tasks/NunjucksWorkflowRunner.test.ts | 21 +++++++++++++++++++ .../tasks/NunjucksWorkflowRunner.ts | 20 ++++++++++++------ 3 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 .changeset/fair-rocks-dream.md diff --git a/.changeset/fair-rocks-dream.md b/.changeset/fair-rocks-dream.md new file mode 100644 index 0000000000..4d33a60bcd --- /dev/null +++ b/.changeset/fair-rocks-dream.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +add validation for `each` values diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 7e204d35df..31ba7195b2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -988,6 +988,27 @@ describe('NunjucksWorkflowRunner', () => { ); expect(fakeActionHandler).not.toHaveBeenCalled(); }); + + it('should validate each parameter renders to a valid value', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'scaffolder.backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + each: '${{parameters.data}}', + action: 'jest-validated-action', + input: { foo: '${{each.value}}' }, + }, + ], + output: {}, + parameters: {}, + }); + await expect(runner.execute(task)).rejects.toThrow( + 'Invalid each value passed to action jest-validated-action, "${{parameters.data}}" cannot be resolved to a value', + ); + expect(fakeActionHandler).not.toHaveBeenCalled(); + }); }); describe('secrets', () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index ad9d85cb2d..9865f4db87 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -304,13 +304,21 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { return; } } + + const resolvedEach = + step.each && this.render(step.each, context, renderTemplate); + + if (step.each && !resolvedEach) { + throw new InputError( + `Invalid each value passed to action ${action.id}, "${step.each}" cannot be resolved to a value`, + ); + } + const iterations = ( - step.each - ? Object.entries(this.render(step.each, context, renderTemplate)).map( - ([key, value]) => ({ - each: { key, value }, - }), - ) + resolvedEach + ? Object.entries(resolvedEach).map(([key, value]) => ({ + each: { key, value }, + })) : [{}] ).map(i => ({ ...i, From 9fc6ffc33e40d89c19d83a5dff4bc2ba52dd6708 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Wed, 15 Jan 2025 13:56:35 +0100 Subject: [PATCH 2/3] update changes message Co-authored-by: Vincenzo Scamporlino Signed-off-by: Juan Pablo Garcia Ripa --- .changeset/fair-rocks-dream.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/fair-rocks-dream.md b/.changeset/fair-rocks-dream.md index 4d33a60bcd..d1f7f3f0f7 100644 --- a/.changeset/fair-rocks-dream.md +++ b/.changeset/fair-rocks-dream.md @@ -2,4 +2,4 @@ '@backstage/plugin-scaffolder-backend': patch --- -add validation for `each` values +Fixed an issue where invalid expressions or non-object values in `step.each` caused an error. From 8ddb719564e1a1e1a18a099e76ad58666644aab1 Mon Sep 17 00:00:00 2001 From: Juan Pablo Garcia Ripa Date: Thu, 23 Jan 2025 21:23:36 +0100 Subject: [PATCH 3/3] make message clearer Signed-off-by: Juan Pablo Garcia Ripa --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts | 2 +- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 31ba7195b2..542a989d8a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -1005,7 +1005,7 @@ describe('NunjucksWorkflowRunner', () => { parameters: {}, }); await expect(runner.execute(task)).rejects.toThrow( - 'Invalid each value passed to action jest-validated-action, "${{parameters.data}}" cannot be resolved to a value', + 'Invalid value on action jest-validated-action.each parameter, "${{parameters.data}}" cannot be resolved to a value', ); expect(fakeActionHandler).not.toHaveBeenCalled(); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 9865f4db87..6fe92c0e2d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -310,7 +310,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { if (step.each && !resolvedEach) { throw new InputError( - `Invalid each value passed to action ${action.id}, "${step.each}" cannot be resolved to a value`, + `Invalid value on action ${action.id}.each parameter, "${step.each}" cannot be resolved to a value`, ); }