Merge pull request #9038 from backstage/blam/scaffolder-secrets

scaffolder: Support supplying secrets at Task Creation

.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.

plugins/scaffolder-backend/api-report.md

@@ -43,6 +43,7 @@ export type ActionContext<Input extends InputBase> = {
   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 = Record<string, string> & {
+  token?: string;
+  backstageToken?: string;
 };
 
 export { TaskSpec };

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) {

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<JsonObject>;
 type PartialJsonValue = PartialJsonObject | JsonValue | undefined;
@@ -35,8 +35,10 @@ export type ActionContext<Input extends InputBase> = {
 
   /**
    * 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;

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,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: null,
         });
 
       if (updateCount < 1) {
@@ -235,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}`,

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(

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({

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,

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);

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 = Record<string, string> & {
+  /** @deprecated Use `backstageToken` instead */
+  token?: string;
+  backstageToken?: string;
 };
 
 /**

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,
       });
 

plugins/scaffolder/api-report.md

@@ -192,7 +192,11 @@ export interface ScaffolderApi {
   //
   // (undocumented)
   listActions(): Promise<ListActionsResponse>;
-  scaffold(templateName: string, values: Record<string, any>): Promise<string>;
+  scaffold(
+    templateName: string,
+    values: Record<string, any>,
+    secrets?: Record<string, string>,
+  ): Promise<string>;
   // 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<TemplateParameterSchema>;
   // (undocumented)
   listActions(): Promise<ListActionsResponse>;
-  scaffold(templateName: string, values: Record<string, any>): Promise<string>;
+  scaffold(
+    templateName: string,
+    values: Record<string, any>,
+    secrets?: Record<string, string>,
+  ): Promise<string>;
   // (undocumented)
   streamLogs(opts: { taskId: string; after?: number }): Observable<LogEvent>;
 }

plugins/scaffolder/src/api.ts

@@ -69,8 +69,13 @@ 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<string, any>): Promise<string>;
+  scaffold(
+    templateName: string,
+    values: Record<string, any>,
+    secrets?: Record<string, string>,
+  ): Promise<string>;
 
   getTask(taskId: string): Promise<ScaffolderTask>;
 
@@ -151,10 +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<string, any>,
+    secrets: Record<string, string> = {},
   ): Promise<string> {
     const { token } = await this.identityApi.getCredentials();
     const url = `${await this.discoveryApi.getBaseUrl('scaffolder')}/v2/tasks`;
@@ -164,7 +171,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,
+      }),
     });
 
     if (response.status !== 201) {