From b2113723d0192f24014c01c6cc72db3a8e3d8d2b Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 22 Sep 2021 17:47:09 +0200 Subject: [PATCH 01/29] feat(catalog-model): adding `TemplateEntityV1Beta3` as a kind in `catalog-model` Signed-off-by: blam --- .../src/kinds/TemplateEntityV1beta3.test.ts | 160 +++++++++++++++ .../src/kinds/TemplateEntityV1beta3.ts | 43 +++++ .../schema/kinds/Template.v1beta3.schema.json | 182 ++++++++++++++++++ 3 files changed, 385 insertions(+) create mode 100644 packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts create mode 100644 packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts create mode 100644 packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json diff --git a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts new file mode 100644 index 0000000000..092dfd7c7b --- /dev/null +++ b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts @@ -0,0 +1,160 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + TemplateEntityV1beta3, + templateEntityV1beta3Validator as validator, +} from './TemplateEntityV1beta3'; + +describe('templateEntityV1beta3Validator', () => { + let entity: TemplateEntityV1beta3; + + beforeEach(() => { + entity = { + apiVersion: 'backstage.io/v1beta3', + kind: 'Template', + metadata: { + name: 'test', + }, + spec: { + parameters: { + required: ['storePath', 'owner'], + properties: { + owner: { + type: 'string', + title: 'Owner', + description: 'Who is going to own this component', + }, + storePath: { + type: 'string', + title: 'Store path', + description: 'GitHub store path in org/repo format', + }, + }, + }, + steps: [ + { + id: 'fetch', + name: 'Fetch', + action: 'fetch:plan', + input: { + url: './template', + }, + if: '${{ parameters.owner }}', + }, + ], + output: { + fetchUrl: '${{ steps.fetch.output.targetUrl }}', + }, + owner: 'team-b@example.com', + }, + }; + }); + + it('happy path: accepts valid data', async () => { + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('ignores unknown apiVersion', async () => { + (entity as any).apiVersion = 'backstage.io/v1beta0'; + await expect(validator.check(entity)).resolves.toBe(false); + }); + + it('ignores unknown kind', async () => { + (entity as any).kind = 'Wizard'; + await expect(validator.check(entity)).resolves.toBe(false); + }); + + it('rejects missing type', async () => { + delete (entity as any).spec.type; + await expect(validator.check(entity)).rejects.toThrow(/type/); + }); + + it('accepts any other type', async () => { + (entity as any).spec.type = 'hallo'; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('accepts missing parameters', async () => { + delete (entity as any).spec.parameters; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('accepts missing outputs', async () => { + delete (entity as any).spec.outputs; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('rejects empty type', async () => { + (entity as any).spec.type = ''; + await expect(validator.check(entity)).rejects.toThrow(/type/); + }); + + it('rejects missing steps', async () => { + delete (entity as any).spec.steps; + await expect(validator.check(entity)).rejects.toThrow(/steps/); + }); + + it('accepts step with missing id', async () => { + delete (entity as any).spec.steps[0].id; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('accepts step with missing name', async () => { + delete (entity as any).spec.steps[0].name; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('rejects step with missing action', async () => { + delete (entity as any).spec.steps[0].action; + await expect(validator.check(entity)).rejects.toThrow(/action/); + }); + + it('accepts missing owner', async () => { + delete (entity as any).spec.owner; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('rejects empty owner', async () => { + (entity as any).spec.owner = ''; + await expect(validator.check(entity)).rejects.toThrow(/owner/); + }); + + it('rejects wrong type owner', async () => { + (entity as any).spec.owner = 5; + await expect(validator.check(entity)).rejects.toThrow(/owner/); + }); + + it('accepts missing if', async () => { + delete (entity as any).spec.steps[0].if; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('accepts boolean in if', async () => { + (entity as any).spec.steps[0].if = true; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('accepts empty if', async () => { + (entity as any).spec.steps[0].if = ''; + await expect(validator.check(entity)).resolves.toBe(true); + }); + + it('rejects wrong type if', async () => { + (entity as any).spec.steps[0].if = 5; + await expect(validator.check(entity)).rejects.toThrow(/if/); + }); +}); diff --git a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts new file mode 100644 index 0000000000..2da9a0b6b8 --- /dev/null +++ b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts @@ -0,0 +1,43 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { JsonObject } from '@backstage/config'; +import type { Entity } from '../entity/Entity'; +import schema from '../schema/kinds/Template.v1beta3.schema.json'; +import { ajvCompiledJsonSchemaValidator } from './util'; + +/** @public */ +export interface TemplateEntityV1beta3 extends Entity { + apiVersion: 'backstage.io/v1beta3'; + kind: 'Template'; + spec: { + type: string; + parameters?: JsonObject | JsonObject[]; + steps: Array<{ + id?: string; + name?: string; + action: string; + input?: JsonObject; + if?: string | boolean; + }>; + output?: { [name: string]: string }; + owner?: string; + }; +} + +/** @public */ +export const templateEntityV1beta3Validator = + ajvCompiledJsonSchemaValidator(schema); diff --git a/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json b/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json new file mode 100644 index 0000000000..236944c9a1 --- /dev/null +++ b/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json @@ -0,0 +1,182 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema", + "$id": "TemplateV1beta3", + "description": "A Template describes a scaffolding task for use with the Scaffolder. It describes the required parameters as well as a series of steps that will be taken to execute the scaffolding task.", + "examples": [ + { + "apiVersion": "backstage.io/v1beta3", + "kind": "Template", + "metadata": { + "name": "react-ssr-template", + "title": "React SSR Template", + "description": "Next.js application skeleton for creating isomorphic web applications.", + "tags": ["recommended", "react"] + }, + "spec": { + "owner": "artist-relations-team", + "type": "website", + "parameters": { + "required": ["name", "description"], + "properties": { + "name": { + "title": "Name", + "type": "string", + "description": "Unique name of the component" + }, + "description": { + "title": "Description", + "type": "string", + "description": "Description of the component" + } + } + }, + "steps": [ + { + "id": "fetch", + "name": "Fetch", + "action": "fetch:plain", + "parameters": { + "url": "./template" + } + }, + { + "id": "publish", + "name": "Publish to GitHub", + "action": "publish:github", + "parameters": { + "repoUrl": "{{ parameters.repoUrl }}" + }, + "if": "{{ parameters.repoUrl }}" + } + ], + "output": { + "catalogInfoUrl": "{{ steps.publish.output.catalogInfoUrl }}" + } + } + } + ], + "allOf": [ + { + "$ref": "Entity" + }, + { + "type": "object", + "required": ["spec"], + "properties": { + "apiVersion": { + "enum": ["backstage.io/v1beta3"] + }, + "kind": { + "enum": ["Template"] + }, + "spec": { + "type": "object", + "required": ["type", "steps"], + "properties": { + "type": { + "type": "string", + "description": "The type of component created by the template. The software catalog accepts any type value, but an organization should take great care to establish a proper taxonomy for these. Tools including Backstage itself may read this field and behave differently depending on its value. For example, a website type component may present tooling in the Backstage interface that is specific to just websites.", + "examples": ["service", "website", "library"], + "minLength": 1 + }, + "parameters": { + "oneOf": [ + { + "type": "object", + "description": "The JSONSchema describing the inputs for the template." + }, + { + "type": "array", + "description": "A list of separate forms to collect parameters.", + "items": { + "type": "object", + "description": "The JSONSchema describing the inputs for the template." + } + } + ] + }, + "steps": { + "type": "array", + "description": "A list of steps to execute.", + "items": { + "type": "object", + "description": "A description of the step to execute.", + "required": ["action"], + "properties": { + "id": { + "type": "string", + "description": "The ID of the step, which can be used to refer to its outputs." + }, + "name": { + "type": "string", + "description": "The name of the step, which will be displayed in the UI during the scaffolding process." + }, + "action": { + "type": "string", + "description": "The name of the action to execute." + }, + "input": { + "type": "object", + "description": "A templated object describing the inputs to the action." + }, + "if": { + "type": ["string", "boolean"], + "description": "A templated condition that skips the step when evaluated to false. If the condition is true or not defined, the step is executed. The condition is true, if the input is not `false`, `undefined`, `null`, `\"\"`, `0`, or `[]`." + } + } + } + }, + "output": { + "type": "object", + "description": "A templated object describing the outputs of the scaffolding task.", + "properties": { + "links": { + "type": "array", + "description": "A list of external hyperlinks, typically pointing to resources created or updated by the template", + "items": { + "type": "object", + "required": [], + "properties": { + "url": { + "type": "string", + "description": "A url in a standard uri format.", + "examples": ["https://github.com/my-org/my-new-repo"], + "minLength": 1 + }, + "entityRef": { + "type": "string", + "description": "An entity reference to an entity in the catalog.", + "examples": ["Component:default/my-app"], + "minLength": 1 + }, + "title": { + "type": "string", + "description": "A user friendly display name for the link.", + "examples": ["View new repo"], + "minLength": 1 + }, + "icon": { + "type": "string", + "description": "A key representing a visual icon to be displayed in the UI.", + "examples": ["dashboard"], + "minLength": 1 + } + } + } + } + }, + "additionalProperties": { + "type": "string" + } + }, + "owner": { + "type": "string", + "description": "The user (or group) owner of the template", + "minLength": 1 + } + } + } + } + } + ] +} From ccea0b8271b3cd054297383b15b11e85ddc4bfcb Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 22 Sep 2021 18:08:21 +0200 Subject: [PATCH 02/29] feat(catalog-backend): add the new kind to the validation workflow Signed-off-by: blam --- packages/catalog-model/src/kinds/index.ts | 2 ++ .../processors/BuiltinKindsEntityProcessor.ts | 7 ++++- .../fixtures/test-v1beta3/template.yaml | 30 +++++++++++++++++++ .../scaffolder/actions/builtin/debug/log.ts | 5 ++++ 4 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml diff --git a/packages/catalog-model/src/kinds/index.ts b/packages/catalog-model/src/kinds/index.ts index be9f7a0d6a..edac9c505a 100644 --- a/packages/catalog-model/src/kinds/index.ts +++ b/packages/catalog-model/src/kinds/index.ts @@ -52,6 +52,8 @@ export type { } from './SystemEntityV1alpha1'; export { templateEntityV1beta2Validator } from './TemplateEntityV1beta2'; export type { TemplateEntityV1beta2 } from './TemplateEntityV1beta2'; +export { templateEntityV1beta3Validator } from './TemplateEntityV1beta3'; +export type { TemplateEntityV1beta3 } from './TemplateEntityV1beta3'; export type { KindValidator } from './types'; export { userEntityV1alpha1Validator } from './UserEntityV1alpha1'; export type { diff --git a/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts index bcabc00f32..18a8ad2a27 100644 --- a/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts @@ -48,6 +48,8 @@ import { systemEntityV1alpha1Validator, TemplateEntityV1beta2, templateEntityV1beta2Validator, + TemplateEntityV1beta3, + templateEntityV1beta3Validator, UserEntity, userEntityV1alpha1Validator, } from '@backstage/catalog-model'; @@ -61,6 +63,9 @@ export class BuiltinKindsEntityProcessor implements CatalogProcessor { resourceEntityV1alpha1Validator, groupEntityV1alpha1Validator, locationEntityV1alpha1Validator, + templateEntityV1beta3Validator, + + // TODO: remove once beta3 is stable templateEntityV1beta2Validator, userEntityV1alpha1Validator, systemEntityV1alpha1Validator, @@ -134,7 +139,7 @@ export class BuiltinKindsEntityProcessor implements CatalogProcessor { * Emit relations for the Template kind */ if (entity.kind === 'Template') { - const template = entity as TemplateEntityV1beta2; + const template = entity as TemplateEntityV1beta2 | TemplateEntityV1beta3; doEmit( template.spec.owner, { defaultKind: 'Group', defaultNamespace: selfRef.namespace }, diff --git a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml new file mode 100644 index 0000000000..b885357ff9 --- /dev/null +++ b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml @@ -0,0 +1,30 @@ +apiVersion: backstage.io/v1beta3 +kind: Template +metadata: + name: test-v1beta3 + title: Test v1beta3 + description: Test V1 Beta 3 Demo Templates +spec: + type: website + parameters: + - name: Enter some stuff + description: Enter some stuff + properties: + inputString: + type: string + inputObject: + type: object + properties: + first: + type: string + second: + type: number + steps: + - id: debug + name: Debug + action: debug:log + input: + message: ${{ parameters.inputString }} + extra: ${{ parameters.inputObject }} + + diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts index 557190dbbc..9bf9b7fb90 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts @@ -39,10 +39,15 @@ export function createDebugLogAction() { title: 'List all files in the workspace, if true.', type: 'boolean', }, + extra: { + title: 'Extra info', + }, }, }, }, async handler(ctx) { + ctx.logger.info(JSON.stringify(ctx.input, null, 2)); + if (ctx.input?.message) { ctx.logStream.write(ctx.input.message); } From e3dbdf66797fb332897cd93950e51a610782d84f Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 23 Sep 2021 15:20:43 +0200 Subject: [PATCH 03/29] feat(TaskWorker): split the parsing of the templates with the new syntax Signed-off-by: blam --- .../src/scaffolder/tasks/TaskWorker.ts | 9 ++++++++ .../scaffolder-backend/src/service/router.ts | 22 +++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 819df56109..7577a62d1f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -92,6 +92,15 @@ export class TaskWorker { }; } = { parameters: task.spec.values, steps: {} }; + const { output } = + task.spec.apiVersion === 'backstage.io/v1beta3' + ? await TemplateWorkflowRunner.execute(task) + : await LegacyWorkflowRunner.execute(task); + if (task.spec.apiVersion === 'backstage.io/v1beta3') { + const { output } = await TemplateWorkflowRunnger.execute(task); + } else { + } + for (const step of task.spec.steps) { const metadata = { stepId: step.id }; try { diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index f2a9c5b708..077d605083 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -34,7 +34,11 @@ import { } from '@backstage/backend-common'; import { InputError, NotFoundError } from '@backstage/errors'; import { CatalogApi } from '@backstage/catalog-client'; -import { TemplateEntityV1beta2, Entity } from '@backstage/catalog-model'; +import { + TemplateEntityV1beta2, + Entity, + TemplateEntityV1beta3, +} from '@backstage/catalog-model'; import { ScmIntegrations } from '@backstage/integration'; import { TemplateAction } from '../scaffolder/actions'; import { createBuiltinActions } from '../scaffolder/actions/builtin/createBuiltinActions'; @@ -50,10 +54,13 @@ export interface RouterOptions { containerRunner: ContainerRunner; } -function isBeta2Template( - entity: TemplateEntityV1beta2, -): entity is TemplateEntityV1beta2 { - return entity.apiVersion === 'backstage.io/v1beta2'; +function isSupportedTemplate( + entity: TemplateEntityV1beta2 | TemplateEntityV1beta3, +) { + return ( + entity.apiVersion === 'backstage.io/v1beta2' || + entity.apiVersion === 'backstage.io/v1beta3' + ); } export async function createRouter( @@ -128,7 +135,7 @@ export async function createRouter( const template = await entityClient.findTemplate(name, { token: getBearerToken(req.headers.authorization), }); - if (isBeta2Template(template)) { + if (isSupportedTemplate(template)) { const parameters = [template.spec.parameters ?? []].flat(); res.json({ title: template.metadata.title ?? template.metadata.name, @@ -166,7 +173,7 @@ export async function createRouter( let taskSpec; - if (isBeta2Template(template)) { + if (isSupportedTemplate(template)) { for (const parameters of [template.spec.parameters ?? []].flat()) { const result = validate(values, parameters); @@ -179,6 +186,7 @@ export async function createRouter( const baseUrl = getEntityBaseUrl(template); taskSpec = { + apiVersion: template.apiVersion, baseUrl, values, steps: template.spec.steps.map((step, index) => ({ From edb04593ff85eb77c335809ff993a813728321e8 Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 24 Sep 2021 14:01:23 +0200 Subject: [PATCH 04/29] feat(TaskWorker): move out the logic into the `WorkflowRunner` implementations Signed-off-by: blam --- .../src/kinds/TemplateEntityV1beta3.test.ts | 6 +- .../schema/kinds/Template.v1beta3.schema.json | 14 +- .../scaffolder/tasks/DefaultWorkflowRunner.ts | 31 ++ .../scaffolder/tasks/LegacyWorkflowRunner.ts | 297 ++++++++++++++++++ .../src/scaffolder/tasks/TaskWorker.ts | 268 +--------------- .../src/scaffolder/tasks/types.ts | 6 + 6 files changed, 355 insertions(+), 267 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts diff --git a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts index 092dfd7c7b..cc275435e8 100644 --- a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts +++ b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts @@ -30,6 +30,7 @@ describe('templateEntityV1beta3Validator', () => { name: 'test', }, spec: { + type: 'website', parameters: { required: ['storePath', 'owner'], properties: { @@ -38,11 +39,6 @@ describe('templateEntityV1beta3Validator', () => { title: 'Owner', description: 'Who is going to own this component', }, - storePath: { - type: 'string', - title: 'Store path', - description: 'GitHub store path in org/repo format', - }, }, }, steps: [ diff --git a/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json b/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json index 236944c9a1..e87a436d34 100644 --- a/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json +++ b/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json @@ -14,9 +14,8 @@ }, "spec": { "owner": "artist-relations-team", - "type": "website", "parameters": { - "required": ["name", "description"], + "required": ["name", "description", "repoUrl"], "properties": { "name": { "title": "Name", @@ -27,6 +26,11 @@ "title": "Description", "type": "string", "description": "Description of the component" + }, + "repoUrl": { + "title": "Pick a repository", + "type": "string", + "ui:field": "RepoUrlPicker" } } }, @@ -44,13 +48,13 @@ "name": "Publish to GitHub", "action": "publish:github", "parameters": { - "repoUrl": "{{ parameters.repoUrl }}" + "repoUrl": "${{ parameters.repoUrl }}" }, - "if": "{{ parameters.repoUrl }}" + "if": "${{ parameters.repoUrl }}" } ], "output": { - "catalogInfoUrl": "{{ steps.publish.output.catalogInfoUrl }}" + "catalogInfoUrl": "${{ steps.publish.output.catalogInfoUrl }}" } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts new file mode 100644 index 0000000000..813fa1b363 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { ScmIntegrations } from '@backstage/integration'; +import { TemplateActionRegistry } from '..'; +import { Task, WorkflowResponse, WorkflowRunner } from './types'; + +type Options = { + workingDirectory: string; + actionRegistry: TemplateActionRegistry; + integrations: ScmIntegrations; +}; + +export class DefaultWorkflowRunner implements WorkflowRunner { + constructor(private readonly options: Options) {} + async execute(task: Task): Promise { + throw new Error('Method not implemented.'); + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts new file mode 100644 index 0000000000..a5bf740c07 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts @@ -0,0 +1,297 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Task, WorkflowRunner, WorkflowResponse } from './types'; +import * as Handlebars from 'handlebars'; +import { TemplateActionRegistry } from '..'; +import { ScmIntegrations } from '@backstage/integration'; +import { parseRepoUrl } from '../actions/builtin/publish/util'; +import { isTruthy } from './helper'; +import { PassThrough } from 'stream'; +import * as winston from 'winston'; +import { Logger } from 'winston'; +import path from 'path'; +import fs from 'fs-extra'; +import { validate as validateJsonSchema } from 'jsonschema'; +import { JsonObject, JsonValue } from '@backstage/config'; +import { InputError } from '@backstage/errors'; + +type Options = { + workingDirectory: string; + actionRegistry: TemplateActionRegistry; + integrations: ScmIntegrations; + logger: Logger; +}; + +/** + * This is the legacy workflow runner, which supports handlebars. This entire implementation will be replaced + * with the default workflow runner interface in the future so this entire thing can go bye bye. + */ +export class LegacyWorkflowRunner implements WorkflowRunner { + 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.options.integrations)); + }); + + this.handlebars.registerHelper('projectSlug', repoUrl => { + const { owner, repo } = parseRepoUrl(repoUrl, this.options.integrations); + return `${owner}/${repo}`; + }); + + this.handlebars.registerHelper('json', obj => JSON.stringify(obj)); + + this.handlebars.registerHelper('not', value => !isTruthy(value)); + + this.handlebars.registerHelper('eq', (a, b) => a === b); + } + + async execute(task: Task): Promise { + const { actionRegistry } = this.options; + + const workspacePath = path.join( + this.options.workingDirectory, + await task.getWorkspaceName(), + ); + try { + await fs.ensureDir(workspacePath); + await task.emitLog( + `Starting up task with ${task.spec.steps.length} steps`, + ); + + const templateCtx: { + parameters: JsonObject; + steps: { + [stepName: string]: { output: { [outputName: string]: JsonValue } }; + }; + } = { parameters: task.spec.values, steps: {} }; + + for (const step of task.spec.steps) { + const metadata = { stepId: step.id }; + try { + const taskLogger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: {}, + }); + + const stream = new PassThrough(); + stream.on('data', async data => { + const message = data.toString().trim(); + if (message?.length > 1) { + await task.emitLog(message, metadata); + } + }); + + taskLogger.add(new winston.transports.Stream({ stream })); + + if (step.if !== undefined) { + // Support passing values like false to disable steps + let skip = !step.if; + + // Evaluate strings as handlebar templates + if (typeof step.if === 'string') { + const condition = JSON.parse( + JSON.stringify(step.if), + (_key, value) => { + if (typeof value === 'string') { + const templated = this.handlebars.compile(value, { + noEscape: true, + data: false, + preventIndent: true, + })(templateCtx); + + // If it's just an empty string, treat it as undefined + if (templated === '') { + return undefined; + } + + try { + return JSON.parse(templated); + } catch { + return templated; + } + } + + return value; + }, + ); + + skip = !isTruthy(condition); + } + + if (skip) { + await task.emitLog(`Skipped step ${step.name}`, { + ...metadata, + status: 'skipped', + }); + continue; + } + } + + await task.emitLog(`Beginning step ${step.name}`, { + ...metadata, + status: 'processing', + }); + + const action = actionRegistry.get(step.action); + if (!action) { + throw new Error(`Action '${step.action}' does not exist`); + } + + const input = + step.input && + JSON.parse(JSON.stringify(step.input), (_key, value) => { + if (typeof value === 'string') { + const templated = this.handlebars.compile(value, { + noEscape: 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('"')) || + (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 + return JSON.parse(templated); + } catch { + return templated; + } + } + return templated; + } + + return value; + }); + + if (action.schema?.input) { + const validateResult = validateJsonSchema( + input, + action.schema.input, + ); + if (!validateResult.valid) { + const errors = validateResult.errors.join(', '); + throw new InputError( + `Invalid input passed to action ${action.id}, ${errors}`, + ); + } + } + + const stepOutputs: { [name: string]: JsonValue } = {}; + + // Keep track of all tmp dirs that are created by the action so we can remove them after + const tmpDirs = new Array(); + + this.options.logger.debug(`Running ${action.id} with input`, { + input: JSON.stringify(input, null, 2), + }); + + await action.handler({ + baseUrl: task.spec.baseUrl, + logger: taskLogger, + logStream: stream, + input, + token: task.secrets?.token, + workspacePath, + async createTemporaryDirectory() { + const tmpDir = await fs.mkdtemp( + `${workspacePath}_step-${step.id}-`, + ); + tmpDirs.push(tmpDir); + return tmpDir; + }, + output(name: string, value: JsonValue) { + stepOutputs[name] = value; + }, + }); + + // Remove all temporary directories that were created when executing the action + for (const tmpDir of tmpDirs) { + await fs.remove(tmpDir); + } + + templateCtx.steps[step.id] = { output: stepOutputs }; + + await task.emitLog(`Finished step ${step.name}`, { + ...metadata, + status: 'completed', + }); + } catch (error) { + await task.emitLog(String(error.stack), { + ...metadata, + status: 'failed', + }); + throw error; + } + } + + const output = JSON.parse( + JSON.stringify(task.spec.output), + (_key, value) => { + if (typeof value === 'string') { + const templated = this.handlebars.compile(value, { + noEscape: true, + data: false, + preventIndent: true, + })(templateCtx); + + // If it's just an empty string, treat it as undefined + if (templated === '') { + return undefined; + } + + // 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('"')) || + (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 + return JSON.parse(templated); + } catch { + return templated; + } + } + return templated; + } + return value; + }, + ); + + return { output }; + } finally { + if (workspacePath) { + await fs.remove(workspacePath); + } + } + } +} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 7577a62d1f..a1af3df2b0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -17,17 +17,17 @@ import { JsonObject, JsonValue } from '@backstage/config'; import { InputError } from '@backstage/errors'; import fs from 'fs-extra'; -import * as Handlebars from 'handlebars'; -import { validate as validateJsonSchema } from 'jsonschema'; + import path from 'path'; -import { PassThrough } from 'stream'; -import * as winston from 'winston'; + import { Logger } from 'winston'; import { parseRepoUrl } from '../actions/builtin/publish/util'; import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; import { isTruthy } from './helper'; -import { Task, TaskBroker } from './types'; +import { Task, TaskBroker, WorkflowRunner } from './types'; import { ScmIntegrations } from '@backstage/integration'; +import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; +import { DefaultWorkflowRunner } from './DefaultWorkflowRunner'; type Options = { logger: Logger; @@ -38,28 +38,12 @@ type Options = { }; export class TaskWorker { - private readonly handlebars: typeof Handlebars; + private readonly legacyWorkflowRunner: LegacyWorkflowRunner; + private readonly workflowRunner: WorkflowRunner; 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, options.integrations)); - }); - - this.handlebars.registerHelper('projectSlug', repoUrl => { - const { owner, repo } = parseRepoUrl(repoUrl, options.integrations); - return `${owner}/${repo}`; - }); - - this.handlebars.registerHelper('json', obj => JSON.stringify(obj)); - - this.handlebars.registerHelper('not', value => !isTruthy(value)); - - this.handlebars.registerHelper('eq', (a, b) => a === b); + this.legacyWorkflowRunner = new LegacyWorkflowRunner(options); + this.workflowRunner = new DefaultWorkflowRunner(options); } start() { @@ -72,247 +56,17 @@ export class TaskWorker { } async runOneTask(task: Task) { - let workspacePath: string | undefined = undefined; try { - const { actionRegistry } = this.options; - - workspacePath = path.join( - this.options.workingDirectory, - await task.getWorkspaceName(), - ); - await fs.ensureDir(workspacePath); - await task.emitLog( - `Starting up task with ${task.spec.steps.length} steps`, - ); - - const templateCtx: { - parameters: JsonObject; - steps: { - [stepName: string]: { output: { [outputName: string]: JsonValue } }; - }; - } = { parameters: task.spec.values, steps: {} }; - const { output } = task.spec.apiVersion === 'backstage.io/v1beta3' - ? await TemplateWorkflowRunner.execute(task) - : await LegacyWorkflowRunner.execute(task); - if (task.spec.apiVersion === 'backstage.io/v1beta3') { - const { output } = await TemplateWorkflowRunnger.execute(task); - } else { - } - - for (const step of task.spec.steps) { - const metadata = { stepId: step.id }; - try { - const taskLogger = winston.createLogger({ - level: process.env.LOG_LEVEL || 'info', - format: winston.format.combine( - winston.format.colorize(), - winston.format.timestamp(), - winston.format.simple(), - ), - defaultMeta: {}, - }); - - const stream = new PassThrough(); - stream.on('data', async data => { - const message = data.toString().trim(); - if (message?.length > 1) { - await task.emitLog(message, metadata); - } - }); - - taskLogger.add(new winston.transports.Stream({ stream })); - - if (step.if !== undefined) { - // Support passing values like false to disable steps - let skip = !step.if; - - // Evaluate strings as handlebar templates - if (typeof step.if === 'string') { - const condition = JSON.parse( - JSON.stringify(step.if), - (_key, value) => { - if (typeof value === 'string') { - const templated = this.handlebars.compile(value, { - noEscape: true, - data: false, - preventIndent: true, - })(templateCtx); - - // If it's just an empty string, treat it as undefined - if (templated === '') { - return undefined; - } - - try { - return JSON.parse(templated); - } catch { - return templated; - } - } - - return value; - }, - ); - - skip = !isTruthy(condition); - } - - if (skip) { - await task.emitLog(`Skipped step ${step.name}`, { - ...metadata, - status: 'skipped', - }); - continue; - } - } - - await task.emitLog(`Beginning step ${step.name}`, { - ...metadata, - status: 'processing', - }); - - const action = actionRegistry.get(step.action); - if (!action) { - throw new Error(`Action '${step.action}' does not exist`); - } - - const input = - step.input && - JSON.parse(JSON.stringify(step.input), (_key, value) => { - if (typeof value === 'string') { - const templated = this.handlebars.compile(value, { - noEscape: 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('"')) || - (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 - return JSON.parse(templated); - } catch { - return templated; - } - } - return templated; - } - - return value; - }); - - if (action.schema?.input) { - const validateResult = validateJsonSchema( - input, - action.schema.input, - ); - if (!validateResult.valid) { - const errors = validateResult.errors.join(', '); - throw new InputError( - `Invalid input passed to action ${action.id}, ${errors}`, - ); - } - } - - const stepOutputs: { [name: string]: JsonValue } = {}; - - // Keep track of all tmp dirs that are created by the action so we can remove them after - const tmpDirs = new Array(); - - this.options.logger.debug(`Running ${action.id} with input`, { - input: JSON.stringify(input, null, 2), - }); - - await action.handler({ - baseUrl: task.spec.baseUrl, - logger: taskLogger, - logStream: stream, - input, - token: task.secrets?.token, - workspacePath, - async createTemporaryDirectory() { - const tmpDir = await fs.mkdtemp( - `${workspacePath}_step-${step.id}-`, - ); - tmpDirs.push(tmpDir); - return tmpDir; - }, - output(name: string, value: JsonValue) { - stepOutputs[name] = value; - }, - }); - - // Remove all temporary directories that were created when executing the action - for (const tmpDir of tmpDirs) { - await fs.remove(tmpDir); - } - - templateCtx.steps[step.id] = { output: stepOutputs }; - - await task.emitLog(`Finished step ${step.name}`, { - ...metadata, - status: 'completed', - }); - } catch (error) { - await task.emitLog(String(error.stack), { - ...metadata, - status: 'failed', - }); - throw error; - } - } - - const output = JSON.parse( - JSON.stringify(task.spec.output), - (_key, value) => { - if (typeof value === 'string') { - const templated = this.handlebars.compile(value, { - noEscape: true, - data: false, - preventIndent: true, - })(templateCtx); - - // If it's just an empty string, treat it as undefined - if (templated === '') { - return undefined; - } - - // 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('"')) || - (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 - return JSON.parse(templated); - } catch { - return templated; - } - } - return templated; - } - return value; - }, - ); + ? await this.workflowRunner.execute(task) + : await this.legacyWorkflowRunner.execute(task); await task.complete('completed', { output }); } catch (error) { await task.complete('failed', { error: { name: error.name, message: error.message }, }); - } finally { - if (workspacePath) { - await fs.remove(workspacePath); - } } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 3b1805f117..5ff40b9459 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -44,6 +44,7 @@ export type DbTaskEventRow = { }; export type TaskSpec = { + apiVersion: 'backstage.io/v1beta2' | 'backstage.io/v1beta3'; baseUrl?: string; values: JsonObject; steps: Array<{ @@ -122,3 +123,8 @@ export interface TaskStore { after, }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }>; } + +export type WorkflowResponse = { output: { [name: string]: JsonValue } }; +export interface WorkflowRunner { + execute(task: Task): Promise; +} From 4e005b13cfc09a0ca5c227367794ab1664beb500 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 27 Sep 2021 14:26:23 +0200 Subject: [PATCH 05/29] feat: moving all the legacy tests over to the new LegacyWorkflowRunner tests Signed-off-by: blam --- .../tasks/LegacyWorkflowRunner.test.ts | 400 ++++++++++++++++++ 1 file changed, 400 insertions(+) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts new file mode 100644 index 0000000000..a82f6b25fa --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts @@ -0,0 +1,400 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { createTemplateAction, TemplateActionRegistry } from '../actions'; +import { ScmIntegrations } from '@backstage/integration'; +import { ConfigReader } from '@backstage/config'; +import { getVoidLogger } from '@backstage/backend-common'; +import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; +import os from 'os'; +import { Task, TaskSpec } from './types'; +import { RepoSpec } from '../actions/builtin/publish/util'; + +describe('LegacyWorkflowRunner', () => { + let runner: LegacyWorkflowRunner; + const workingDirectory = os.tmpdir(); + const logger = getVoidLogger(); + let actionRegistry = new TemplateActionRegistry(); + + const integrations = ScmIntegrations.fromConfig( + new ConfigReader({ + integrations: { + github: [{ host: 'github.com', token: 'token' }], + }, + }), + ); + + const createMockTaskWithSpec = (spec: TaskSpec): Task => ({ + spec, + complete: async () => {}, + done: false, + emitLog: async () => {}, + getWorkspaceName: () => Promise.resolve('test-workspace'), + }); + + beforeEach(() => { + actionRegistry = new TemplateActionRegistry(); + actionRegistry.register({ + id: 'test-action', + handler: async ctx => { + ctx.output('testOutput', 'mockOutputData'); + ctx.output('badOutput', false); + }, + }); + + runner = new LegacyWorkflowRunner({ + actionRegistry, + integrations, + workingDirectory, + logger, + }); + }); + + it('should fail when the action does not exist', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], + output: { + result: '{{ steps.test.output.testOutput }}', + }, + values: {}, + }); + + await expect(() => runner.execute(task)).rejects.toThrow( + /Template action with ID 'not-found-action' is not registered/, + ); + }); + + describe('templating', () => { + it('should template the output', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [{ id: 'test', name: 'test', action: 'test-action' }], + output: { + result: '{{ steps.test.output.testOutput }}', + }, + values: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBe('mockOutputData'); + }); + + it('should template the input', async () => { + const inputAction = createTemplateAction<{ + name: string; + }>({ + id: 'test-input', + schema: { + input: { + type: 'object', + required: ['name'], + properties: { + name: { + title: 'name', + description: 'Enter name', + type: 'string', + }, + }, + }, + }, + async handler(ctx) { + if (ctx.input.name !== 'mockOutputData') { + throw new Error( + `expected name to be "mockOutputData" got ${ctx.input.name}`, + ); + } + }, + }); + actionRegistry.register(inputAction); + + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [ + { id: 'test', name: 'test', action: 'test-action' }, + { + id: 'test-input', + name: 'test-input', + action: 'test-input', + input: { + name: '{{ steps.test.output.testOutput }}', + }, + }, + ], + output: { + result: '{{ steps.test.output.testOutput }}', + }, + values: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBe('mockOutputData'); + }); + }); + + describe('conditionals', () => { + it('should execute steps conditionally', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [ + { id: 'test', name: 'test', action: 'test-action' }, + { + id: 'conditional', + name: 'conditional', + action: 'test-action', + if: '{{ steps.test.output.testOutput }}', + }, + ], + output: { + result: '{{ steps.conditional.output.testOutput }}', + }, + values: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBe('mockOutputData'); + }); + + it('should execute steps conditionally with eq helper', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [ + { id: 'test', name: 'test', action: 'test-action' }, + { + id: 'conditional', + name: 'conditional', + action: 'test-action', + if: '{{ eq steps.test.output.testOutput "mockOutputData" }}', + }, + ], + output: { + result: '{{ steps.conditional.output.testOutput }}', + }, + values: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBe('mockOutputData'); + }); + + it('should skip test conditionally', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [ + { id: 'test', name: 'test', action: 'test-action' }, + { + id: 'conditional', + name: 'conditional', + action: 'test-action', + if: '{{ steps.test.output.badOutput }}', + }, + ], + output: { + result: '{{ steps.conditional.output.testOutput }}', + }, + values: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBeUndefined(); + }); + }); + + describe('parsing', () => { + it('should parse strings as objects if possible', async () => { + const inputAction = createTemplateAction<{ + address: { line1: string }; + list: 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', + }, + 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}`, + ); + } + + 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 task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + steps: [ + { + id: 'test-input', + name: 'test-input', + action: 'test-input', + input: { + address: JSON.stringify({ line1: 'line 1' }), + list: JSON.stringify(['hey!']), + address2: '{"not valid"}', + }, + }, + ], + output: { + result: '{{ steps.test-input.output.address }}', + }, + values: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBe('line 1'); + }); + + it('should provide a parseRepoUrl helper', 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', + }, + workspace: { + type: 'string', + }, + project: { + type: 'string', + }, + }, + }, + }, + }, + }, + async handler(ctx) { + ctx.output('host', ctx.input.destination.host); + ctx.output('repo', ctx.input.destination.repo); + + if (ctx.input.destination.owner) { + 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 && + ctx.input.destination.owner !== 'owner' + ) { + throw new Error( + `expected repo to be "owner" got ${ctx.input.destination.owner}`, + ); + } + }, + }); + actionRegistry.register(inputAction); + + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta2', + 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 { output } = await runner.execute(task); + + expect(output.host).toBe('github.com'); + expect(output.repo).toBe('repo'); + expect(output.owner).toBe('owner'); + }); + }); +}); From 6ab1202fc8ec684b058b2f3cac548b4986460f37 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 27 Sep 2021 17:09:34 +0200 Subject: [PATCH 06/29] feat: rework the TaskWorker to just be concerned with the task delegation Signed-off-by: blam --- .../src/scaffolder/tasks/TaskWorker.test.ts | 459 +++--------------- .../src/scaffolder/tasks/TaskWorker.ts | 34 +- 2 files changed, 63 insertions(+), 430 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 2c274df7fc..66c435862a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -23,6 +23,8 @@ import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker } from './StorageTaskBroker'; import { TaskWorker } from './TaskWorker'; import { ScmIntegrations } from '@backstage/integration'; +import { WorkflowRunner } from './types'; +import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; async function createStore(): Promise { const manager = DatabaseManager.fromConfig( @@ -40,71 +42,91 @@ async function createStore(): Promise { describe('TaskWorker', () => { let storage: DatabaseTaskStore; - let actionRegistry = new TemplateActionRegistry(); + const workflowRunner: WorkflowRunner = { + execute: jest.fn(), + } as unknown as WorkflowRunner; - const integrations = ScmIntegrations.fromConfig( - new ConfigReader({ - integrations: { - github: [{ host: 'github.com', token: 'token' }], - }, - }), - ); + const legacyWorkflowRunner: LegacyWorkflowRunner = { + execute: jest.fn(), + } as unknown as LegacyWorkflowRunner; beforeAll(async () => { storage = await createStore(); }); beforeEach(() => { - actionRegistry = new TemplateActionRegistry(); - actionRegistry.register({ - id: 'test-action', - handler: async ctx => { - ctx.output('testOutput', 'winning'); - ctx.output('badOutput', false); - }, - }); + jest.resetAllMocks(); }); const logger = getVoidLogger(); - it('should fail when action does not exist', async () => { + it('should call the legacy workflow runner when the apiVersion is not beta3', async () => { const broker = new StorageTaskBroker(storage, logger); const taskWorker = new TaskWorker({ - logger, - workingDirectory: os.tmpdir(), - actionRegistry, taskBroker: broker, - integrations, + runners: { + legacyWorkflowRunner, + workflowRunner, + }, }); - const { taskId } = await broker.dispatch({ + + await broker.dispatch({ + apiVersion: 'backstage.io/v1beta2', steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], output: { result: '{{ steps.test.output.testOutput }}', }, 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?.error as JsonObject)?.message).toBe( - "Template action with ID 'not-found-action' is not registered.", - ); + expect(legacyWorkflowRunner.execute).toHaveBeenCalled(); }); - it('should template output', async () => { + it('should call the default workflow runner when the apiVersion is beta3', async () => { const broker = new StorageTaskBroker(storage, logger); const taskWorker = new TaskWorker({ - logger, - workingDirectory: os.tmpdir(), - actionRegistry, taskBroker: broker, - integrations, + runners: { + legacyWorkflowRunner, + workflowRunner, + }, + }); + + await broker.dispatch({ + apiVersion: 'backstage.io/v1beta3', + steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], + output: { + result: '{{ steps.test.output.testOutput }}', + }, + values: {}, + }); + + const task = await broker.claim(); + await taskWorker.runOneTask(task); + + expect(workflowRunner.execute).toHaveBeenCalled(); + }); + + it('should save the output to the task', async () => { + (workflowRunner.execute as jest.Mock).mockResolvedValue({ + output: { testOutput: 'testmockoutput' }, + }); + + const broker = new StorageTaskBroker(storage, logger); + const taskWorker = new TaskWorker({ + taskBroker: broker, + runners: { + legacyWorkflowRunner, + workflowRunner, + }, }); const { taskId } = await broker.dispatch({ - steps: [{ id: 'test', name: 'test', action: 'test-action' }], + apiVersion: 'backstage.io/v1beta3', + steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], output: { result: '{{ steps.test.output.testOutput }}', }, @@ -116,375 +138,6 @@ describe('TaskWorker', () => { const { events } = await storage.listEvents({ taskId }); const event = events.find(e => e.type === 'completion'); - expect((event?.body?.output as JsonObject).result).toBe('winning'); - }); - - it('should template input', async () => { - const inputAction = createTemplateAction<{ - name: string; - }>({ - id: 'test-input', - schema: { - input: { - type: 'object', - required: ['name'], - properties: { - name: { - title: 'name', - description: 'Enter name', - type: 'string', - }, - }, - }, - }, - async handler(ctx) { - if (ctx.input.name !== 'winning') { - throw new Error( - `expected name to be "winning" got ${ctx.input.name}`, - ); - } - }, - }); - actionRegistry.register(inputAction); - - const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ - logger, - workingDirectory: os.tmpdir(), - actionRegistry, - taskBroker: broker, - integrations, - }); - - const { taskId } = await broker.dispatch({ - steps: [ - { id: 'test', name: 'test', action: 'test-action' }, - { - id: 'test-input', - name: 'test-input', - action: 'test-input', - input: { - name: '{{ steps.test.output.testOutput }}', - }, - }, - ], - output: { - result: '{{ steps.test.output.testOutput }}', - }, - 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('winning'); - }); - - it('should execute steps conditionally', async () => { - const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ - logger, - workingDirectory: os.tmpdir(), - actionRegistry, - taskBroker: broker, - integrations, - }); - - const { taskId } = await broker.dispatch({ - steps: [ - { id: 'test', name: 'test', action: 'test-action' }, - { - id: 'conditional', - name: 'conditional', - action: 'test-action', - if: '{{ steps.test.output.testOutput }}', - }, - ], - output: { - result: '{{ steps.conditional.output.testOutput }}', - }, - 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('winning'); - }); - - it('should execute steps conditionally with eq helper', async () => { - const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ - logger, - workingDirectory: os.tmpdir(), - actionRegistry, - taskBroker: broker, - integrations, - }); - - const { taskId } = await broker.dispatch({ - steps: [ - { id: 'test', name: 'test', action: 'test-action' }, - { - id: 'conditional', - name: 'conditional', - action: 'test-action', - if: '{{ eq steps.test.output.testOutput "winning" }}', - }, - ], - output: { - result: '{{ steps.conditional.output.testOutput }}', - }, - 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('winning'); - }); - - it('should skip steps conditionally', async () => { - const broker = new StorageTaskBroker(storage, logger); - const taskWorker = new TaskWorker({ - logger, - workingDirectory: os.tmpdir(), - actionRegistry, - taskBroker: broker, - integrations, - }); - - const { taskId } = await broker.dispatch({ - steps: [ - { id: 'test', name: 'test', action: 'test-action' }, - { - id: 'conditional', - name: 'conditional', - action: 'test-action', - if: '{{ steps.test.output.badOutput }}', - }, - ], - output: { - result: '{{ steps.conditional.output.testOutput }}', - }, - 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).toBeUndefined(); - }); - - it('should parse strings as objects if possible', async () => { - const inputAction = createTemplateAction<{ - address: { line1: string }; - list: 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', - }, - 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}`, - ); - } - - 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, - integrations, - }); - - const { taskId } = await broker.dispatch({ - steps: [ - { - id: 'test-input', - name: 'test-input', - action: 'test-input', - input: { - address: JSON.stringify({ line1: 'line 1' }), - list: JSON.stringify(['hey!']), - 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'); - }); - - // 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', - }, - workspace: { - type: 'string', - }, - project: { - type: 'string', - }, - }, - }, - }, - }, - }, - async handler(ctx) { - ctx.output('host', ctx.input.destination.host); - ctx.output('repo', ctx.input.destination.repo); - - if (ctx.input.destination.owner) { - 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 && - 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, - integrations, - }); - - 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'); + expect(event?.body.output).toEqual({ testOutput: 'testmockoutput' }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index a1af3df2b0..5b69ecc6df 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -13,39 +13,19 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import { JsonObject, JsonValue } from '@backstage/config'; -import { InputError } from '@backstage/errors'; -import fs from 'fs-extra'; - -import path from 'path'; - -import { Logger } from 'winston'; -import { parseRepoUrl } from '../actions/builtin/publish/util'; -import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; -import { isTruthy } from './helper'; import { Task, TaskBroker, WorkflowRunner } from './types'; -import { ScmIntegrations } from '@backstage/integration'; import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; -import { DefaultWorkflowRunner } from './DefaultWorkflowRunner'; type Options = { - logger: Logger; taskBroker: TaskBroker; - workingDirectory: string; - actionRegistry: TemplateActionRegistry; - integrations: ScmIntegrations; + runners: { + legacyWorkflowRunner: LegacyWorkflowRunner; + workflowRunner: WorkflowRunner; + }; }; export class TaskWorker { - private readonly legacyWorkflowRunner: LegacyWorkflowRunner; - private readonly workflowRunner: WorkflowRunner; - - constructor(private readonly options: Options) { - this.legacyWorkflowRunner = new LegacyWorkflowRunner(options); - this.workflowRunner = new DefaultWorkflowRunner(options); - } - + constructor(private readonly options: Options) {} start() { (async () => { for (;;) { @@ -59,8 +39,8 @@ export class TaskWorker { try { const { output } = task.spec.apiVersion === 'backstage.io/v1beta3' - ? await this.workflowRunner.execute(task) - : await this.legacyWorkflowRunner.execute(task); + ? await this.options.runners.workflowRunner.execute(task) + : await this.options.runners.legacyWorkflowRunner.execute(task); await task.complete('completed', { output }); } catch (error) { From 34b1278929753161cfae704843016141c9523d02 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 02:56:37 +0200 Subject: [PATCH 07/29] chore: worked out a niceish way to template objects and stuff as first class citizens Signed-off-by: blam --- .../tasks/DefaultWorkflowRunner.test.ts | 193 ++++++++++++++++++ .../scaffolder/tasks/DefaultWorkflowRunner.ts | 151 +++++++++++++- .../scaffolder/tasks/LegacyWorkflowRunner.ts | 12 +- .../src/scaffolder/tasks/types.ts | 23 ++- 4 files changed, 372 insertions(+), 7 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts new file mode 100644 index 0000000000..e508d619c4 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -0,0 +1,193 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import os from 'os'; +import { getVoidLogger } from '@backstage/backend-common'; +import { DefaultWorkflowRunner } from './DefaultWorkflowRunner'; +import { TemplateActionRegistry } from '../actions'; +import { ScmIntegrations } from '@backstage/integration'; +import { ConfigReader } from '@backstage/config'; +import { Task, TaskSpec } from './types'; + +describe('DefaultWorkflowRunner', () => { + const workingDirectory = os.tmpdir(); + const logger = getVoidLogger(); + let actionRegistry = new TemplateActionRegistry(); + let runner: DefaultWorkflowRunner; + let fakeActionHandler: jest.Mock; + + const integrations = ScmIntegrations.fromConfig( + new ConfigReader({ + integrations: { + github: [{ host: 'github.com', token: 'token' }], + }, + }), + ); + + const createMockTaskWithSpec = (spec: TaskSpec): Task => ({ + spec, + complete: async () => {}, + done: false, + emitLog: async () => {}, + getWorkspaceName: () => Promise.resolve('test-workspace'), + }); + + beforeEach(() => { + jest.resetAllMocks(); + actionRegistry = new TemplateActionRegistry(); + fakeActionHandler = jest.fn(); + + actionRegistry.register({ + id: 'jest-mock-action', + description: 'Mock action for testing', + handler: fakeActionHandler, + }); + + runner = new DefaultWorkflowRunner({ + actionRegistry, + integrations, + workingDirectory, + logger, + }); + }); + + it('should throw an error if the action does not exist', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [{ id: 'test', name: 'name', action: 'does-not-exist' }], + }); + + await expect(runner.execute(task)).rejects.toThrowError( + "Template action with ID 'does-not-exist' is not registered.", + ); + }); + + describe('validation', () => {}); + describe('running', () => {}); + + describe('templating', () => { + it('should template the input to an action', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: { + foo: '${{parameters.input | lower }}', + }, + }, + ], + output: {}, + parameters: { + input: 'BACKSTAGE', + }, + }); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledWith( + expect.objectContaining({ input: { foo: 'backstage' } }), + ); + }); + + it('should template complex values into the action', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: { + foo: '${{parameters.complex}}', + }, + }, + ], + output: {}, + parameters: { + complex: { bar: 'BACKSTAGE' }, + }, + }); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledWith( + expect.objectContaining({ input: { foo: { bar: 'BACKSTAGE' } } }), + ); + }); + + it('supports really complex structures', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: { + foo: '${{parameters.complex.baz.something}}', + }, + }, + ], + output: {}, + parameters: { + complex: { + bar: 'BACKSTAGE', + baz: { something: 'nested', here: 'yas' }, + }, + }, + }); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledWith( + expect.objectContaining({ input: { foo: 'nested' } }), + ); + }); + + it('supports numbers as first class too', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: { + foo: '${{parameters.complex.baz.number}}', + }, + }, + ], + output: {}, + parameters: { + complex: { + bar: 'BACKSTAGE', + baz: { number: 1 }, + }, + }, + }); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledWith( + expect.objectContaining({ input: { foo: 1 } }), + ); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 813fa1b363..e84d7a71e6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -15,17 +15,162 @@ */ import { ScmIntegrations } from '@backstage/integration'; import { TemplateActionRegistry } from '..'; -import { Task, WorkflowResponse, WorkflowRunner } from './types'; +import { + Task, + TaskSpec, + TaskSpecV1beta3, + TaskStep, + WorkflowResponse, + WorkflowRunner, +} from './types'; +import * as winston from 'winston'; +import nunjucks from 'nunjucks'; +import fs from 'fs-extra'; +import path from 'path'; +import { JsonObject, JsonValue } from '@backstage/config'; +import { InputError } from '@backstage/errors'; +import { PassThrough } from 'stream'; type Options = { workingDirectory: string; actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; + logger: winston.Logger; +}; + +type TemplateContext = { + parameters: JsonObject; + steps: { + [stepName: string]: { output: { [outputName: string]: JsonValue } }; + }; +}; + +const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => { + return taskSpec.apiVersion === 'backstage.io/v1beta3'; +}; + +const createStepLogger = ({ task, step }: { task: Task; step: TaskStep }) => { + const metadata = { stepId: step.id }; + const taskLogger = winston.createLogger({ + level: process.env.LOG_LEVEL || 'info', + format: winston.format.combine( + winston.format.colorize(), + winston.format.timestamp(), + winston.format.simple(), + ), + defaultMeta: {}, + }); + + const streamLogger = new PassThrough(); + streamLogger.on('data', async data => { + const message = data.toString().trim(); + if (message?.length > 1) { + await task.emitLog(message, metadata); + } + }); + + taskLogger.add(new winston.transports.Stream({ stream: streamLogger })); + + return { taskLogger, streamLogger }; }; export class DefaultWorkflowRunner implements WorkflowRunner { - constructor(private readonly options: Options) {} + private readonly nunjucks: nunjucks.Environment; + + constructor(private readonly options: Options) { + this.nunjucks = nunjucks.configure({ + autoescape: false, + tags: { + variableStart: '${{', + variableEnd: '}}', + }, + }); + } + async execute(task: Task): Promise { - throw new Error('Method not implemented.'); + if (!isValidTaskSpec(task.spec)) { + throw new InputError( + 'Wrong template version executed with the workflow engine', + ); + } + const workspacePath = path.join( + this.options.workingDirectory, + await task.getWorkspaceName(), + ); + try { + await fs.ensureDir(workspacePath); + await task.emitLog( + `Starting up task with ${task.spec.steps.length} steps`, + ); + + /** + * This is a little bit of a hack / magic so that when we use nunjucks and we try to + * pass through an object from the `parameters` section of the task spec, it will + * actually work as the toString method is called from the nunjucks template. + */ + const parsedParams = JSON.parse( + JSON.stringify(task.spec.parameters), + (key: string, value: JsonObject) => { + if (typeof value === 'object' && key) { + value.toString = () => JSON.stringify(value); + } + + return value; + }, + ); + + const context: TemplateContext = { + parameters: parsedParams, + steps: {}, + }; + + for (const step of task.spec.steps) { + const action = this.options.actionRegistry.get(step.action); + const { taskLogger, streamLogger } = createStepLogger({ task, step }); + + const input = + step.input && + JSON.parse(JSON.stringify(step.input), (_key, value) => { + try { + if (typeof value === 'string') { + const templated = this.nunjucks.renderString(value, context); + try { + return JSON.parse(templated); + } catch { + return templated; + } + } + } catch { + return value; + } + return value; + }); + + const tmpDirs = new Array(); + const stepOutputs: { [outputName: string]: JsonValue } = {}; + + await action.handler({ + baseUrl: task.spec.baseUrl, + input, + logger: taskLogger, + logStream: streamLogger, + workspacePath, + createTemporaryDirectory: async () => { + const tmpDir = await fs.mkdtemp( + `${workspacePath}_step-${step.id}-`, + ); + tmpDirs.push(tmpDir); + return tmpDir; + }, + output(name: string, value: JsonValue) { + stepOutputs[name] = value; + }, + }); + } + } finally { + if (workspacePath) { + await fs.remove(workspacePath); + } + } } } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts index a5bf740c07..a3c98f6cd4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts @@ -13,7 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Task, WorkflowRunner, WorkflowResponse } from './types'; +import { + Task, + WorkflowRunner, + WorkflowResponse, + TaskSpecV1beta2, +} from './types'; import * as Handlebars from 'handlebars'; import { TemplateActionRegistry } from '..'; import { ScmIntegrations } from '@backstage/integration'; @@ -35,6 +40,8 @@ type Options = { logger: Logger; }; +const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta2 => + taskSpec.apiVersion === 'backstage.io/v1beta2'; /** * This is the legacy workflow runner, which supports handlebars. This entire implementation will be replaced * with the default workflow runner interface in the future so this entire thing can go bye bye. @@ -65,6 +72,9 @@ export class LegacyWorkflowRunner implements WorkflowRunner { } async execute(task: Task): Promise { + if (!isValidTaskSpec(task.spec)) { + throw new InputError(`Task spec is not a valid v1beta2 task spec`); + } const { actionRegistry } = this.options; const workspacePath = path.join( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 5ff40b9459..2ab38edec9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -43,8 +43,8 @@ export type DbTaskEventRow = { createdAt: string; }; -export type TaskSpec = { - apiVersion: 'backstage.io/v1beta2' | 'backstage.io/v1beta3'; +export interface TaskSpecV1beta2 { + apiVersion: 'backstage.io/v1beta2'; baseUrl?: string; values: JsonObject; steps: Array<{ @@ -55,7 +55,24 @@ export type TaskSpec = { if?: string | boolean; }>; output: { [name: string]: string }; -}; +} + +export interface TaskStep { + id: string; + name: string; + action: string; + input?: JsonObject; + if?: string | boolean; +} +export interface TaskSpecV1beta3 { + apiVersion: 'backstage.io/v1beta3'; + baseUrl?: string; + parameters: JsonObject; + steps: TaskStep[]; + output: { [name: string]: string }; +} + +export type TaskSpec = TaskSpecV1beta2 | TaskSpecV1beta3; export type TaskSecrets = { token: string | undefined; From 1ec5ef3699ec198f4089abb836cc34d9bbd254ba Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 03:43:26 +0200 Subject: [PATCH 08/29] chore: acutally make the scaffolder backend run the v3 template Signed-off-by: blam --- .../fixtures/test-v1beta3/template.yaml | 4 + .../tasks/DefaultWorkflowRunner.test.ts | 30 ++++ .../scaffolder/tasks/DefaultWorkflowRunner.ts | 136 +++++++++++------- .../scaffolder/tasks/LegacyWorkflowRunner.ts | 2 + .../src/scaffolder/tasks/TaskWorker.test.ts | 4 +- .../scaffolder-backend/src/service/router.ts | 62 +++++--- 6 files changed, 166 insertions(+), 72 deletions(-) diff --git a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml index b885357ff9..007e23a3d6 100644 --- a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml +++ b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml @@ -12,13 +12,17 @@ spec: properties: inputString: type: string + title: string input test inputObject: type: object + title: object input test properties: first: type: string + title: first second: type: number + title: second steps: - id: debug name: Debug diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index e508d619c4..dea94e9b52 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -55,6 +55,14 @@ describe('DefaultWorkflowRunner', () => { handler: fakeActionHandler, }); + actionRegistry.register({ + id: 'output-action', + description: 'Mock action for testing', + handler: async ctx => { + ctx.output('mock', 'backstage'); + }, + }); + runner = new DefaultWorkflowRunner({ actionRegistry, integrations, @@ -189,5 +197,27 @@ describe('DefaultWorkflowRunner', () => { expect.objectContaining({ input: { foo: 1 } }), ); }); + + it('should template the output from simple actions', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'output-action', + input: {}, + }, + ], + output: { + foo: '${{steps.test.output.mock | upper}}', + }, + parameters: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.foo).toEqual('BACKSTAGE'); + }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index e84d7a71e6..c8f77f066a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -87,6 +87,43 @@ export class DefaultWorkflowRunner implements WorkflowRunner { }); } + private render(input: T, context: TemplateContext): T { + return JSON.parse(JSON.stringify(input), (_key, value) => { + try { + if (typeof value === 'string') { + const templated = this.nunjucks.renderString(value, context); + try { + return JSON.parse(templated); + } catch { + return templated; + } + } + } catch { + return value; + } + return value; + }); + } + + private makeStringifyableParams(input: T): T { + /** + * This is a little bit of a hack / magic so that when we use nunjucks and we try to + * pass through something other than a string from the parameters section. + * When an accessor is used that is an object, it's toString is the JSON.stringify'd version of it's children + * Which makes it work really well in string templating as we can parse the result again after.yarn + */ + return JSON.parse( + JSON.stringify(input), + (key: string, value: JsonObject) => { + if (typeof value === 'object' && key) { + value.toString = () => JSON.stringify(value); + } + + return value; + }, + ); + } + async execute(task: Task): Promise { if (!isValidTaskSpec(task.spec)) { throw new InputError( @@ -103,70 +140,61 @@ export class DefaultWorkflowRunner implements WorkflowRunner { `Starting up task with ${task.spec.steps.length} steps`, ); - /** - * This is a little bit of a hack / magic so that when we use nunjucks and we try to - * pass through an object from the `parameters` section of the task spec, it will - * actually work as the toString method is called from the nunjucks template. - */ - const parsedParams = JSON.parse( - JSON.stringify(task.spec.parameters), - (key: string, value: JsonObject) => { - if (typeof value === 'object' && key) { - value.toString = () => JSON.stringify(value); - } - - return value; - }, - ); - const context: TemplateContext = { - parameters: parsedParams, + parameters: this.makeStringifyableParams(task.spec.parameters), steps: {}, }; for (const step of task.spec.steps) { - const action = this.options.actionRegistry.get(step.action); - const { taskLogger, streamLogger } = createStepLogger({ task, step }); + try { + const action = this.options.actionRegistry.get(step.action); + const { taskLogger, streamLogger } = createStepLogger({ task, step }); - const input = - step.input && - JSON.parse(JSON.stringify(step.input), (_key, value) => { - try { - if (typeof value === 'string') { - const templated = this.nunjucks.renderString(value, context); - try { - return JSON.parse(templated); - } catch { - return templated; - } - } - } catch { - return value; - } - return value; + const input = step.input && this.render(step.input, context); + + const tmpDirs = new Array(); + const stepOutput: { [outputName: string]: JsonValue } = {}; + + await task.emitLog(`Beginning step ${step.name}`, { + stepId: step.id, + status: 'processing', }); - const tmpDirs = new Array(); - const stepOutputs: { [outputName: string]: JsonValue } = {}; + await action.handler({ + baseUrl: task.spec.baseUrl, + input, + logger: taskLogger, + logStream: streamLogger, + workspacePath, + createTemporaryDirectory: async () => { + const tmpDir = await fs.mkdtemp( + `${workspacePath}_step-${step.id}-`, + ); + tmpDirs.push(tmpDir); + return tmpDir; + }, + output(name: string, value: JsonValue) { + stepOutput[name] = value; + }, + }); - await action.handler({ - baseUrl: task.spec.baseUrl, - input, - logger: taskLogger, - logStream: streamLogger, - workspacePath, - createTemporaryDirectory: async () => { - const tmpDir = await fs.mkdtemp( - `${workspacePath}_step-${step.id}-`, - ); - tmpDirs.push(tmpDir); - return tmpDir; - }, - output(name: string, value: JsonValue) { - stepOutputs[name] = value; - }, - }); + context.steps[step.id] = { output: stepOutput }; + + await task.emitLog(`Finished step ${step.name}`, { + stepId: step.id, + status: 'completed', + }); + } catch (err) { + await task.emitLog(String(err.stack), { + stepId: step.id, + status: 'failed', + }); + } } + + const output = this.render(task.spec.output, context); + + return { output }; } finally { if (workspacePath) { await fs.remove(workspacePath); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts index a3c98f6cd4..97e3a4dec4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.ts @@ -18,6 +18,7 @@ import { WorkflowRunner, WorkflowResponse, TaskSpecV1beta2, + TaskSpec, } from './types'; import * as Handlebars from 'handlebars'; import { TemplateActionRegistry } from '..'; @@ -75,6 +76,7 @@ export class LegacyWorkflowRunner implements WorkflowRunner { if (!isValidTaskSpec(task.spec)) { throw new InputError(`Task spec is not a valid v1beta2 task spec`); } + const { actionRegistry } = this.options; const workspacePath = path.join( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 66c435862a..456836046c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -101,7 +101,7 @@ describe('TaskWorker', () => { output: { result: '{{ steps.test.output.testOutput }}', }, - values: {}, + parameters: {}, }); const task = await broker.claim(); @@ -130,7 +130,7 @@ describe('TaskWorker', () => { output: { result: '{{ steps.test.output.testOutput }}', }, - values: {}, + parameters: {}, }); const task = await broker.claim(); diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 077d605083..b75a5d0368 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -42,6 +42,8 @@ import { import { ScmIntegrations } from '@backstage/integration'; import { TemplateAction } from '../scaffolder/actions'; import { createBuiltinActions } from '../scaffolder/actions/builtin/createBuiltinActions'; +import { LegacyWorkflowRunner } from '../scaffolder/tasks/LegacyWorkflowRunner'; +import { DefaultWorkflowRunner } from '../scaffolder/tasks/DefaultWorkflowRunner'; export interface RouterOptions { logger: Logger; @@ -90,14 +92,28 @@ export async function createRouter( ); const taskBroker = new StorageTaskBroker(databaseTaskStore, logger); const actionRegistry = new TemplateActionRegistry(); + const legacyWorkflowRunner = new LegacyWorkflowRunner({ + logger, + actionRegistry, + integrations, + workingDirectory, + }); + + const workflowRunner = new DefaultWorkflowRunner({ + actionRegistry, + integrations, + logger, + workingDirectory, + }); + const workers = []; for (let i = 0; i < (taskWorkers || 1); i++) { const worker = new TaskWorker({ - logger, taskBroker, - actionRegistry, - workingDirectory, - integrations, + runners: { + legacyWorkflowRunner, + workflowRunner, + }, }); workers.push(worker); } @@ -184,18 +200,32 @@ export async function createRouter( } const baseUrl = getEntityBaseUrl(template); - - taskSpec = { - apiVersion: template.apiVersion, - baseUrl, - values, - steps: template.spec.steps.map((step, index) => ({ - ...step, - id: step.id ?? `step-${index + 1}`, - name: step.name ?? step.action, - })), - output: template.spec.output ?? {}, - }; + // TODO: need to make sure that the TaskSpec is the right format here. + // If it's beta2 use values, beta3 uses parameters to clear that up. + taskSpec = + template.apiVersion === 'backstage.io/v1beta2' + ? { + apiVersion: template.apiVersion, + baseUrl, + values, + steps: template.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: template.spec.output ?? {}, + } + : { + apiVersion: template.apiVersion, + baseUrl, + parameters: values, + steps: template.spec.steps.map((step, index) => ({ + ...step, + id: step.id ?? `step-${index + 1}`, + name: step.name ?? step.action, + })), + output: template.spec.output ?? {}, + }; } else { throw new InputError( `Unsupported apiVersion field in schema entity, ${ From c7b089a5aeb2817e91543dee1db571fe7e66d7d3 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 04:10:33 +0200 Subject: [PATCH 09/29] chore: adding the new if syntax and using the power of nunjucks Signed-off-by: blam --- .../tasks/DefaultWorkflowRunner.test.ts | 72 ++++++++++++++++++- .../scaffolder/tasks/DefaultWorkflowRunner.ts | 15 ++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index dea94e9b52..c249606658 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -60,6 +60,7 @@ describe('DefaultWorkflowRunner', () => { description: 'Mock action for testing', handler: async ctx => { ctx.output('mock', 'backstage'); + ctx.output('shouldRun', true); }, }); @@ -85,7 +86,76 @@ describe('DefaultWorkflowRunner', () => { }); describe('validation', () => {}); - describe('running', () => {}); + describe('conditionals', () => { + it('should execute steps conditionally', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { id: 'test', name: 'test', action: 'output-action' }, + { + id: 'conditional', + name: 'conditional', + action: 'output-action', + if: '${{ steps.test.output.shouldRun }}', + }, + ], + output: { + result: '${{ steps.conditional.output.mock }}', + }, + parameters: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBe('backstage'); + }); + + it('should skips steps conditionally', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { id: 'test', name: 'test', action: 'output-action' }, + { + id: 'conditional', + name: 'conditional', + action: 'output-action', + if: '${{ not steps.test.output.shouldRun}}', + }, + ], + output: { + result: '${{ steps.conditional.output.mock }}', + }, + parameters: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBeUndefined(); + }); + + it('should skips steps using the negating equals operator', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { id: 'test', name: 'test', action: 'output-action' }, + { + id: 'conditional', + name: 'conditional', + action: 'output-action', + if: '${{ steps.test.output.mock !== "backstage"}}', + }, + ], + output: { + result: '${{ steps.conditional.output.mock }}', + }, + parameters: {}, + }); + + const { output } = await runner.execute(task); + + expect(output.result).toBeUndefined(); + }); + }); describe('templating', () => { it('should template the input to an action', async () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index c8f77f066a..889048d04a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -30,6 +30,7 @@ import path from 'path'; import { JsonObject, JsonValue } from '@backstage/config'; import { InputError } from '@backstage/errors'; import { PassThrough } from 'stream'; +import { isTruthy } from './helper'; type Options = { workingDirectory: string; @@ -92,6 +93,9 @@ export class DefaultWorkflowRunner implements WorkflowRunner { try { if (typeof value === 'string') { const templated = this.nunjucks.renderString(value, context); + if (templated === '') { + return undefined; + } try { return JSON.parse(templated); } catch { @@ -147,6 +151,16 @@ export class DefaultWorkflowRunner implements WorkflowRunner { for (const step of task.spec.steps) { try { + if (step.if) { + const ifResult = await this.render(step.if, context); + if (!isTruthy(ifResult)) { + await task.emitLog( + `Skipping step ${step.id} because it's if condition was false`, + ); + continue; + } + } + const action = this.options.actionRegistry.get(step.action); const { taskLogger, streamLogger } = createStepLogger({ task, step }); @@ -189,6 +203,7 @@ export class DefaultWorkflowRunner implements WorkflowRunner { stepId: step.id, status: 'failed', }); + throw err; } } From fe2324e63007b50e5be1c885c9de406c6df8415a Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 04:29:19 +0200 Subject: [PATCH 10/29] chore: added validation for schemas for actions Signed-off-by: blam Signed-off-by: blam --- .../tasks/DefaultWorkflowRunner.test.ts | 52 ++++++++++++++++++- .../scaffolder/tasks/DefaultWorkflowRunner.ts | 18 ++++++- 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index c249606658..6447782654 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -55,6 +55,23 @@ describe('DefaultWorkflowRunner', () => { handler: fakeActionHandler, }); + actionRegistry.register({ + id: 'jest-validated-action', + description: 'Mock action for testing', + handler: fakeActionHandler, + schema: { + input: { + type: 'object', + required: ['foo'], + properties: { + foo: { + type: 'number', + }, + }, + }, + }, + }); + actionRegistry.register({ id: 'output-action', description: 'Mock action for testing', @@ -85,7 +102,40 @@ describe('DefaultWorkflowRunner', () => { ); }); - describe('validation', () => {}); + describe('validation', () => { + it('should throw an error if the action has a schema and the input does not match', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [{ id: 'test', name: 'name', action: 'jest-validated-action' }], + }); + + await expect(runner.execute(task)).rejects.toThrowError( + /Invalid input passed to action jest-validated-action, instance requires property \"foo\"/, + ); + }); + + it('should run the action when the validation passes', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-validated-action', + input: { foo: 1 }, + }, + ], + }); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledTimes(1); + }); + }); describe('conditionals', () => { it('should execute steps conditionally', async () => { const task = createMockTaskWithSpec({ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 889048d04a..c08363857d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -31,6 +31,7 @@ import { JsonObject, JsonValue } from '@backstage/config'; import { InputError } from '@backstage/errors'; import { PassThrough } from 'stream'; import { isTruthy } from './helper'; +import { validate as validateJsonSchema } from 'jsonschema'; type Options = { workingDirectory: string; @@ -79,6 +80,8 @@ export class DefaultWorkflowRunner implements WorkflowRunner { private readonly nunjucks: nunjucks.Environment; constructor(private readonly options: Options) { + // TODO(blam): Probably need the repo helper here. + // Or we move to returning Objects in the RepoUrlPickerV2 or something? this.nunjucks = nunjucks.configure({ autoescape: false, tags: { @@ -164,7 +167,20 @@ export class DefaultWorkflowRunner implements WorkflowRunner { const action = this.options.actionRegistry.get(step.action); const { taskLogger, streamLogger } = createStepLogger({ task, step }); - const input = step.input && this.render(step.input, context); + const input = (step.input && this.render(step.input, context)) ?? {}; + + if (action.schema?.input) { + const validateResult = validateJsonSchema( + input, + action.schema.input, + ); + if (!validateResult.valid) { + const errors = validateResult.errors.join(', '); + throw new InputError( + `Invalid input passed to action ${action.id}, ${errors}`, + ); + } + } const tmpDirs = new Array(); const stepOutput: { [outputName: string]: JsonValue } = {}; From 154b86e1da0bb1eb2f4773f32c4c4eec9c430f97 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 04:45:09 +0200 Subject: [PATCH 11/29] feat: think we're at feature parity Signed-off-by: blam --- .../fixtures/test-v1beta3/template.yaml | 47 +++++++++---------- .../tasks/DefaultWorkflowRunner.test.ts | 29 ++++++++++++ .../scaffolder/tasks/DefaultWorkflowRunner.ts | 15 +++++- .../src/scaffolder/tasks/types.ts | 4 +- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml index 007e23a3d6..01bd5c2e4f 100644 --- a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml +++ b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml @@ -7,28 +7,27 @@ metadata: spec: type: website parameters: - - name: Enter some stuff - description: Enter some stuff - properties: - inputString: - type: string - title: string input test - inputObject: - type: object - title: object input test - properties: - first: - type: string - title: first - second: - type: number - title: second + - name: Enter some stuff + description: Enter some stuff + properties: + inputString: + type: string + title: string input test + inputObject: + type: object + title: object input test + properties: + first: + type: string + title: first + second: + type: number + title: second steps: - - id: debug - name: Debug - action: debug:log - input: - message: ${{ parameters.inputString }} - extra: ${{ parameters.inputObject }} - - + - id: debug + if: ${{ true === true }} + name: Debug + action: debug:log + input: + message: ${{ parameters.inputString }} + extra: ${{ parameters.inputObject }} diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index 6447782654..d1e2c7e098 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -136,6 +136,7 @@ describe('DefaultWorkflowRunner', () => { expect(fakeActionHandler).toHaveBeenCalledTimes(1); }); }); + describe('conditionals', () => { it('should execute steps conditionally', async () => { const task = createMockTaskWithSpec({ @@ -340,4 +341,32 @@ describe('DefaultWorkflowRunner', () => { expect(output.foo).toEqual('BACKSTAGE'); }); }); + + describe('filters', () => { + it('provides the parseRepoUrl filter', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'output-action', + input: {}, + }, + ], + output: { + foo: '${{parameters.repoUrl | parseRepoUrl}}', + }, + parameters: { + repoUrl: 'github.com?repo=repo&owner=owner', + }, + }); + + const { output } = await runner.execute(task); + + expect(output.foo.host).toEqual('github.com'); + expect(output.foo.owner).toEqual('owner'); + expect(output.foo.repo).toEqual('repo'); + }); + }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index c08363857d..30b855ac30 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -32,6 +32,7 @@ import { InputError } from '@backstage/errors'; import { PassThrough } from 'stream'; import { isTruthy } from './helper'; import { validate as validateJsonSchema } from 'jsonschema'; +import { parseRepoUrl } from '../actions/builtin/publish/util'; type Options = { workingDirectory: string; @@ -80,8 +81,6 @@ export class DefaultWorkflowRunner implements WorkflowRunner { private readonly nunjucks: nunjucks.Environment; constructor(private readonly options: Options) { - // TODO(blam): Probably need the repo helper here. - // Or we move to returning Objects in the RepoUrlPickerV2 or something? this.nunjucks = nunjucks.configure({ autoescape: false, tags: { @@ -89,6 +88,18 @@ export class DefaultWorkflowRunner implements WorkflowRunner { variableEnd: '}}', }, }); + + // TODO(blam): let's work out how we can deprecate these. + // We shouln't really need to be exposing these now we can deal with + // objects in the params block + this.nunjucks.addFilter('parseRepoUrl', repoUrl => { + return JSON.stringify(parseRepoUrl(repoUrl, this.options.integrations)); + }); + + this.nunjucks.addFilter('projectSlug', repoUrl => { + const { owner, repo } = parseRepoUrl(repoUrl, this.options.integrations); + return `${owner}/${repo}`; + }); } private render(input: T, context: TemplateContext): T { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 2ab38edec9..08baaca7f0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -69,7 +69,7 @@ export interface TaskSpecV1beta3 { baseUrl?: string; parameters: JsonObject; steps: TaskStep[]; - output: { [name: string]: string }; + output: { [name: string]: JsonValue }; } export type TaskSpec = TaskSpecV1beta2 | TaskSpecV1beta3; @@ -141,7 +141,7 @@ export interface TaskStore { }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }>; } -export type WorkflowResponse = { output: { [name: string]: JsonValue } }; +export type WorkflowResponse = { output: { [key: string]: JsonObject } }; export interface WorkflowRunner { execute(task: Task): Promise; } From 5bdbb6caec21c6ce2e09ecb7d57f15346b2aab44 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 04:56:34 +0200 Subject: [PATCH 12/29] chore: fix up some types Signed-off-by: blam --- .../src/scaffolder/tasks/DefaultWorkflowRunner.test.ts | 8 +++++--- .../src/scaffolder/tasks/DefaultWorkflowRunner.ts | 3 ++- .../src/scaffolder/tasks/TaskWorker.test.ts | 6 +----- plugins/scaffolder-backend/src/scaffolder/tasks/types.ts | 2 +- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index d1e2c7e098..0217236e92 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -364,9 +364,11 @@ describe('DefaultWorkflowRunner', () => { const { output } = await runner.execute(task); - expect(output.foo.host).toEqual('github.com'); - expect(output.foo.owner).toEqual('owner'); - expect(output.foo.repo).toEqual('repo'); + expect(output.foo).toEqual({ + host: 'github.com', + owner: 'owner', + repo: 'repo', + }); }); }); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 30b855ac30..ab1525fe49 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -91,7 +91,8 @@ export class DefaultWorkflowRunner implements WorkflowRunner { // TODO(blam): let's work out how we can deprecate these. // We shouln't really need to be exposing these now we can deal with - // objects in the params block + // objects in the params block. + // Maybe we can expose a new RepoUrlPicker with secrets for V3 that provides an object already. this.nunjucks.addFilter('parseRepoUrl', repoUrl => { return JSON.stringify(parseRepoUrl(repoUrl, this.options.integrations)); }); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 456836046c..035398d448 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -14,15 +14,11 @@ * limitations under the License. */ -import os from 'os'; import { getVoidLogger, DatabaseManager } from '@backstage/backend-common'; -import { ConfigReader, JsonObject } from '@backstage/config'; -import { createTemplateAction, TemplateActionRegistry } from '../actions'; -import { RepoSpec } from '../actions/builtin/publish/util'; +import { ConfigReader } from '@backstage/config'; import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker } from './StorageTaskBroker'; import { TaskWorker } from './TaskWorker'; -import { ScmIntegrations } from '@backstage/integration'; import { WorkflowRunner } from './types'; import { LegacyWorkflowRunner } from './LegacyWorkflowRunner'; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 08baaca7f0..4f11d37da6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -141,7 +141,7 @@ export interface TaskStore { }: TaskStoreGetEventsOptions): Promise<{ events: DbTaskEventRow[] }>; } -export type WorkflowResponse = { output: { [key: string]: JsonObject } }; +export type WorkflowResponse = { output: { [key: string]: JsonValue } }; export interface WorkflowRunner { execute(task: Task): Promise; } From f8355c99cb649d286354a83e07da0f77c542d536 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 13:58:31 +0200 Subject: [PATCH 13/29] chore: remove all tracked temp directories and make sure to emit the processing and skipped events Signed-off-by: blam --- .../src/scaffolder/tasks/DefaultWorkflowRunner.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index ab1525fe49..9efb3f24bf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -129,7 +129,7 @@ export class DefaultWorkflowRunner implements WorkflowRunner { * This is a little bit of a hack / magic so that when we use nunjucks and we try to * pass through something other than a string from the parameters section. * When an accessor is used that is an object, it's toString is the JSON.stringify'd version of it's children - * Which makes it work really well in string templating as we can parse the result again after.yarn + * Which makes it work really well in string templating as we can parse the result again after. */ return JSON.parse( JSON.stringify(input), @@ -171,11 +171,17 @@ export class DefaultWorkflowRunner implements WorkflowRunner { if (!isTruthy(ifResult)) { await task.emitLog( `Skipping step ${step.id} because it's if condition was false`, + { stepId: step.id, status: 'skipped' }, ); continue; } } + await task.emitLog(`Beginning step ${step.name}`, { + stepId: step.id, + status: 'processing', + }); + const action = this.options.actionRegistry.get(step.action); const { taskLogger, streamLogger } = createStepLogger({ task, step }); @@ -220,6 +226,11 @@ export class DefaultWorkflowRunner implements WorkflowRunner { }, }); + // Remove all temporary directories that were created when executing the action + for (const tmpDir of tmpDirs) { + await fs.remove(tmpDir); + } + context.steps[step.id] = { output: stepOutput }; await task.emitLog(`Finished step ${step.name}`, { From 5a0d2e9fa55addf41ded8258138bf8b2cdf40cf6 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 14:01:51 +0200 Subject: [PATCH 14/29] feat(catalog-model/docs): Updating `api-report` for `catalog-model` Signed-off-by: blam Signed-off-by: blam --- packages/catalog-model/api-report.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/catalog-model/api-report.md b/packages/catalog-model/api-report.md index 1e7eb8fd52..1b6c3029b7 100644 --- a/packages/catalog-model/api-report.md +++ b/packages/catalog-model/api-report.md @@ -553,6 +553,33 @@ export interface TemplateEntityV1beta2 extends Entity { // @public (undocumented) export const templateEntityV1beta2Validator: KindValidator; +// @public (undocumented) +export interface TemplateEntityV1beta3 extends Entity { + // (undocumented) + apiVersion: 'backstage.io/v1beta3'; + // (undocumented) + kind: 'Template'; + // (undocumented) + spec: { + type: string; + parameters?: JsonObject | JsonObject[]; + steps: Array<{ + id?: string; + name?: string; + action: string; + input?: JsonObject; + if?: string | boolean; + }>; + output?: { + [name: string]: string; + }; + owner?: string; + }; +} + +// @public (undocumented) +export const templateEntityV1beta3Validator: KindValidator; + // @alpha export type UNSTABLE_EntityStatus = { items?: UNSTABLE_EntityStatusItem[]; From 18083d18213fbe8d70e10e5d5036ee3ef9061df1 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 14:08:35 +0200 Subject: [PATCH 15/29] chore: add changeset Signed-off-by: blam --- .changeset/polite-timers-watch.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/polite-timers-watch.md diff --git a/.changeset/polite-timers-watch.md b/.changeset/polite-timers-watch.md new file mode 100644 index 0000000000..d8f2ea80fb --- /dev/null +++ b/.changeset/polite-timers-watch.md @@ -0,0 +1,7 @@ +--- +'@backstage/catalog-model': patch +'@backstage/plugin-catalog-backend': patch +'@backstage/plugin-scaffolder-backend': patch +--- + +Introduce the new `backstage.io/v1beta3` template kind with nunjucks support 🥋 From cbe2803100e1d4d00037db77e40a8e2051c0b02b Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 15:39:31 +0200 Subject: [PATCH 16/29] chore: remove the hackery and wrap up the template stringify Signed-off-by: blam --- .../tasks/DefaultWorkflowRunner.test.ts | 30 ++++++++- .../scaffolder/tasks/DefaultWorkflowRunner.ts | 61 +++++++++++-------- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index 0217236e92..542cbb4b2f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -235,6 +235,34 @@ describe('DefaultWorkflowRunner', () => { ); }); + it('should keep the original types for the input and not parse things that arent meant to be parsed', async () => { + const task = createMockTaskWithSpec({ + apiVersion: 'backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: { + number: '${{parameters.number}}', + string: '${{parameters.string}}', + }, + }, + ], + output: {}, + parameters: { + number: 0, + string: '1', + }, + }); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledWith( + expect.objectContaining({ input: { number: 0, string: '1' } }), + ); + }); + it('should template complex values into the action', async () => { const task = createMockTaskWithSpec({ apiVersion: 'backstage.io/v1beta3', @@ -355,7 +383,7 @@ describe('DefaultWorkflowRunner', () => { }, ], output: { - foo: '${{parameters.repoUrl | parseRepoUrl}}', + foo: '${{ parameters.repoUrl | parseRepoUrl }}', }, parameters: { repoUrl: 'github.com?repo=repo&owner=owner', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 9efb3f24bf..da59951d36 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -94,7 +94,7 @@ export class DefaultWorkflowRunner implements WorkflowRunner { // objects in the params block. // Maybe we can expose a new RepoUrlPicker with secrets for V3 that provides an object already. this.nunjucks.addFilter('parseRepoUrl', repoUrl => { - return JSON.stringify(parseRepoUrl(repoUrl, this.options.integrations)); + return parseRepoUrl(repoUrl, this.options.integrations); }); this.nunjucks.addFilter('projectSlug', repoUrl => { @@ -107,15 +107,43 @@ export class DefaultWorkflowRunner implements WorkflowRunner { return JSON.parse(JSON.stringify(input), (_key, value) => { try { if (typeof value === 'string') { + try { + // Let's assume that we're dealing with a template string. + if (value.startsWith('${{') && value.endsWith('}}')) { + // Lets convert ${{ parameters.bob }} to ${{ (parameters.bob) | dump }} so we can keep the input type + const wrappedDumped = value.replace( + /\${{(.+)}}/g, + '${{ ( $1 ) | dump }}', + ); + + // Run the templating + const templated = this.nunjucks.renderString( + wrappedDumped, + context, + ); + + // If there's emtpy string returned, then it's undefined + if (templated === '') { + return undefined; + } + + // Reparse the dumped string + return JSON.parse(templated); + } + } catch (ex) { + this.options.logger.debug( + `Failed to parse template string: ${value} with error ${ex.message}`, + ); + } + + // Fallback to default behaviour const templated = this.nunjucks.renderString(value, context); + if (templated === '') { return undefined; } - try { - return JSON.parse(templated); - } catch { - return templated; - } + + return templated; } } catch { return value; @@ -124,25 +152,6 @@ export class DefaultWorkflowRunner implements WorkflowRunner { }); } - private makeStringifyableParams(input: T): T { - /** - * This is a little bit of a hack / magic so that when we use nunjucks and we try to - * pass through something other than a string from the parameters section. - * When an accessor is used that is an object, it's toString is the JSON.stringify'd version of it's children - * Which makes it work really well in string templating as we can parse the result again after. - */ - return JSON.parse( - JSON.stringify(input), - (key: string, value: JsonObject) => { - if (typeof value === 'object' && key) { - value.toString = () => JSON.stringify(value); - } - - return value; - }, - ); - } - async execute(task: Task): Promise { if (!isValidTaskSpec(task.spec)) { throw new InputError( @@ -160,7 +169,7 @@ export class DefaultWorkflowRunner implements WorkflowRunner { ); const context: TemplateContext = { - parameters: this.makeStringifyableParams(task.spec.parameters), + parameters: task.spec.parameters, steps: {}, }; From f39de106b5677e3dbb71047f52f9ab5fdc6930f0 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 17:51:25 +0200 Subject: [PATCH 17/29] chore: only dump when there's only one param ref Signed-off-by: blam --- .../scaffolder/tasks/DefaultWorkflowRunner.ts | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index da59951d36..63ba392dcc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -80,17 +80,19 @@ const createStepLogger = ({ task, step }: { task: Task; step: TaskStep }) => { export class DefaultWorkflowRunner implements WorkflowRunner { private readonly nunjucks: nunjucks.Environment; + private readonly nunjucksOptions: nunjucks.ConfigureOptions = { + autoescape: false, + tags: { + variableStart: '${{', + variableEnd: '}}', + }, + }; + constructor(private readonly options: Options) { - this.nunjucks = nunjucks.configure({ - autoescape: false, - tags: { - variableStart: '${{', - variableEnd: '}}', - }, - }); + this.nunjucks = nunjucks.configure(this.nunjucksOptions); // TODO(blam): let's work out how we can deprecate these. - // We shouln't really need to be exposing these now we can deal with + // We shouldn't really need to be exposing these now we can deal with // objects in the params block. // Maybe we can expose a new RepoUrlPicker with secrets for V3 that provides an object already. this.nunjucks.addFilter('parseRepoUrl', repoUrl => { @@ -103,13 +105,21 @@ export class DefaultWorkflowRunner 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) + ); + } + private render(input: T, context: TemplateContext): T { return JSON.parse(JSON.stringify(input), (_key, value) => { try { if (typeof value === 'string') { try { - // Let's assume that we're dealing with a template string. - if (value.startsWith('${{') && value.endsWith('}}')) { + if (this.isSingleTemplateString(value)) { // Lets convert ${{ parameters.bob }} to ${{ (parameters.bob) | dump }} so we can keep the input type const wrappedDumped = value.replace( /\${{(.+)}}/g, @@ -122,7 +132,7 @@ export class DefaultWorkflowRunner implements WorkflowRunner { context, ); - // If there's emtpy string returned, then it's undefined + // If there's an empty string returned, then it's undefined if (templated === '') { return undefined; } @@ -131,7 +141,7 @@ export class DefaultWorkflowRunner implements WorkflowRunner { return JSON.parse(templated); } } catch (ex) { - this.options.logger.debug( + this.options.logger.error( `Failed to parse template string: ${value} with error ${ex.message}`, ); } From 808d30cc307bb2ec5118153718d99e9f508ad520 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 28 Sep 2021 20:29:19 +0200 Subject: [PATCH 18/29] docs: removing older documentation for now Signed-off-by: blam --- docs/features/software-templates/legacy.md | 117 ------ .../migrating-from-v1alpha1-to-v1beta2.md | 333 ------------------ .../migrating-from-v1beta2-to-v1beta3.md | 148 ++++++++ microsite/sidebars.json | 3 +- 4 files changed, 149 insertions(+), 452 deletions(-) delete mode 100644 docs/features/software-templates/legacy.md delete mode 100644 docs/features/software-templates/migrating-from-v1alpha1-to-v1beta2.md create mode 100644 docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md diff --git a/docs/features/software-templates/legacy.md b/docs/features/software-templates/legacy.md deleted file mode 100644 index 49f3e21904..0000000000 --- a/docs/features/software-templates/legacy.md +++ /dev/null @@ -1,117 +0,0 @@ ---- -id: template-legacy -title: Writing Templates (Legacy) -# prettier-ignore -description: Old documentation describing the backstage.io/v1alpha1 format of the Template Schema ---- - -## Kind: Template - -Describes the following entity kind: - -| Field | Value | -| ------------ | ----------------------- | -| `apiVersion` | `backstage.io/v1alpha1` | -| `kind` | `Template` | - -A Template describes a skeleton for use with the Scaffolder. It is used for -describing what templating library is supported, and also for documenting the -variables that the template requires using -[JSON Forms Schema](https://jsonforms.io/). - -Descriptor files for this kind may look as follows. - -```yaml -apiVersion: backstage.io/v1alpha1 -kind: Template -metadata: - name: react-ssr-template - title: React SSR Template - description: - Next.js application skeleton for creating isomorphic web applications. - tags: - - recommended - - react -spec: - owner: web@example.com - templater: cookiecutter - type: website - path: '.' - schema: - required: - - component_id - - description - properties: - component_id: - title: Name - type: string - description: Unique name of the component - description: - title: Description - type: string - description: Description of the component -``` - -In addition to the [common envelope metadata](#common-to-all-kinds-the-metadata) -shape, this kind has the following structure. - -### `apiVersion` and `kind` [required] - -Exactly equal to `backstage.io/v1alpha1` and `Template`, respectively. - -### `metadata.title` [required] - -The nice display name for the template as a string, e.g. `React SSR Template`. -This field is required as is used to reference the template to the user instead -of the `metadata.name` field. - -### `metadata.tags` [optional] - -A list of strings that can be associated with the template, e.g. -`['recommended', 'react']`. - -This list will also be used in the frontend to display to the user so you can -potentially search and group templates by these tags. - -### `spec.type` [optional] - -The type of component as a string, e.g. `website`. This field is optional but -recommended. - -The software catalog accepts any type value, but an organization should take -great care to establish a proper taxonomy for these. Tools including Backstage -itself may read this field and behave differently depending on its value. For -example, a website type component may present tooling in the Backstage interface -that is specific to just websites. - -The current set of well-known and common values for this field is: - -- `service` - a backend service, typically exposing an API -- `website` - a website -- `library` - a software library, such as an npm module or a Java library - -### `spec.templater` [required] - -The templating library that is supported by the template skeleton as a string, -e.g `cookiecutter`. - -Different skeletons will use different templating syntax, so it's common that -the template will need to be run with a particular piece of software. - -This key will be used to identify the correct templater which is registered into -the `TemplatersBuilder`. - -The values which are available by default are: - -- `cookiecutter` - [cookiecutter](https://github.com/cookiecutter/cookiecutter). - -### `spec.path` [optional] - -The string location where the templater should be run if it is not on the same -level as the `template.yaml` definition, e.g. `./cookiecutter/skeleton`. - -This will set the `cwd` when running the templater to the folder path that you -specify relative to the `template.yaml` definition. - -This is also particularly useful when you have multiple template definitions in -the same repository but only a single `template.yaml` registered in backstage. diff --git a/docs/features/software-templates/migrating-from-v1alpha1-to-v1beta2.md b/docs/features/software-templates/migrating-from-v1alpha1-to-v1beta2.md deleted file mode 100644 index 58d88e8e9e..0000000000 --- a/docs/features/software-templates/migrating-from-v1alpha1-to-v1beta2.md +++ /dev/null @@ -1,333 +0,0 @@ ---- -id: migrating-from-v1alpha1-to-v1beta2 -title: Migrating to v1beta2 templates -# prettier-ignore -description: How to move your old templates from v1alpha1 to the more declarative v1beta2 ---- - -# What's new? - -Previously, the scaffolder was very restricted in what you could do when -creating new software components from templates. There were three scaffolding -steps which was pretty hard to extend and add new functionality to, difficult to -re-use logic between templates. There used to be a fixed pipeline of -`preparers`, `templaters`, and `publishers`, which were defined by the backend -and needed to be run for each template. This is now changed, to give the -template total control over what should be executed as part of the templating -run. This makes templates a little more declarative as you can now register -different `actions` or `functions` with the `scaffolder-backend` which you then -can decide how, and in what order, to run using the template definition YAML -file. - -We've also made some improvements, and added some helpers to work with -cookiecutter. The skeleton for a template can now be stored in a different place -to where your entity definition is: previously you needed to have your -`template.yaml` next to the skeleton source (`{{cookiecutter.component_id}}` -directory), but now that's not the case. Part of the changes with the `v1beta2` -syntax is that you can grab your template source from any repository, and re-use -them between templates. - -We've also renamed the `schema` property to `parameters` as this makes more -sense when using them as parameters to the actions or steps that you've setup -for your templates. There's the added benefit that you can now assign an array -to the `parameters` property, which will then give you multiple steps in the UI, -so you can split apart your input parameters and group them as needed rather -than having one long list of input fields. - -## The `parameters` property - -The `schema` key has now been renamed to `parameters` with a few more features. -You can pass an array now to break apart the input form into different steps in -the UI. You can also specify `ui:schema` fields that are passed along to -[`react-jsonschema-form`](https://rjsf-team.github.io/react-jsonschema-form/) -inline with the JSON schema. - -```yaml -spec: - parameters: - - title: Fill in some steps - required: - - name - properties: - name: - title: Name - type: string - description: Unique name of the component - ui:autofocus: true - ui:options: - rows: 5 -``` - -## The `steps` property - -`v1beta2` template syntax introduces the new `steps` property, which is an array -of `actions` that the scaffolder will run in combination with the user input -that is declared in the `schema`. Actions look like the following: - -```yaml -spec: - steps: - - id: publish # a unique id for the step, can be anything you like - name: Publish # a user friendly name for the step, this is what is shown in the frontend - action: publish:github # the action ID that has been registered with the scaffolder-backend - input: # parameters that are passed as input to the action handler function - allowedHosts: ['github.com'] - description: 'This is {{ parameters.name }}' # handlebars templating is supported with the values from the parameters section in the same file. - repoUrl: '{{ parameters.repoUrl }}' -``` - -# Migrating a `v1alpha1` template - -## The template definition (.yaml) - -### `parameters` - -Because of the changes to invert the control to the `template.yaml` definition -for running the workflow, we need to adjust the `schema` property and we also -now need to define what the template is actually going to do as part of the -template run. - -A simple migration would move the following yaml: - -```yaml -apiVersion: backstage.io/v1alpha1 -kind: Template -metadata: - name: react-ssr-template - title: React SSR Template - description: Create a website powered with Next.js - tags: - - recommended - - react -spec: - owner: web@example.com - templater: cookiecutter - type: website - path: '.' - schema: - required: - - component_id - - description - properties: - component_id: - title: Name - type: string - description: Unique name of the component - description: - title: Description - type: string - description: Help others understand what this website is for. -``` - -To something that looks like the following: - -```yaml -apiVersion: backstage.io/v1beta2 -kind: Template -metadata: - name: react-ssr-template - title: React SSR Template - description: Create a website powered with Next.js - tags: - - recommended - - react -spec: - owner: web@example.com - type: website - parameters: - - title: Add some input - required: - - component_id - - description - properties: - component_id: - title: Name - type: string - description: Unique name of the component - description: - title: Description - type: string - description: Help others understand what this website is for. - - title: Some more additional info that was previously provided automatically - required: - - owner - - repoUrl - properties: - owner: - title: Owner - type: string - description: Owner of the component - ui:field: OwnerPicker - ui:options: - allowedKinds: - - Group - - title: Choose a location - repoUrl: - title: Repository Location - type: string - ui:field: RepoUrlPicker - ui:options: - allowedHosts: - - github.com -``` - -There are a few things to note here. On the `alpha` version, the second step of -the template flow in the frontend was provided by Backstage for free, so we used -to collect the user input for the `owner` field and the `repositoryUrl` that you -were going to publish to. Now because `actions` can have any workflow they like, -it doesn't make sense to still provide these fields for every scaffolding -workflow, as you might not need these anymore. That's why we now manually add -those fields back into the template parameters that are shown to the user: - -```yaml - - title: Some more additional info that was previously provided automatically - required: - - owner - - repoUrl - properties: - owner: - title: Owner - type: string - description: Owner of the component - ui:field: OwnerPicker - ui:options: - allowedKinds: - - Group - - title: Choose a location - repoUrl: - title: Repository Location - type: string - ui:field: RepoUrlPicker - ui:options: - allowedHosts: - - github.com -``` - -Maybe you also don't need to publish to `github.com`, you should replace this -with your VCS provider URL that is listed in your `integrations` config instead. - -### `steps` - -So now we should have all the required information that we need from the user in -a much more extensible way. We now need to tell the scaffolder what to do with -these parameters and what to do with the user input. - -We've made templating using `cookiecutter` a little simpler. You don't need to -store the `cookiecutter` skeleton in the same directory as the `template.yaml` -definition, it can live wherever you like - maybe a shared repository somewhere -so you can re-use the skeletons but apply different actions for different -templates depending on your use case. - -We also no longer need to have a directory called -`{{cookiecutter.component_id}}`. This is because now we can't ensure that -`component_id` will be a parameter that is provided from the frontend, this -could break `cookiecutter`. If your directory structure used to look like this: - -``` -my-awesome-template - -> {{cookiecutter.component_id}} - -> file.txt - -> some_more_files.ts - -> hooks - -> post_gen_project.sh - -> template.yaml -``` - -We now recommend that you move to the following structure: - -``` -my-awesome-template - -> skeleton - -> file.txt - -> some_more_files.ts - -> template.yaml -``` - -This migration renames the skeleton folder to something more semantic, and also -drops support for `cookiecutter` hooks. We've dropped support for `cookiecutter` -hooks for now, as hopefully everything that is stored in these hooks can be -moved to `actions` instead, and for security reasons, it's more secure to run -trusted code that you ship with Backstage as an action rather than some script -that can be pulled in from anywhere which doesn't get vetted first. It's a -pretty big security risk that those scripts will be run on Backstage instances -inside your infrastructure, especially `.sh` files. - -If you really need hooks and can't find a suitable solution by using actions -please reach out to us through a ticket and we'll see what we can do to assist -:) - -You'll notice that we removed the `templater` property from the `spec` -definition in the template `yaml`, so there's no way to define that this is a -`cookiecutter` `templater`. - -We've created a built-in action that you can use which will when run, go grab a -directory from anywhere and run `cookiecutter` on top of it, and then extract -the contents into the working directory for the scaffolder. - -Adding the `steps` for a simple template should look something like the -following: - -```yaml -spec: - steps: - # this action will go use cookiecutter to template some files into the working directory - - id: template # an ID for the templating step - name: Create skeleton # A user friendly name for the action - action: fetch:cookiecutter - input: - url: ./skeleton # this is the directory for your skeleton files. - # If it's located next to the `template.yaml` then you can use a relative path, - # otherwise you can use absolute URLs that point at the VCS: https://github.com/backstage/backstage/tree/master/some_folder_somewhere - values: - # for each value that you need to pass to cookiecutter, they should be listed here and set in this values object. - # You can use the handlebars templating syntax to pull them from the input parameters listed in the same file - name: '{{ parameters.name }}' - owner: '{{ parameters.owner }}' - destination: '{{ parseRepoUrl parameters.repoUrl }}' - - # this action is for publishing the working directory to the VCS - - id: publish - name: Publish - action: publish:github - input: - allowedHosts: ['github.com'] - description: 'This is {{ parameters.name }}' - repoUrl: '{{ parameters.repoUrl }}' - - # this action will then register the created component in Backstage - - id: register - name: Register - action: catalog:register - input: - repoContentsUrl: '{{ steps.publish.output.repoContentsUrl }}' - catalogInfoPath: '/catalog-info.yaml' -``` - -### `output` - -Steps can output values, and so can the template itself. This is good for -returning values to the frontend, so we can make the buttons like -`Go to catalog` and `Go to repo` work correctly. You can add the following to -your `template.yaml` to make sure you return the right values from the steps: - -```yaml -spec: - output: - remoteUrl: '{{ steps.publish.output.remoteUrl }}' - entityRef: '{{ steps.register.output.entityRef }}' -``` - -Or you can return a `links` array with text and a URL explicitly: - -```yaml -spec: - output: - links: - - url: '{{steps.publish.output.remoteUrl}}' - title: 'Go to Repo' -``` - -## Questions? - -If you have any questions or feedback, please reach out to us on GitHub or -Discord and we will do our best to help! diff --git a/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md b/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md new file mode 100644 index 0000000000..8ccacfa301 --- /dev/null +++ b/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md @@ -0,0 +1,148 @@ +--- +id: migrating-from-v1beta2-to-v1beta3 +title: Migrating to v1beta3 templates +# prettier-ignore +description: How to migrate your existing templates to beta3 syntax +--- + +# What's new? + +Well then, here we are! 🚀 + +Backstage has had many forms of templating languages throughout different +plugins and different systems. We've had `cookiecutter` syntax in templates, and +we also had `handlebars` templating in the `kind: Template`. Then we wanted to +remove the additional dependency on `cookiecutter` for `Software Templates` out +of the box, so we introduced `nunjucks` as an alternative in `fetch:template` +action which is based on the `jinja2` syntax so they're pretty similar. In an +effort to reduce confusion and unify on to one templating language, we're +officially deprecating support for `handlebars` templating in the +`kind: Template` entities with version `backstage.io/v1beta3` and moving to +using `nunjucks` instead. + +This provides us a lot of built in `filters` (`handlebars` helpers), that as +Template authors will give you much more flexibility out of the box, and also +open up sharing of filters in the `entity` and the actual `skeleton` too, and +removing the slight differences between the two languages. + +We've also removed a lot of the built in helpers that we shipped with +`handlebars`, as they're now supported as first class citizens by either +`nunjucks` or the new `scaffolder` when using `backstage.io/v1beta3` +`apiVersion` + +The migration path is pretty simple, and we've removed some of the pain points +from writing the `handlebars` templates too. Let's go through what's new and how +to upgrade. + +## `${{ }}` instead of `"{{ }}"` + +One really big readability and cause for confusing was the fact that with +`handlebars` and `yaml` was that you always had to wrap your templating strings +in quotes in `yaml` so that it didn't try to parse it as a `json` object and +fail. This was pretty annoying, as it also meant that all things look like +strings. Now that's no longer the case, you can now remove the `""` and take +advantage of writing nice `yaml` files that just work. + +```diff + spec: + steps: + input: + allowedHosts: ['github.com'] +- description: 'This is {{ parameters.name }}' ++ description: This is ${{ parameters.name }} +- repoUrl: '{{ parameters.repoUrl }}' ++ repoUrl: ${{ parameters.repoUrl }} +``` + +## No more `eq` or `not` helper + +These helpers are no longer needed with the more expressive `api` that +`nunjucks` provides. You can simply use the built-in `nunjucks` and `jinja2` +style operators. + +```diff + spec: + steps: + input: +- if: '{{ eq parameters.value "backstage" }}' ++ if: ${{ parameters.value === "backstage" }} + ... + +``` + +And then for the `not` + +```diff + spec: + steps: + input: +- if: '{{ not parameters.value "backstage" }}' ++ if: ${{ parameters.value !== "backstage" }} + ... + +``` + +Much better right? ✨ + +## No more `json` helper + +This helper is no longer needed, as we've added support for complex values and +supporting the additional primitive values now rather than everything being a +`string`. This means that now that you can pass around `parameters` and it +should all work as expected and keep the type that has been declared in the +input schema. + +```diff + spec: + parameters: + test: + type: number + name: Test Number + address: + type: object + required: + - line1 + properties: + line1:🙏 + type: string + name: Line 1 + line2: + type: string + name: Line 2 + + steps: + - id: test step + action: run:something + input: +- address: '{{ json parameters.address }}' ++ address: ${{ parameters.address }} +- number: '{{ parameters.number }}' ++ number: ${{ parameters.number }} # this will now make sure that the type of number is a number now 🙏 + +``` + +## `parseRepoUrl` is now a `filter` + +All calls to `parseRepoUrl` are now a `jinja2` `filter`, which means you'll need +to update the syntax. + +```diff + spec: + steps: + input: +- repoUrl: '{{ parseRepoUrl parameters.repoUrl }}' ++ repoUrl: ${{ parameters.repoUrl | parseRepoUrl }} + ... +``` + +Now we have complex value support here too, expect that this `filter` will go +away in future versions and the `RepoUrlPicker` will return an object so +`parameters.repoUrl` will already be a +`{ host: string; owner: string; repo: string }` 🚀 + +### Summary + +Of course, we're always available on [discord](https://discord.gg/MUpMjP2) if +you're stuck or something's not working as expected. You can also +[raise an issue](https://github.com/backstage/backstage/issues/new/choose) with +feedback or bugs! diff --git a/microsite/sidebars.json b/microsite/sidebars.json index c269f8ac8f..cdbfdccec3 100644 --- a/microsite/sidebars.json +++ b/microsite/sidebars.json @@ -78,8 +78,7 @@ "features/software-templates/builtin-actions", "features/software-templates/writing-custom-actions", "features/software-templates/writing-custom-field-extensions", - "features/software-templates/template-legacy", - "features/software-templates/migrating-from-v1alpha1-to-v1beta2" + "features/software-templates/migrating-from-v1beta2-to-v1beta3" ] }, { From a61204bc4ba1f8cb14d743fb3557b914b6364948 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 29 Sep 2021 10:53:34 +0200 Subject: [PATCH 19/29] chore: remove the link to the legacy docs Signed-off-by: blam --- docs/features/software-catalog/descriptor-format.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/features/software-catalog/descriptor-format.md b/docs/features/software-catalog/descriptor-format.md index 7a9aa099e9..c174b94557 100644 --- a/docs/features/software-catalog/descriptor-format.md +++ b/docs/features/software-catalog/descriptor-format.md @@ -620,9 +620,6 @@ The following describes the following entity kind: | `apiVersion` | `backstage.io/v1beta2` | | `kind` | `Template` | -If you're looking for docs on `v1alpha1` you can find them -[here](../software-templates/legacy.md) - A template definition describes both the parameters that are rendered in the frontend part of the scaffolding wizard, and the steps that are executed when scaffolding that component. From ca9c69f3a88463474abafe91cfdc7848b46235e8 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 30 Sep 2021 17:16:08 +0200 Subject: [PATCH 20/29] chore: fix code review comments Signed-off-by: blam --- .../src/scaffolder/tasks/DefaultWorkflowRunner.ts | 2 +- plugins/scaffolder-backend/src/service/router.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 63ba392dcc..354092ee06 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -14,7 +14,6 @@ * limitations under the License. */ import { ScmIntegrations } from '@backstage/integration'; -import { TemplateActionRegistry } from '..'; import { Task, TaskSpec, @@ -33,6 +32,7 @@ import { PassThrough } from 'stream'; import { isTruthy } from './helper'; import { validate as validateJsonSchema } from 'jsonschema'; import { parseRepoUrl } from '../actions/builtin/publish/util'; +import { TemplateActionRegistry } from '../actions'; type Options = { workingDirectory: string; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index b75a5d0368..8e71dde461 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -200,8 +200,7 @@ export async function createRouter( } const baseUrl = getEntityBaseUrl(template); - // TODO: need to make sure that the TaskSpec is the right format here. - // If it's beta2 use values, beta3 uses parameters to clear that up. + taskSpec = template.apiVersion === 'backstage.io/v1beta2' ? { From 8893feca7306facecee44ca5737619a41442c45f Mon Sep 17 00:00:00 2001 From: blam Date: Fri, 1 Oct 2021 11:21:59 +0200 Subject: [PATCH 21/29] chore: docs updates Signed-off-by: blam --- .../migrating-from-v1beta2-to-v1beta3.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md b/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md index 8ccacfa301..84d76b9fb1 100644 --- a/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md +++ b/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md @@ -36,12 +36,12 @@ to upgrade. ## `${{ }}` instead of `"{{ }}"` -One really big readability and cause for confusing was the fact that with -`handlebars` and `yaml` was that you always had to wrap your templating strings -in quotes in `yaml` so that it didn't try to parse it as a `json` object and -fail. This was pretty annoying, as it also meant that all things look like -strings. Now that's no longer the case, you can now remove the `""` and take -advantage of writing nice `yaml` files that just work. +One really big readability issue and cause for confusion was the fact that with +`handlebars` and `yaml`you always had to wrap your templating strings in quotes +in `yaml` so that it didn't try to parse it as a `json` object and fail. This +was pretty annoying, as it also meant that all things look like strings. Now +that's no longer the case, you can now remove the `""` and take advantage of +writing nice `yaml` files that just work. ```diff spec: @@ -54,7 +54,7 @@ advantage of writing nice `yaml` files that just work. + repoUrl: ${{ parameters.repoUrl }} ``` -## No more `eq` or `not` helper +## No more `eq` or `not` helpers These helpers are no longer needed with the more expressive `api` that `nunjucks` provides. You can simply use the built-in `nunjucks` and `jinja2` @@ -117,7 +117,7 @@ input schema. - address: '{{ json parameters.address }}' + address: ${{ parameters.address }} - number: '{{ parameters.number }}' -+ number: ${{ parameters.number }} # this will now make sure that the type of number is a number now 🙏 ++ number: ${{ parameters.number }} # this will now make sure that the type of number is a number 🙏 ``` From 18404c795b11013613399c31d3aef96cf66cb6aa Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Oct 2021 14:31:53 +0200 Subject: [PATCH 22/29] chore: reset the changes to the catalog-model Signed-off-by: blam Signed-off-by: blam --- packages/catalog-model/api-report.md | 27 ----------------------- packages/catalog-model/src/kinds/index.ts | 2 -- 2 files changed, 29 deletions(-) diff --git a/packages/catalog-model/api-report.md b/packages/catalog-model/api-report.md index 1b6c3029b7..1e7eb8fd52 100644 --- a/packages/catalog-model/api-report.md +++ b/packages/catalog-model/api-report.md @@ -553,33 +553,6 @@ export interface TemplateEntityV1beta2 extends Entity { // @public (undocumented) export const templateEntityV1beta2Validator: KindValidator; -// @public (undocumented) -export interface TemplateEntityV1beta3 extends Entity { - // (undocumented) - apiVersion: 'backstage.io/v1beta3'; - // (undocumented) - kind: 'Template'; - // (undocumented) - spec: { - type: string; - parameters?: JsonObject | JsonObject[]; - steps: Array<{ - id?: string; - name?: string; - action: string; - input?: JsonObject; - if?: string | boolean; - }>; - output?: { - [name: string]: string; - }; - owner?: string; - }; -} - -// @public (undocumented) -export const templateEntityV1beta3Validator: KindValidator; - // @alpha export type UNSTABLE_EntityStatus = { items?: UNSTABLE_EntityStatusItem[]; diff --git a/packages/catalog-model/src/kinds/index.ts b/packages/catalog-model/src/kinds/index.ts index edac9c505a..be9f7a0d6a 100644 --- a/packages/catalog-model/src/kinds/index.ts +++ b/packages/catalog-model/src/kinds/index.ts @@ -52,8 +52,6 @@ export type { } from './SystemEntityV1alpha1'; export { templateEntityV1beta2Validator } from './TemplateEntityV1beta2'; export type { TemplateEntityV1beta2 } from './TemplateEntityV1beta2'; -export { templateEntityV1beta3Validator } from './TemplateEntityV1beta3'; -export type { TemplateEntityV1beta3 } from './TemplateEntityV1beta3'; export type { KindValidator } from './types'; export { userEntityV1alpha1Validator } from './UserEntityV1alpha1'; export type { From 5118da771f7fbfc77a20c41e847db7c9e55f1f94 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Oct 2021 14:34:55 +0200 Subject: [PATCH 23/29] chore: remove the changes for catalog-backend too Signed-off-by: blam --- .../ingestion/processors/BuiltinKindsEntityProcessor.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts b/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts index 18a8ad2a27..bcabc00f32 100644 --- a/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts +++ b/plugins/catalog-backend/src/ingestion/processors/BuiltinKindsEntityProcessor.ts @@ -48,8 +48,6 @@ import { systemEntityV1alpha1Validator, TemplateEntityV1beta2, templateEntityV1beta2Validator, - TemplateEntityV1beta3, - templateEntityV1beta3Validator, UserEntity, userEntityV1alpha1Validator, } from '@backstage/catalog-model'; @@ -63,9 +61,6 @@ export class BuiltinKindsEntityProcessor implements CatalogProcessor { resourceEntityV1alpha1Validator, groupEntityV1alpha1Validator, locationEntityV1alpha1Validator, - templateEntityV1beta3Validator, - - // TODO: remove once beta3 is stable templateEntityV1beta2Validator, userEntityV1alpha1Validator, systemEntityV1alpha1Validator, @@ -139,7 +134,7 @@ export class BuiltinKindsEntityProcessor implements CatalogProcessor { * Emit relations for the Template kind */ if (entity.kind === 'Template') { - const template = entity as TemplateEntityV1beta2 | TemplateEntityV1beta3; + const template = entity as TemplateEntityV1beta2; doEmit( template.spec.owner, { defaultKind: 'Group', defaultNamespace: selfRef.namespace }, From b1d5d584e57d2f5f2551020fe47a03dacdf69288 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Oct 2021 14:43:05 +0200 Subject: [PATCH 24/29] chore: updating the version string Signed-off-by: blam --- .../src/scaffolder/tasks/types.ts | 2 +- plugins/scaffolder-backend/src/service/router.ts | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 4f11d37da6..cd93404165 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -65,7 +65,7 @@ export interface TaskStep { if?: string | boolean; } export interface TaskSpecV1beta3 { - apiVersion: 'backstage.io/v1beta3'; + apiVersion: 'scaffolder.backstage.io/v1beta3'; baseUrl?: string; parameters: JsonObject; steps: TaskStep[]; diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 8e71dde461..f8ba99336f 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -34,16 +34,15 @@ import { } from '@backstage/backend-common'; import { InputError, NotFoundError } from '@backstage/errors'; import { CatalogApi } from '@backstage/catalog-client'; -import { - TemplateEntityV1beta2, - Entity, - TemplateEntityV1beta3, -} from '@backstage/catalog-model'; +import { TemplateEntityV1beta2, Entity } from '@backstage/catalog-model'; +import { TemplateEntityV1beta3 } from '@backstage/plugin-scaffolder-common'; + import { ScmIntegrations } from '@backstage/integration'; import { TemplateAction } from '../scaffolder/actions'; import { createBuiltinActions } from '../scaffolder/actions/builtin/createBuiltinActions'; import { LegacyWorkflowRunner } from '../scaffolder/tasks/LegacyWorkflowRunner'; import { DefaultWorkflowRunner } from '../scaffolder/tasks/DefaultWorkflowRunner'; +import { TaskSpec } from '../scaffolder/tasks/types'; export interface RouterOptions { logger: Logger; @@ -61,7 +60,7 @@ function isSupportedTemplate( ) { return ( entity.apiVersion === 'backstage.io/v1beta2' || - entity.apiVersion === 'backstage.io/v1beta3' + entity.apiVersion === 'scaffolder.backstage.io/v1beta3' ); } @@ -187,7 +186,7 @@ export async function createRouter( token, }); - let taskSpec; + let taskSpec: TaskSpec; if (isSupportedTemplate(template)) { for (const parameters of [template.spec.parameters ?? []].flat()) { From 17d7f5bc57dfb5d57db11927694de2d6dfcb9f25 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Oct 2021 14:51:43 +0200 Subject: [PATCH 25/29] feat: removing the older stuff and updating to work with the new common packages Signed-off-by: blam --- .../src/kinds/TemplateEntityV1beta3.test.ts | 156 ------------------ .../src/kinds/TemplateEntityV1beta3.ts | 43 ----- .../tasks/DefaultWorkflowRunner.test.ts | 26 +-- .../scaffolder/tasks/DefaultWorkflowRunner.ts | 2 +- .../src/scaffolder/tasks/TaskWorker.test.ts | 4 +- .../src/scaffolder/tasks/TaskWorker.ts | 2 +- 6 files changed, 17 insertions(+), 216 deletions(-) delete mode 100644 packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts delete mode 100644 packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts diff --git a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts deleted file mode 100644 index cc275435e8..0000000000 --- a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - TemplateEntityV1beta3, - templateEntityV1beta3Validator as validator, -} from './TemplateEntityV1beta3'; - -describe('templateEntityV1beta3Validator', () => { - let entity: TemplateEntityV1beta3; - - beforeEach(() => { - entity = { - apiVersion: 'backstage.io/v1beta3', - kind: 'Template', - metadata: { - name: 'test', - }, - spec: { - type: 'website', - parameters: { - required: ['storePath', 'owner'], - properties: { - owner: { - type: 'string', - title: 'Owner', - description: 'Who is going to own this component', - }, - }, - }, - steps: [ - { - id: 'fetch', - name: 'Fetch', - action: 'fetch:plan', - input: { - url: './template', - }, - if: '${{ parameters.owner }}', - }, - ], - output: { - fetchUrl: '${{ steps.fetch.output.targetUrl }}', - }, - owner: 'team-b@example.com', - }, - }; - }); - - it('happy path: accepts valid data', async () => { - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('ignores unknown apiVersion', async () => { - (entity as any).apiVersion = 'backstage.io/v1beta0'; - await expect(validator.check(entity)).resolves.toBe(false); - }); - - it('ignores unknown kind', async () => { - (entity as any).kind = 'Wizard'; - await expect(validator.check(entity)).resolves.toBe(false); - }); - - it('rejects missing type', async () => { - delete (entity as any).spec.type; - await expect(validator.check(entity)).rejects.toThrow(/type/); - }); - - it('accepts any other type', async () => { - (entity as any).spec.type = 'hallo'; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('accepts missing parameters', async () => { - delete (entity as any).spec.parameters; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('accepts missing outputs', async () => { - delete (entity as any).spec.outputs; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('rejects empty type', async () => { - (entity as any).spec.type = ''; - await expect(validator.check(entity)).rejects.toThrow(/type/); - }); - - it('rejects missing steps', async () => { - delete (entity as any).spec.steps; - await expect(validator.check(entity)).rejects.toThrow(/steps/); - }); - - it('accepts step with missing id', async () => { - delete (entity as any).spec.steps[0].id; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('accepts step with missing name', async () => { - delete (entity as any).spec.steps[0].name; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('rejects step with missing action', async () => { - delete (entity as any).spec.steps[0].action; - await expect(validator.check(entity)).rejects.toThrow(/action/); - }); - - it('accepts missing owner', async () => { - delete (entity as any).spec.owner; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('rejects empty owner', async () => { - (entity as any).spec.owner = ''; - await expect(validator.check(entity)).rejects.toThrow(/owner/); - }); - - it('rejects wrong type owner', async () => { - (entity as any).spec.owner = 5; - await expect(validator.check(entity)).rejects.toThrow(/owner/); - }); - - it('accepts missing if', async () => { - delete (entity as any).spec.steps[0].if; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('accepts boolean in if', async () => { - (entity as any).spec.steps[0].if = true; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('accepts empty if', async () => { - (entity as any).spec.steps[0].if = ''; - await expect(validator.check(entity)).resolves.toBe(true); - }); - - it('rejects wrong type if', async () => { - (entity as any).spec.steps[0].if = 5; - await expect(validator.check(entity)).rejects.toThrow(/if/); - }); -}); diff --git a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts b/packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts deleted file mode 100644 index 2da9a0b6b8..0000000000 --- a/packages/catalog-model/src/kinds/TemplateEntityV1beta3.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { JsonObject } from '@backstage/config'; -import type { Entity } from '../entity/Entity'; -import schema from '../schema/kinds/Template.v1beta3.schema.json'; -import { ajvCompiledJsonSchemaValidator } from './util'; - -/** @public */ -export interface TemplateEntityV1beta3 extends Entity { - apiVersion: 'backstage.io/v1beta3'; - kind: 'Template'; - spec: { - type: string; - parameters?: JsonObject | JsonObject[]; - steps: Array<{ - id?: string; - name?: string; - action: string; - input?: JsonObject; - if?: string | boolean; - }>; - output?: { [name: string]: string }; - owner?: string; - }; -} - -/** @public */ -export const templateEntityV1beta3Validator = - ajvCompiledJsonSchemaValidator(schema); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts index 542cbb4b2f..3ecdd6dcd8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.test.ts @@ -91,7 +91,7 @@ describe('DefaultWorkflowRunner', () => { it('should throw an error if the action does not exist', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', parameters: {}, output: {}, steps: [{ id: 'test', name: 'name', action: 'does-not-exist' }], @@ -105,7 +105,7 @@ describe('DefaultWorkflowRunner', () => { describe('validation', () => { it('should throw an error if the action has a schema and the input does not match', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', parameters: {}, output: {}, steps: [{ id: 'test', name: 'name', action: 'jest-validated-action' }], @@ -118,7 +118,7 @@ describe('DefaultWorkflowRunner', () => { it('should run the action when the validation passes', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', parameters: {}, output: {}, steps: [ @@ -140,7 +140,7 @@ describe('DefaultWorkflowRunner', () => { describe('conditionals', () => { it('should execute steps conditionally', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', name: 'test', action: 'output-action' }, { @@ -163,7 +163,7 @@ describe('DefaultWorkflowRunner', () => { it('should skips steps conditionally', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', name: 'test', action: 'output-action' }, { @@ -186,7 +186,7 @@ describe('DefaultWorkflowRunner', () => { it('should skips steps using the negating equals operator', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', name: 'test', action: 'output-action' }, { @@ -211,7 +211,7 @@ describe('DefaultWorkflowRunner', () => { describe('templating', () => { it('should template the input to an action', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', @@ -237,7 +237,7 @@ describe('DefaultWorkflowRunner', () => { it('should keep the original types for the input and not parse things that arent meant to be parsed', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', @@ -265,7 +265,7 @@ describe('DefaultWorkflowRunner', () => { it('should template complex values into the action', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', @@ -291,7 +291,7 @@ describe('DefaultWorkflowRunner', () => { it('supports really complex structures', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', @@ -320,7 +320,7 @@ describe('DefaultWorkflowRunner', () => { it('supports numbers as first class too', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', @@ -349,7 +349,7 @@ describe('DefaultWorkflowRunner', () => { it('should template the output from simple actions', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', @@ -373,7 +373,7 @@ describe('DefaultWorkflowRunner', () => { describe('filters', () => { it('provides the parseRepoUrl filter', async () => { const task = createMockTaskWithSpec({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [ { id: 'test', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 354092ee06..4a94b9bf73 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -49,7 +49,7 @@ type TemplateContext = { }; const isValidTaskSpec = (taskSpec: TaskSpec): taskSpec is TaskSpecV1beta3 => { - return taskSpec.apiVersion === 'backstage.io/v1beta3'; + return taskSpec.apiVersion === 'scaffolder.backstage.io/v1beta3'; }; const createStepLogger = ({ task, step }: { task: Task; step: TaskStep }) => { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 035398d448..87a0229b5d 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -92,7 +92,7 @@ describe('TaskWorker', () => { }); await broker.dispatch({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], output: { result: '{{ steps.test.output.testOutput }}', @@ -121,7 +121,7 @@ describe('TaskWorker', () => { }); const { taskId } = await broker.dispatch({ - apiVersion: 'backstage.io/v1beta3', + apiVersion: 'scaffolder.backstage.io/v1beta3', steps: [{ id: 'test', name: 'test', action: 'not-found-action' }], output: { result: '{{ steps.test.output.testOutput }}', diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 5b69ecc6df..7122c2fc3e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -38,7 +38,7 @@ export class TaskWorker { async runOneTask(task: Task) { try { const { output } = - task.spec.apiVersion === 'backstage.io/v1beta3' + task.spec.apiVersion === 'scaffolder.backstage.io/v1beta3' ? await this.options.runners.workflowRunner.execute(task) : await this.options.runners.legacyWorkflowRunner.execute(task); From ac29021b53c5730404c9aba291214683ed799151 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Oct 2021 14:55:16 +0200 Subject: [PATCH 26/29] chore: re-running things Signed-off-by: blam --- .changeset/polite-timers-watch.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.changeset/polite-timers-watch.md b/.changeset/polite-timers-watch.md index d8f2ea80fb..dccae47437 100644 --- a/.changeset/polite-timers-watch.md +++ b/.changeset/polite-timers-watch.md @@ -1,7 +1,5 @@ --- -'@backstage/catalog-model': patch -'@backstage/plugin-catalog-backend': patch '@backstage/plugin-scaffolder-backend': patch --- -Introduce the new `backstage.io/v1beta3` template kind with nunjucks support 🥋 +Introduce the new `scaffolder.backstage.io/v1beta3` template kind with nunjucks support 🥋 From ca16be0611964b9e5e74c43b7f030929f13812e6 Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 11 Oct 2021 15:23:29 +0200 Subject: [PATCH 27/29] chore: removing the older json schema not required anymore Signed-off-by: blam --- .../schema/kinds/Template.v1beta3.schema.json | 186 ------------------ 1 file changed, 186 deletions(-) delete mode 100644 packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json diff --git a/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json b/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json deleted file mode 100644 index e87a436d34..0000000000 --- a/packages/catalog-model/src/schema/kinds/Template.v1beta3.schema.json +++ /dev/null @@ -1,186 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema", - "$id": "TemplateV1beta3", - "description": "A Template describes a scaffolding task for use with the Scaffolder. It describes the required parameters as well as a series of steps that will be taken to execute the scaffolding task.", - "examples": [ - { - "apiVersion": "backstage.io/v1beta3", - "kind": "Template", - "metadata": { - "name": "react-ssr-template", - "title": "React SSR Template", - "description": "Next.js application skeleton for creating isomorphic web applications.", - "tags": ["recommended", "react"] - }, - "spec": { - "owner": "artist-relations-team", - "parameters": { - "required": ["name", "description", "repoUrl"], - "properties": { - "name": { - "title": "Name", - "type": "string", - "description": "Unique name of the component" - }, - "description": { - "title": "Description", - "type": "string", - "description": "Description of the component" - }, - "repoUrl": { - "title": "Pick a repository", - "type": "string", - "ui:field": "RepoUrlPicker" - } - } - }, - "steps": [ - { - "id": "fetch", - "name": "Fetch", - "action": "fetch:plain", - "parameters": { - "url": "./template" - } - }, - { - "id": "publish", - "name": "Publish to GitHub", - "action": "publish:github", - "parameters": { - "repoUrl": "${{ parameters.repoUrl }}" - }, - "if": "${{ parameters.repoUrl }}" - } - ], - "output": { - "catalogInfoUrl": "${{ steps.publish.output.catalogInfoUrl }}" - } - } - } - ], - "allOf": [ - { - "$ref": "Entity" - }, - { - "type": "object", - "required": ["spec"], - "properties": { - "apiVersion": { - "enum": ["backstage.io/v1beta3"] - }, - "kind": { - "enum": ["Template"] - }, - "spec": { - "type": "object", - "required": ["type", "steps"], - "properties": { - "type": { - "type": "string", - "description": "The type of component created by the template. The software catalog accepts any type value, but an organization should take great care to establish a proper taxonomy for these. Tools including Backstage itself may read this field and behave differently depending on its value. For example, a website type component may present tooling in the Backstage interface that is specific to just websites.", - "examples": ["service", "website", "library"], - "minLength": 1 - }, - "parameters": { - "oneOf": [ - { - "type": "object", - "description": "The JSONSchema describing the inputs for the template." - }, - { - "type": "array", - "description": "A list of separate forms to collect parameters.", - "items": { - "type": "object", - "description": "The JSONSchema describing the inputs for the template." - } - } - ] - }, - "steps": { - "type": "array", - "description": "A list of steps to execute.", - "items": { - "type": "object", - "description": "A description of the step to execute.", - "required": ["action"], - "properties": { - "id": { - "type": "string", - "description": "The ID of the step, which can be used to refer to its outputs." - }, - "name": { - "type": "string", - "description": "The name of the step, which will be displayed in the UI during the scaffolding process." - }, - "action": { - "type": "string", - "description": "The name of the action to execute." - }, - "input": { - "type": "object", - "description": "A templated object describing the inputs to the action." - }, - "if": { - "type": ["string", "boolean"], - "description": "A templated condition that skips the step when evaluated to false. If the condition is true or not defined, the step is executed. The condition is true, if the input is not `false`, `undefined`, `null`, `\"\"`, `0`, or `[]`." - } - } - } - }, - "output": { - "type": "object", - "description": "A templated object describing the outputs of the scaffolding task.", - "properties": { - "links": { - "type": "array", - "description": "A list of external hyperlinks, typically pointing to resources created or updated by the template", - "items": { - "type": "object", - "required": [], - "properties": { - "url": { - "type": "string", - "description": "A url in a standard uri format.", - "examples": ["https://github.com/my-org/my-new-repo"], - "minLength": 1 - }, - "entityRef": { - "type": "string", - "description": "An entity reference to an entity in the catalog.", - "examples": ["Component:default/my-app"], - "minLength": 1 - }, - "title": { - "type": "string", - "description": "A user friendly display name for the link.", - "examples": ["View new repo"], - "minLength": 1 - }, - "icon": { - "type": "string", - "description": "A key representing a visual icon to be displayed in the UI.", - "examples": ["dashboard"], - "minLength": 1 - } - } - } - } - }, - "additionalProperties": { - "type": "string" - } - }, - "owner": { - "type": "string", - "description": "The user (or group) owner of the template", - "minLength": 1 - } - } - } - } - } - ] -} From 7fe6c0bb70232818e74d5bcd1845ab2627cbe6a5 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 12 Oct 2021 16:48:56 +0200 Subject: [PATCH 28/29] chore: reworking the docs a little bit Signed-off-by: blam --- .../migrating-from-v1beta2-to-v1beta3.md | 28 ++++++++++++------- .../fixtures/test-v1beta3/template.yaml | 4 ++- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md b/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md index 84d76b9fb1..280eab083d 100644 --- a/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md +++ b/docs/features/software-templates/migrating-from-v1beta2-to-v1beta3.md @@ -12,32 +12,43 @@ Well then, here we are! 🚀 Backstage has had many forms of templating languages throughout different plugins and different systems. We've had `cookiecutter` syntax in templates, and we also had `handlebars` templating in the `kind: Template`. Then we wanted to -remove the additional dependency on `cookiecutter` for `Software Templates` out -of the box, so we introduced `nunjucks` as an alternative in `fetch:template` +remove the additional dependency on `cookiecutter` for Software Templates out of +the box, so we introduced `nunjucks` as an alternative in `fetch:template` action which is based on the `jinja2` syntax so they're pretty similar. In an effort to reduce confusion and unify on to one templating language, we're officially deprecating support for `handlebars` templating in the -`kind: Template` entities with version `backstage.io/v1beta3` and moving to -using `nunjucks` instead. +`kind: Template` entities with `apiVersion` `scaffolder.backstage.io/v1beta3` +and moving to using `nunjucks` instead. This provides us a lot of built in `filters` (`handlebars` helpers), that as Template authors will give you much more flexibility out of the box, and also -open up sharing of filters in the `entity` and the actual `skeleton` too, and +open up sharing of filters in the Entity and the actual `skeleton` too, and removing the slight differences between the two languages. We've also removed a lot of the built in helpers that we shipped with `handlebars`, as they're now supported as first class citizens by either -`nunjucks` or the new `scaffolder` when using `backstage.io/v1beta3` +`nunjucks` or the new `scaffolder` when using `scaffolder.backstage.io/v1beta3` `apiVersion` The migration path is pretty simple, and we've removed some of the pain points from writing the `handlebars` templates too. Let's go through what's new and how to upgrade. +## `backstage.io/v1beta2` -> `scaffolder.backstage.io/v1beta3` + +The most important change is that you'll need to switch over the `apiVersion` in +your templates to the new one. + +```diff + kind: Template +- apiVersion: backstage.io/v1beta2 ++ apiVersion: scaffolder.backstage.io/v1beta3 +``` + ## `${{ }}` instead of `"{{ }}"` One really big readability issue and cause for confusion was the fact that with -`handlebars` and `yaml`you always had to wrap your templating strings in quotes +`handlebars` and `yaml` you always had to wrap your templating strings in quotes in `yaml` so that it didn't try to parse it as a `json` object and fail. This was pretty annoying, as it also meant that all things look like strings. Now that's no longer the case, you can now remove the `""` and take advantage of @@ -67,7 +78,6 @@ style operators. - if: '{{ eq parameters.value "backstage" }}' + if: ${{ parameters.value === "backstage" }} ... - ``` And then for the `not` @@ -79,7 +89,6 @@ And then for the `not` - if: '{{ not parameters.value "backstage" }}' + if: ${{ parameters.value !== "backstage" }} ... - ``` Much better right? ✨ @@ -118,7 +127,6 @@ input schema. + address: ${{ parameters.address }} - number: '{{ parameters.number }}' + number: ${{ parameters.number }} # this will now make sure that the type of number is a number 🙏 - ``` ## `parseRepoUrl` is now a `filter` diff --git a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml index 01bd5c2e4f..10d4a39ff7 100644 --- a/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml +++ b/plugins/scaffolder-backend/fixtures/test-v1beta3/template.yaml @@ -1,4 +1,4 @@ -apiVersion: backstage.io/v1beta3 +apiVersion: scaffolder.backstage.io/v1beta3 kind: Template metadata: name: test-v1beta3 @@ -6,6 +6,7 @@ metadata: description: Test V1 Beta 3 Demo Templates spec: type: website + owner: team-a parameters: - name: Enter some stuff description: Enter some stuff @@ -16,6 +17,7 @@ spec: inputObject: type: object title: object input test + description: a little nested thing never hurt anyone right? properties: first: type: string From f71fda7600dbaba982b0aeda6b7ce739c0e5ca2f Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 14 Oct 2021 10:39:00 +0200 Subject: [PATCH 29/29] chore: removing double log Lines Signed-off-by: blam --- .../src/scaffolder/tasks/DefaultWorkflowRunner.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts index 4a94b9bf73..2ac309a1d4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DefaultWorkflowRunner.ts @@ -222,11 +222,6 @@ export class DefaultWorkflowRunner implements WorkflowRunner { const tmpDirs = new Array(); const stepOutput: { [outputName: string]: JsonValue } = {}; - await task.emitLog(`Beginning step ${step.name}`, { - stepId: step.id, - status: 'processing', - }); - await action.handler({ baseUrl: task.spec.baseUrl, input,