From c2a685ddb1217541547713ae3f2d257217c4be84 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 13 Apr 2021 19:38:44 +0200 Subject: [PATCH 1/4] feat(scaffolder): add support for parsing the valuues as a json object in the parameters Signed-off-by: blam --- .../src/scaffolder/tasks/TaskWorker.test.ts | 97 +++++++++++++++++-- .../src/scaffolder/tasks/TaskWorker.ts | 14 ++- 2 files changed, 103 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index b598b91036..014fdf2d99 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -41,20 +41,24 @@ async function createStore(): Promise { describe('TaskWorker', () => { let storage: DatabaseTaskStore; + let actionRegistry = new TemplateActionRegistry(); beforeAll(async () => { storage = await createStore(); }); - const logger = getVoidLogger(); - const actionRegistry = new TemplateActionRegistry(); - actionRegistry.register({ - id: 'test-action', - handler: async ctx => { - ctx.output('testOutput', 'winning'); - }, + beforeEach(() => { + actionRegistry = new TemplateActionRegistry(); + actionRegistry.register({ + id: 'test-action', + handler: async ctx => { + ctx.output('testOutput', 'winning'); + }, + }); }); + const logger = getVoidLogger(); + it('should fail when action does not exist', async () => { const broker = new StorageTaskBroker(storage, logger); const taskWorker = new TaskWorker({ @@ -166,4 +170,83 @@ describe('TaskWorker', () => { const event = events.find(e => e.type === 'completion'); expect((event?.body?.output as JsonObject).result).toBe('winning'); }); + + it('should parse strings as objects if possible', async () => { + const inputAction = createTemplateAction<{ + address: { line1: string }; + address2: string; + }>({ + id: 'test-input', + schema: { + input: { + type: 'object', + required: ['address'], + properties: { + address: { + title: 'address', + description: 'Enter name', + type: 'object', + properties: { + line1: { + type: 'string', + }, + }, + }, + address2: { + type: 'string', + }, + }, + }, + }, + async handler(ctx) { + if (ctx.input.address.line1 !== 'line 1') { + throw new Error( + `expected address.line1 to be "line 1" got ${ctx.input.address.line1}`, + ); + } + + if (ctx.input.address2 !== '{"not valid"}') { + throw new Error( + `expected address2 to be "{"not valid"}" got ${ctx.input.address2}`, + ); + } + ctx.output('address', ctx.input.address.line1); + }, + }); + actionRegistry.register(inputAction); + + const broker = new StorageTaskBroker(storage, logger); + const taskWorker = new TaskWorker({ + logger, + workingDirectory: os.tmpdir(), + actionRegistry, + taskBroker: broker, + }); + + const { taskId } = await broker.dispatch({ + steps: [ + { + id: 'test-input', + name: 'test-input', + action: 'test-input', + input: { + address: JSON.stringify({ line1: 'line 1' }), + address2: '{"not valid"}', + }, + }, + ], + output: { + result: '{{ steps.test-input.output.address }}', + }, + values: {}, + }); + + const task = await broker.claim(); + await taskWorker.runOneTask(task); + + const { events } = await storage.listEvents({ taskId }); + const event = events.find(e => e.type === 'completion'); + + expect((event?.body?.output as JsonObject).result).toBe('line 1'); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 3a846f79a5..aef9e22ada 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -102,13 +102,25 @@ export class TaskWorker { step.input && JSON.parse(JSON.stringify(step.input), (_key, value) => { if (typeof value === 'string') { - return handlebars.compile(value, { + const templated = handlebars.compile(value, { noEscape: true, strict: true, data: false, preventIndent: true, })(templateCtx); + + // If it smells like a JSON object then give it a parse as an object and if it fails return the string + if (templated.startsWith('{') && templated.endsWith('}')) { + try { + // Don't recursively JSON parse the values of this string. Shouldn't need to, don't want to encourage the use of returning handlebars from somewhere else + return JSON.parse(templated); + } catch { + return templated; + } + } + return templated; } + return value; }); From 096c0d85e06a654ac734e2bf98acc2342acdaee8 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 15 Apr 2021 11:59:09 +0200 Subject: [PATCH 2/4] feat(scaffolder): added parseRepoUrl helper to parse to json object Signed-off-by: blam --- .../actions/builtin/publish/util.ts | 10 +- .../src/scaffolder/tasks/TaskWorker.test.ts | 112 ++++++++++++++++++ .../src/scaffolder/tasks/TaskWorker.ts | 30 ++++- 3 files changed, 144 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts index 07b3823e7b..acf1351004 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.ts @@ -30,8 +30,14 @@ export const getRepoSourceDirectory = ( } return workspacePath; }; +export type RepoSpec = { + repo: string; + host: string; + owner: string; + organization?: string; +}; -export const parseRepoUrl = (repoUrl: string) => { +export const parseRepoUrl = (repoUrl: string): RepoSpec => { let parsed; try { parsed = new URL(`https://${repoUrl}`); @@ -55,7 +61,7 @@ export const parseRepoUrl = (repoUrl: string) => { ); } - const organization = parsed.searchParams.get('organization'); + const organization = parsed.searchParams.get('organization') ?? undefined; return { host, owner, repo, organization }; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 014fdf2d99..edef02064f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -24,6 +24,7 @@ import { ConfigReader, JsonObject } from '@backstage/config'; import { StorageTaskBroker } from './StorageTaskBroker'; import { DatabaseTaskStore } from './DatabaseTaskStore'; import { createTemplateAction, TemplateActionRegistry } from '../actions'; +import { RepoSpec } from '../actions/builtin/publish/util'; async function createStore(): Promise { const manager = SingleConnectionDatabaseManager.fromConfig( @@ -174,6 +175,7 @@ describe('TaskWorker', () => { it('should parse strings as objects if possible', async () => { const inputAction = createTemplateAction<{ address: { line1: string }; + list: string[]; address2: string; }>({ id: 'test-input', @@ -195,10 +197,21 @@ describe('TaskWorker', () => { address2: { type: 'string', }, + list: { + type: 'array', + items: { + type: 'string', + }, + }, }, }, }, async handler(ctx) { + if (ctx.input.list.length !== 1) { + throw new Error( + `expected list to have length "1" got ${ctx.input.list.length}`, + ); + } if (ctx.input.address.line1 !== 'line 1') { throw new Error( `expected address.line1 to be "line 1" got ${ctx.input.address.line1}`, @@ -231,6 +244,7 @@ describe('TaskWorker', () => { action: 'test-input', input: { address: JSON.stringify({ line1: 'line 1' }), + list: JSON.stringify(['hey!']), address2: '{"not valid"}', }, }, @@ -249,4 +263,102 @@ describe('TaskWorker', () => { expect((event?.body?.output as JsonObject).result).toBe('line 1'); }); + + // TODO(blam): Can delete this test when we make the helpers a public API + it('should provide a repoUrlParse helper for the templates', async () => { + const inputAction = createTemplateAction<{ + destination: RepoSpec; + }>({ + id: 'test-input', + schema: { + input: { + type: 'object', + required: ['destination'], + properties: { + destination: { + title: 'destination', + type: 'object', + properties: { + repo: { + type: 'string', + }, + host: { + type: 'string', + }, + owner: { + type: 'string', + }, + organization: { + type: 'string', + }, + }, + }, + }, + }, + }, + async handler(ctx) { + ctx.output('host', ctx.input.destination.host); + ctx.output('repo', ctx.input.destination.repo); + ctx.output('owner', ctx.input.destination.owner); + + if (ctx.input.destination.host !== 'github.com') { + throw new Error( + `expected host to be "github.com" got ${ctx.input.destination.host}`, + ); + } + + if (ctx.input.destination.repo !== 'repo') { + throw new Error( + `expected repo to be "repo" got ${ctx.input.destination.repo}`, + ); + } + + if (ctx.input.destination.owner !== 'owner') { + throw new Error( + `expected repo to be "owner" got ${ctx.input.destination.owner}`, + ); + } + }, + }); + actionRegistry.register(inputAction); + + const broker = new StorageTaskBroker(storage, logger); + const taskWorker = new TaskWorker({ + logger, + workingDirectory: os.tmpdir(), + actionRegistry, + taskBroker: broker, + }); + + const { taskId } = await broker.dispatch({ + steps: [ + { + id: 'test-input', + name: 'test-input', + action: 'test-input', + input: { + destination: '{{ parseRepoUrl parameters.repoUrl }}', + }, + }, + ], + output: { + host: '{{ steps.test-input.output.host }}', + repo: '{{ steps.test-input.output.repo }}', + owner: '{{ steps.test-input.output.owner }}', + }, + values: { + repoUrl: 'github.com?repo=repo&owner=owner', + }, + }); + + const task = await broker.claim(); + await taskWorker.runOneTask(task); + + const { events } = await storage.listEvents({ taskId }); + const event = events.find(e => e.type === 'completion'); + + expect((event?.body?.output as JsonObject).host).toBe('github.com'); + expect((event?.body?.output as JsonObject).repo).toBe('repo'); + expect((event?.body?.output as JsonObject).owner).toBe('owner'); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index aef9e22ada..14752e1988 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -23,8 +23,9 @@ import { TaskBroker, Task } from './types'; import fs from 'fs-extra'; import path from 'path'; import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; -import * as handlebars from 'handlebars'; +import * as Handlebars from 'handlebars'; import { InputError } from '@backstage/errors'; +import { parseRepoUrl } from '../actions/builtin/publish/util'; type Options = { logger: Logger; @@ -34,7 +35,20 @@ type Options = { }; export class TaskWorker { - constructor(private readonly options: Options) {} + private readonly handlebars: typeof Handlebars; + + constructor(private readonly options: Options) { + this.handlebars = Handlebars.create(); + + // TODO(blam): this should be a public facing API but it's a little + // scary right now, so we're going to lock it off like the component API is + // in the frontend until we can work out a nice way to do it. + this.handlebars.registerHelper('parseRepoUrl', repoUrl => { + return JSON.stringify(parseRepoUrl(repoUrl)); + }); + + this.handlebars.registerHelper('json', obj => JSON.stringify(obj)); + } start() { (async () => { @@ -102,7 +116,7 @@ export class TaskWorker { step.input && JSON.parse(JSON.stringify(step.input), (_key, value) => { if (typeof value === 'string') { - const templated = handlebars.compile(value, { + const templated = this.handlebars.compile(value, { noEscape: true, strict: true, data: false, @@ -110,9 +124,13 @@ export class TaskWorker { })(templateCtx); // If it smells like a JSON object then give it a parse as an object and if it fails return the string - if (templated.startsWith('{') && templated.endsWith('}')) { + if ( + (templated.startsWith('{') && templated.endsWith('}')) || + (templated.startsWith('[') && templated.endsWith(']')) + ) { try { - // Don't recursively JSON parse the values of this string. Shouldn't need to, don't want to encourage the use of returning handlebars from somewhere else + // Don't recursively JSON parse the values of this string. + // Shouldn't need to, don't want to encourage the use of returning handlebars from somewhere else return JSON.parse(templated); } catch { return templated; @@ -183,7 +201,7 @@ export class TaskWorker { JSON.stringify(task.spec.output), (_key, value) => { if (typeof value === 'string') { - return handlebars.compile(value, { + return this.handlebars.compile(value, { noEscape: true, strict: true, data: false, From 4de6b74cc0be6f332eb0869c421da18163df4ffb Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 15 Apr 2021 12:08:31 +0200 Subject: [PATCH 3/4] chore: update sample template to show how to Signed-off-by: blam --- .../sample-templates/v1beta2-demo/template.yaml | 1 + .../sample-templates/v1beta2-demo/template/catalog-info.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template.yaml b/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template.yaml index e90b43b7f3..1a01f7998c 100644 --- a/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template.yaml +++ b/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template.yaml @@ -50,6 +50,7 @@ spec: values: name: '{{ parameters.name }}' owner: '{{ parameters.owner }}' + destination: '{{ parseRepoUrl parameters.repoUrl }}' - id: fetch-docs name: Fetch Docs diff --git a/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml b/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml index 875664d2a8..6519d68402 100644 --- a/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml +++ b/plugins/scaffolder-backend/sample-templates/v1beta2-demo/template/catalog-info.yaml @@ -2,6 +2,7 @@ apiVersion: backstage.io/v1alpha1 kind: Component metadata: name: {{cookiecutter.name | jsonify}} + github.com/project-slug: {{cookiecutter.destination.owner + "/" + cookiecutter.destination.repo}} spec: type: website lifecycle: experimental From b25846562a69a67a3272ce19ddffdadf27dcfc25 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 15 Apr 2021 12:16:38 +0200 Subject: [PATCH 4/4] chore: add changeset Signed-off-by: blam --- .changeset/silent-papayas-dress.md | 38 ++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .changeset/silent-papayas-dress.md diff --git a/.changeset/silent-papayas-dress.md b/.changeset/silent-papayas-dress.md new file mode 100644 index 0000000000..c3c05971fe --- /dev/null +++ b/.changeset/silent-papayas-dress.md @@ -0,0 +1,38 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Enable the JSON parsing of the response from templated variables in the `v2beta1` syntax. Previously if template parameters json strings they were left as strings, they are now parsed as JSON objects. + +Before: + +```yaml +- id: test + name: test-action + action: custom:run + input: + input: '{"hello":"ben"}' +``` + +Now: + +```yaml +- id: test + name: test-action + action: custom:run + input: + input: + hello: ben +``` + +Also added the `parseRepoUrl` and `json` helpers to the parameters syntax. You can now use these helpers to parse work with some `json` or `repoUrl` strings in templates. + +```yaml +- id: test + name: test-action + action: cookiecutter:fetch + input: + destination: '{{ parseRepoUrl parameters.repoUrl }}' +``` + +Will produce a parsed version of the `repoUrl` of type `{ repo: string, owner: string, host: string }` that you can use in your actions. Specifically `cookiecutter` with `{{ cookiecutter.destination.owner }}` like the `plugins/scaffolder-backend/sample-templates/v1beta2-demo/template.yaml` example.