From 92405f32bb0cf4acd0d860b354fca411281f62ee Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Nov 2021 13:55:28 +0100 Subject: [PATCH 1/3] bug: should check the children of the first parse, not the root. Signed-off-by: blam --- ...WorkflowRunner.test.ts => NunjucksWorkflowRunner.test.ts} | 0 .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 5 +++-- 2 files changed, 3 insertions(+), 2 deletions(-) rename plugins/scaffolder-backend/src/scaffolder/tasks/{DefaultWorkflowRunner.test.ts => NunjucksWorkflowRunner.test.ts} (100%) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index c66d967103..1139ce41c5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -114,9 +114,10 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { private isSingleTemplateString(input: string) { const { parser, nodes } = require('nunjucks'); const parsed = parser.parse(input, {}, this.nunjucksOptions); + return ( parsed.children.length === 1 && - !(parsed.children[0] instanceof nodes.TemplateData) + !(parsed.children[0]?.children?.[0] instanceof nodes.TemplateData) ); } @@ -147,7 +148,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { return JSON.parse(templated); } } catch (ex) { - this.options.logger.error( + console.error( `Failed to parse template string: ${value} with error ${ex.message}`, ); } From 5fd0e077bc3e38cf98dfb172690b4fcdbab4a1d1 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Nov 2021 14:01:26 +0100 Subject: [PATCH 2/3] chore: adding test for this bug Signed-off-by: blam --- .../tasks/NunjucksWorkflowRunner.test.ts | 25 +++++++++++++++++++ .../tasks/NunjucksWorkflowRunner.ts | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 1f7877791c..b2e2e85a6c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -270,6 +270,31 @@ describe('DefaultWorkflowRunner', () => { ); }); + it('should not try and parse something that is not parsable', async () => { + jest.spyOn(logger, 'error'); + const task = createMockTaskWithSpec({ + apiVersion: 'scaffolder.backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: { + foo: 'bob', + }, + }, + ], + output: {}, + parameters: { + input: 'BACKSTAGE', + }, + }); + + await runner.execute(task); + + expect(logger.error).not.toHaveBeenCalled(); + }); + it('should keep the original types for the input and not parse things that arent meant to be parsed', async () => { const task = createMockTaskWithSpec({ apiVersion: 'scaffolder.backstage.io/v1beta3', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 1139ce41c5..5b42eef459 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -148,7 +148,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { return JSON.parse(templated); } } catch (ex) { - console.error( + this.options.logger.error( `Failed to parse template string: ${value} with error ${ex.message}`, ); } From e634a47ce50236b65a73140f0c4b0c7fbd559c1f Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 24 Nov 2021 14:03:22 +0100 Subject: [PATCH 3/3] chore: added changeset Signed-off-by: blam --- .changeset/shiny-insects-remember.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/shiny-insects-remember.md diff --git a/.changeset/shiny-insects-remember.md b/.changeset/shiny-insects-remember.md new file mode 100644 index 0000000000..a26882b65d --- /dev/null +++ b/.changeset/shiny-insects-remember.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Fix bug where there was error log lines written when failing to `JSON.parse` things that were not `JSON` values.