From 8100abfde283003bfe1c6a32e4a5c3d800ab24cd Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 16:43:12 +0100 Subject: [PATCH 01/10] feat: adding the option to pass secrets from the frontend from the ScaffolderApiClient Signed-off-by: blam --- plugins/scaffolder/src/api.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index 70ed803ef7..767d3ac736 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -70,7 +70,11 @@ export interface ScaffolderApi { * @param templateName - Name of the Template entity for the scaffolder to use. New project is going to be created out of this template. * @param values - Parameters for the template, e.g. name, description */ - scaffold(templateName: string, values: Record): Promise; + scaffold( + templateName: string, + values: Record, + secrets?: JsonObject, + ): Promise; getTask(taskId: string): Promise; @@ -155,6 +159,7 @@ export class ScaffolderClient implements ScaffolderApi { async scaffold( templateName: string, values: Record, + secrets: JsonObject = {}, ): Promise { const { token } = await this.identityApi.getCredentials(); const url = `${await this.discoveryApi.getBaseUrl('scaffolder')}/v2/tasks`; @@ -164,7 +169,11 @@ export class ScaffolderClient implements ScaffolderApi { 'Content-Type': 'application/json', ...(token && { Authorization: `Bearer ${token}` }), }, - body: JSON.stringify({ templateName, values: { ...values } }), + body: JSON.stringify({ + templateName, + values: { ...values }, + secrets: secrets, + }), }); if (response.status !== 201) { From c3a28a46fcd494b308258b0aba99a076945b8c23 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 16:43:45 +0100 Subject: [PATCH 02/10] chore: deprecating the token field and renaming to backstageToken as token is far too generic Signed-off-by: blam --- plugins/scaffolder-backend/src/scaffolder/tasks/types.ts | 6 ++++-- plugins/scaffolder-backend/src/service/router.ts | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 1a69f28fb6..7a2afcbaed 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -89,8 +89,10 @@ export type SerializedTaskEvent = { * * @public */ -export type TaskSecrets = { - token: string | undefined; +export type TaskSecrets = JsonObject & { + /** @deprecated Use `backstageToken` instead */ + token?: string; + backstageToken?: string; }; /** diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 9d26857ab9..f4f7398c8a 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -232,6 +232,9 @@ export async function createRouter( } const result = await taskBroker.dispatch(taskSpec, { + ...req.body.secrets, + backstageToken: token, + // This is deprecated, but we need to support it for now if people are running their own task broker. token: token, }); From f999b4063a137f014dc177dc34ee45d7bd789e65 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 17:11:45 +0100 Subject: [PATCH 03/10] chore: deprecating the token that's passed through to the context and using ctx.secrets.backstageToken instead Signed-off-by: blam --- plugins/scaffolder-backend/src/scaffolder/actions/types.ts | 4 ++++ ...orkflowRunner.test.ts => HandlebarsWorkflowRunner.test.ts} | 0 .../src/scaffolder/tasks/HandlebarsWorkflowRunner.ts | 2 ++ .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 2 ++ 4 files changed, 8 insertions(+) rename plugins/scaffolder-backend/src/scaffolder/tasks/{LegacyWorkflowRunner.test.ts => HandlebarsWorkflowRunner.test.ts} (100%) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts index 52149be724..d91af8cd89 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts @@ -35,8 +35,12 @@ export type ActionContext = { /** * User token forwarded from initial request, for use in subsequent api requests + * @deprecated use `secrets.backstageToken` instead */ token?: string | undefined; + + secrets?: TaskSecrets; + workspacePath: string; input: Input; output(name: string, value: JsonValue): void; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.test.ts similarity index 100% rename from plugins/scaffolder-backend/src/scaffolder/tasks/LegacyWorkflowRunner.test.ts rename to plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts index 5582b53023..91c82edebb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/HandlebarsWorkflowRunner.ts @@ -236,7 +236,9 @@ export class HandlebarsWorkflowRunner implements WorkflowRunner { logger: taskLogger, logStream: stream, input, + // this token is deprecated, and will be removed in favour of secrets.backstageToken instead. token: task.secrets?.token, + secrets: task.secrets ?? {}, workspacePath, async createTemporaryDirectory() { const tmpDir = await fs.mkdtemp( diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 71d53cc7ec..46f87c1d00 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -257,7 +257,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { await action.handler({ baseUrl: task.spec.baseUrl, input, + // this token is deprecated, and will be removed in favour of secrets.backstageToken instead. token: task.secrets?.token, + secrets: task.secrets ?? {}, logger: taskLogger, logStream: streamLogger, workspacePath, From d19c88a9e480ad055aeab9e55986ae541d53b55a Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 17:13:59 +0100 Subject: [PATCH 04/10] chore: moving the catalog register to use `backstageToken` instead from the secrets Signed-off-by: blam --- .../src/scaffolder/actions/builtin/catalog/register.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts index 249939b0e6..65d0e1b1e7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts @@ -110,7 +110,9 @@ export function createCatalogRegisterAction(options: { type: 'url', target: catalogInfoUrl, }, - ctx.token ? { token: ctx.token } : {}, + ctx.secrets?.backstageToken + ? { token: ctx.secrets.backstageToken } + : {}, ); try { @@ -120,7 +122,9 @@ export function createCatalogRegisterAction(options: { type: 'url', target: catalogInfoUrl, }, - ctx.token ? { token: ctx.token } : {}, + ctx.secrets?.backstageToken + ? { token: ctx.secrets.backstageToken } + : {}, ); if (result.entities.length > 0) { From 128a489ba0866f1d6fa7475c089f233a708b93d3 Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 17:18:30 +0100 Subject: [PATCH 05/10] chore: delete the secrets when claiming the task so we don't store anything locally Signed-off-by: blam --- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 813d8d0f5b..369da5662e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -139,6 +139,8 @@ export class DatabaseTaskStore implements TaskStore { .update({ status: 'processing', last_heartbeat_at: this.db.fn.now(), + // remove the secrets when moving moving to processing state + secrets: undefined, }); if (updateCount < 1) { From 2d4ef2b78e1dd01d40c2868d45bf9ce6665fc4bb Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 18:13:32 +0100 Subject: [PATCH 06/10] chore: secrets are removed when claiming the task Signed-off-by: blam --- .../src/scaffolder/tasks/DatabaseTaskStore.ts | 8 +++--- .../tasks/NunjucksWorkflowRunner.test.ts | 27 +++++++++++++++++++ .../tasks/StorageTaskBroker.test.ts | 17 +++--------- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts index 369da5662e..dcda3b921e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/DatabaseTaskStore.ts @@ -43,7 +43,7 @@ export type RawDbTaskRow = { status: Status; last_heartbeat_at?: string; created_at: string; - secrets?: string; + secrets?: string | null; }; export type RawDbTaskEventRow = { @@ -139,8 +139,8 @@ export class DatabaseTaskStore implements TaskStore { .update({ status: 'processing', last_heartbeat_at: this.db.fn.now(), - // remove the secrets when moving moving to processing state - secrets: undefined, + // remove the secrets when moving moving to processing state. + secrets: null, }); if (updateCount < 1) { @@ -237,8 +237,8 @@ export class DatabaseTaskStore implements TaskStore { }) .update({ status, - secrets: null as any, }); + if (updateCount !== 1) { throw new ConflictError( `Failed to update status to '${status}' for taskId ${taskId}`, diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index aace2cb437..161d5b865b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -472,6 +472,33 @@ describe('DefaultWorkflowRunner', () => { }); }); + describe('secrets', () => { + it('should pass through the secrets to the context', async () => { + const task = createMockTaskWithSpec( + { + apiVersion: 'scaffolder.backstage.io/v1beta3', + steps: [ + { + id: 'test', + name: 'name', + action: 'jest-mock-action', + input: {}, + }, + ], + output: {}, + parameters: {}, + }, + { foo: 'bar' }, + ); + + await runner.execute(task); + + expect(fakeActionHandler).toHaveBeenCalledWith( + expect.objectContaining({ secrets: { foo: 'bar' } }), + ); + }); + }); + describe('filters', () => { it('provides the parseRepoUrl filter', async () => { const task = createMockTaskWithSpec({ diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts index 76fb8e00f0..130f6ced13 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/StorageTaskBroker.test.ts @@ -94,13 +94,12 @@ describe('StorageTaskBroker', () => { expect(taskRow.status).toBe('completed'); }, 10000); - it('should remove secrets after completing a task', async () => { + it('should remove secrets after picking up a task', async () => { const broker = new StorageTaskBroker(storage, logger); const dispatchResult = await broker.dispatch({} as TaskSpec, fakeSecrets); - const task = await broker.claim(); - await task.complete('completed'); + await broker.claim(); + const taskRow = await storage.getTask(dispatchResult.taskId); - expect(taskRow.status).toBe('completed'); expect(taskRow.secrets).toBeUndefined(); }, 10000); @@ -113,16 +112,6 @@ describe('StorageTaskBroker', () => { expect(taskRow.status).toBe('failed'); }); - it('should remove secrets after failing a task', async () => { - const broker = new StorageTaskBroker(storage, logger); - const dispatchResult = await broker.dispatch({} as TaskSpec, fakeSecrets); - const task = await broker.claim(); - await task.complete('failed'); - const taskRow = await storage.getTask(dispatchResult.taskId); - expect(taskRow.status).toBe('failed'); - expect(taskRow.secrets).toBeUndefined(); - }); - it('multiple brokers should be able to observe a single task', async () => { const broker1 = new StorageTaskBroker(storage, logger); const broker2 = new StorageTaskBroker(storage, logger); From 8181b34c892edf89582439a71ff28ea6d1179aaf Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 18:20:19 +0100 Subject: [PATCH 07/10] chore: set the secrets type as TaskSecrets Signed-off-by: blam --- plugins/scaffolder-backend/src/scaffolder/actions/types.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts index d91af8cd89..4b842e8c5c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts @@ -18,7 +18,7 @@ import { Logger } from 'winston'; import { Writable } from 'stream'; import { JsonValue, JsonObject } from '@backstage/types'; import { Schema } from 'jsonschema'; -import { TemplateMetadata } from '../tasks/types'; +import { TaskSecrets, TemplateMetadata } from '../tasks/types'; type PartialJsonObject = Partial; type PartialJsonValue = PartialJsonObject | JsonValue | undefined; @@ -38,9 +38,7 @@ export type ActionContext = { * @deprecated use `secrets.backstageToken` instead */ token?: string | undefined; - secrets?: TaskSecrets; - workspacePath: string; input: Input; output(name: string, value: JsonValue): void; From b05d3032260a21d28a555b24c01450b97fcc1c4f Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 18:20:54 +0100 Subject: [PATCH 08/10] chore: added changeset Signed-off-by: blam --- .changeset/silver-ties-marry.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/silver-ties-marry.md diff --git a/.changeset/silver-ties-marry.md b/.changeset/silver-ties-marry.md new file mode 100644 index 0000000000..2a3b01f2fc --- /dev/null +++ b/.changeset/silver-ties-marry.md @@ -0,0 +1,10 @@ +--- +'@backstage/plugin-scaffolder': patch +'@backstage/plugin-scaffolder-backend': patch +--- + +Added the ability to support supplying secrets when creating tasks in the `scaffolder-backend`. + +**deprecation**: Deprecated `ctx.token` from actions in the `scaffolder-backend`. Please move to using `ctx.secrets.backstageToken` instead. + +**deprecation**: Deprecated `task.token` in `TaskSpec` in the `scaffolder-backend`. Please move to using `task.secrets.backstageToken` instead. From 783c435e461dffbe7fc08f660eddfcf7f501655c Mon Sep 17 00:00:00 2001 From: blam Date: Wed, 19 Jan 2022 18:38:10 +0100 Subject: [PATCH 09/10] api-docs: regenerate reports for scaffolder and changes Signed-off-by: blam --- plugins/scaffolder-backend/api-report.md | 6 ++++-- plugins/scaffolder/api-report.md | 12 ++++++++++-- plugins/scaffolder/src/api.ts | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index ee84e06ae9..ca6d6ac38e 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -43,6 +43,7 @@ export type ActionContext = { logger: Logger_2; logStream: Writable; token?: string | undefined; + secrets?: TaskSecrets; workspacePath: string; input: Input; output(name: string, value: JsonValue): void; @@ -439,8 +440,9 @@ export class TaskManager implements TaskContext { } // @public -export type TaskSecrets = { - token: string | undefined; +export type TaskSecrets = JsonObject & { + token?: string; + backstageToken?: string; }; export { TaskSpec }; diff --git a/plugins/scaffolder/api-report.md b/plugins/scaffolder/api-report.md index 8443de563e..7145df8097 100644 --- a/plugins/scaffolder/api-report.md +++ b/plugins/scaffolder/api-report.md @@ -192,7 +192,11 @@ export interface ScaffolderApi { // // (undocumented) listActions(): Promise; - scaffold(templateName: string, values: Record): Promise; + scaffold( + templateName: string, + values: Record, + secrets?: JsonObject, + ): Promise; // Warning: (ae-forgotten-export) The symbol "LogEvent" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -236,7 +240,11 @@ export class ScaffolderClient implements ScaffolderApi { ): Promise; // (undocumented) listActions(): Promise; - scaffold(templateName: string, values: Record): Promise; + scaffold( + templateName: string, + values: Record, + secrets?: JsonObject, + ): Promise; // (undocumented) streamLogs(opts: { taskId: string; after?: number }): Observable; } diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index 767d3ac736..d93cdb4924 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -172,7 +172,7 @@ export class ScaffolderClient implements ScaffolderApi { body: JSON.stringify({ templateName, values: { ...values }, - secrets: secrets, + secrets, }), }); From 7e76e7dc123975781f39d4a19e09a8dd665b9c65 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 20 Jan 2022 09:02:46 +0100 Subject: [PATCH 10/10] chore: adjusting the types a little bit and regenerating api-docs Signed-off-by: blam --- plugins/scaffolder-backend/api-report.md | 2 +- plugins/scaffolder-backend/src/scaffolder/tasks/types.ts | 2 +- plugins/scaffolder/api-report.md | 4 ++-- plugins/scaffolder/src/api.ts | 6 ++++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index ca6d6ac38e..12b29cae39 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -440,7 +440,7 @@ export class TaskManager implements TaskContext { } // @public -export type TaskSecrets = JsonObject & { +export type TaskSecrets = Record & { token?: string; backstageToken?: string; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts index 7a2afcbaed..8caee87f57 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/types.ts @@ -89,7 +89,7 @@ export type SerializedTaskEvent = { * * @public */ -export type TaskSecrets = JsonObject & { +export type TaskSecrets = Record & { /** @deprecated Use `backstageToken` instead */ token?: string; backstageToken?: string; diff --git a/plugins/scaffolder/api-report.md b/plugins/scaffolder/api-report.md index 7145df8097..d2bbb2a5e1 100644 --- a/plugins/scaffolder/api-report.md +++ b/plugins/scaffolder/api-report.md @@ -195,7 +195,7 @@ export interface ScaffolderApi { scaffold( templateName: string, values: Record, - secrets?: JsonObject, + secrets?: Record, ): Promise; // Warning: (ae-forgotten-export) The symbol "LogEvent" needs to be exported by the entry point index.d.ts // @@ -243,7 +243,7 @@ export class ScaffolderClient implements ScaffolderApi { scaffold( templateName: string, values: Record, - secrets?: JsonObject, + secrets?: Record, ): Promise; // (undocumented) streamLogs(opts: { taskId: string; after?: number }): Observable; diff --git a/plugins/scaffolder/src/api.ts b/plugins/scaffolder/src/api.ts index d93cdb4924..7f096a02f7 100644 --- a/plugins/scaffolder/src/api.ts +++ b/plugins/scaffolder/src/api.ts @@ -69,11 +69,12 @@ export interface ScaffolderApi { * * @param templateName - Name of the Template entity for the scaffolder to use. New project is going to be created out of this template. * @param values - Parameters for the template, e.g. name, description + * @param secrets - Optional secrets to pass to as the secrets parameter to the template. */ scaffold( templateName: string, values: Record, - secrets?: JsonObject, + secrets?: Record, ): Promise; getTask(taskId: string): Promise; @@ -155,11 +156,12 @@ export class ScaffolderClient implements ScaffolderApi { * * @param templateName - Template name for the scaffolder to use. New project is going to be created out of this template. * @param values - Parameters for the template, e.g. name, description + * @param secrets - Optional secrets to pass to as the secrets parameter to the template. */ async scaffold( templateName: string, values: Record, - secrets: JsonObject = {}, + secrets: Record = {}, ): Promise { const { token } = await this.identityApi.getCredentials(); const url = `${await this.discoveryApi.getBaseUrl('scaffolder')}/v2/tasks`;