From 20ba3205f0ff2578c0da484963cc7af238aed155 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 25 Feb 2021 01:19:42 +0100 Subject: [PATCH] scaffolder-backend: add action parameter type and schema with validation Signed-off-by: Johan Haals --- .../actions/TemplateActionRegistry.ts | 10 ++-- .../actions/builtin/catalog/register.ts | 54 ++++++++++++++++--- .../actions/builtin/fetch/cookiecutter.ts | 31 ++++++++--- .../scaffolder/actions/builtin/fetch/plain.ts | 25 ++++++--- .../actions/builtin/publish/github.ts | 44 ++++++++++----- .../src/scaffolder/actions/types.ts | 16 ++++-- .../src/scaffolder/tasks/TaskWorker.ts | 16 ++++++ 7 files changed, 154 insertions(+), 42 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts index 9d6140fbfe..62405679d0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/TemplateActionRegistry.ts @@ -14,13 +14,15 @@ * limitations under the License. */ -import { TemplateAction } from './types'; +import { ParameterBase, TemplateAction } from './types'; import { ConflictError, NotFoundError } from '@backstage/backend-common'; export class TemplateActionRegistry { - private readonly actions = new Map(); + private readonly actions = new Map>(); - register(action: TemplateAction) { + register( + action: TemplateAction, + ) { if (this.actions.has(action.id)) { throw new ConflictError( `Template action with ID '${action.id}' has already been registered`, @@ -29,7 +31,7 @@ export class TemplateActionRegistry { this.actions.set(action.id, action); } - get(actionId: string): TemplateAction { + get(actionId: string): TemplateAction { const action = this.actions.get(actionId); if (!action) { throw new NotFoundError( 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 4d19f1189f..275ed5ec69 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/catalog/register.ts @@ -23,19 +23,59 @@ import { TemplateAction } from '../../types'; export function createCatalogRegisterAction(options: { catalogClient: CatalogApi; integrations: ScmIntegrations; -}): TemplateAction { +}): TemplateAction< + | { catalogInfoUrl: string } + | { repoContentsUrl: string; catalogInfoPath?: string } +> { const { catalogClient, integrations } = options; return { id: 'catalog:register', + parameterSchema: { + oneOf: [ + { + type: 'object', + required: ['catalogInfoUrl'], + properties: { + catalogInfoUrl: { + title: 'Catalog Info URL', + description: + 'An absolute URL pointing to the catalog info file location', + type: 'string', + }, + }, + }, + { + type: 'object', + required: ['repoContentsUrl'], + properties: { + repoContentsUrl: { + title: 'Repository Contents URL', + description: + 'An absolute URL pointing to the root of a repository directory tree', + type: 'string', + }, + catalogInfoPath: { + title: 'Fetch URL', + description: + 'A relative path from the repo root pointing to the catalog info file, defaults to /catalog-info.yaml', + type: 'string', + }, + }, + }, + ], + }, async handler(ctx) { - const { - repoContentsUrl, - catalogInfoPath = '/catalog-info.yaml', - } = ctx.parameters; + const { parameters } = ctx; - let { catalogInfoUrl } = ctx.parameters; - if (!catalogInfoUrl) { + let catalogInfoUrl; + if ('catalogInfoUrl' in parameters) { + catalogInfoUrl = parameters.catalogInfoUrl; + } else { + const { + repoContentsUrl, + catalogInfoPath = '/catalog-info.yaml', + } = parameters; const integration = integrations.byUrl(repoContentsUrl as string); if (!integration) { throw new InputError('No integration found for host'); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts index 281a6e75d2..4aedb6d2af 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts @@ -19,6 +19,7 @@ import { resolve as resolvePath } from 'path'; import Docker from 'dockerode'; import { InputError, UrlReader } from '@backstage/backend-common'; import { ScmIntegrations } from '@backstage/integration'; +import { JsonObject } from '@backstage/config'; import { TemplaterBuilder, TemplaterValues } from '../../../stages/templater'; import { TemplateAction } from '../../types'; import { fetchContents } from './helpers'; @@ -28,11 +29,34 @@ export function createFetchCookiecutterAction(options: { urlReader: UrlReader; integrations: ScmIntegrations; templaters: TemplaterBuilder; -}): TemplateAction { +}): TemplateAction<{ url: string; targetPath?: string; values: JsonObject }> { const { dockerClient, urlReader, templaters, integrations } = options; return { id: 'fetch:cookiecutter', + parameterSchema: { + type: 'object', + required: ['url'], + properties: { + url: { + title: 'Fetch URL', + description: + 'Relative path or absolute URL pointing to the directory tree to fetch', + type: 'string', + }, + targetPath: { + title: 'Target Path', + description: + 'Target path within the working directory to download the contents to.', + type: 'string', + }, + values: { + title: 'Template Values', + description: 'Values to pass on to cookiecutter for templating', + type: 'object', + }, + }, + }, async handler(ctx) { ctx.logger.info('Fetching and then templating using cookiecutter'); const workDir = await ctx.createTemporaryDirectory(); @@ -66,11 +90,6 @@ export function createFetchCookiecutterAction(options: { // Finally move the template result into the task workspace const targetPath = ctx.parameters.targetPath ?? './'; - if (typeof targetPath !== 'string') { - throw new InputError( - `Fetch action targetPath is not a string, got ${targetPath}`, - ); - } const outputPath = resolvePath(ctx.workspacePath, targetPath); if (!outputPath.startsWith(ctx.workspacePath)) { throw new InputError( diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/plain.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/plain.ts index 371bc35e9a..6fd4d92611 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/plain.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/plain.ts @@ -23,21 +23,34 @@ import { fetchContents } from './helpers'; export function createFetchPlainAction(options: { urlReader: UrlReader; integrations: ScmIntegrations; -}): TemplateAction { +}): TemplateAction<{ url: string; targetPath?: string }> { const { urlReader, integrations } = options; return { id: 'fetch:plain', + parameterSchema: { + type: 'object', + required: ['url'], + properties: { + url: { + title: 'Fetch URL', + description: + 'Relative path or absolute URL pointing to the directory tree to fetch', + type: 'string', + }, + targetPath: { + title: 'Target Path', + description: + 'Target path within the working directory to download the contents to.', + type: 'string', + }, + }, + }, async handler(ctx) { ctx.logger.info('Fetching plain content from remote URL'); // Finally move the template result into the task workspace const targetPath = ctx.parameters.targetPath ?? './'; - if (typeof targetPath !== 'string') { - throw new InputError( - `Fetch action targetPath is not a string, got ${targetPath}`, - ); - } const outputPath = resolvePath(ctx.workspacePath, targetPath); if (!outputPath.startsWith(ctx.workspacePath)) { throw new InputError( diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts index 26f2775c91..822854cadb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/github.ts @@ -26,7 +26,11 @@ import { initRepoAndPush } from '../../../stages/publish/helpers'; export function createPublishGithubAction(options: { integrations: ScmIntegrations; repoVisibility: 'private' | 'internal' | 'public'; -}): TemplateAction { +}): TemplateAction<{ + repoUrl: string; + description?: string; + access?: string; +}> { const { integrations, repoVisibility } = options; const credentialsProviders = new Map( @@ -38,14 +42,27 @@ export function createPublishGithubAction(options: { return { id: 'publish:github', + parameterSchema: { + type: 'object', + required: ['repoUrl'], + properties: { + repoUrl: { + title: 'Repository Location', + type: 'string', + }, + description: { + title: 'Repository Description', + type: 'string', + }, + access: { + title: 'Additional Repository Access', + type: 'string', + }, + }, + }, async handler(ctx) { const { repoUrl, description, access } = ctx.parameters; - if (typeof repoUrl !== 'string') { - throw new Error( - `Invalid repo URL passed to publish:github, got ${typeof repoUrl}`, - ); - } let parsed; try { parsed = new URL(`https://${repoUrl}`); @@ -103,18 +120,17 @@ export function createPublishGithubAction(options: { org: owner, private: repoVisibility !== 'public', visibility: repoVisibility, - description: description as string, + description: description, }) : client.repos.createForAuthenticatedUser({ name: repo, private: repoVisibility === 'private', - description: description as string, + description: description, }); const { data } = await repoCreationPromise; - const accessString = access as string; - if (accessString?.startsWith(`${owner}/`)) { - const [, team] = accessString.split('/'); + if (access?.startsWith(`${owner}/`)) { + const [, team] = access.split('/'); await client.teams.addOrUpdateRepoPermissionsInOrg({ org: owner, team_slug: team, @@ -122,12 +138,12 @@ export function createPublishGithubAction(options: { repo, permission: 'admin', }); - // no need to add accessString if it's the person who own's the personal account - } else if (accessString && accessString !== owner) { + // no need to add access if it's the person who own's the personal account + } else if (access && access !== owner) { await client.repos.addCollaborator({ owner, repo, - username: accessString, + username: access, permission: 'admin', }); } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts index aed91e35f8..bdd74e933a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/types.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/types.ts @@ -16,9 +16,14 @@ import { Logger } from 'winston'; import { Writable } from 'stream'; -import { JsonValue } from '@backstage/config'; +import { JsonValue, JsonObject } from '@backstage/config'; +import { Schema } from 'jsonschema'; -export type ActionContext = { +type PartialJsonObject = Partial; +type PartialJsonValue = PartialJsonObject | JsonValue | undefined; +export type ParameterBase = Partial<{ [name: string]: PartialJsonValue }>; + +export type ActionContext = { /** * Base URL for the location of the task spec, typically the url of the source entity file. */ @@ -28,7 +33,7 @@ export type ActionContext = { logStream: Writable; workspacePath: string; - parameters: { [name: string]: JsonValue }; + parameters: Parameters; output(name: string, value: JsonValue): void; /** @@ -37,7 +42,8 @@ export type ActionContext = { createTemporaryDirectory(): Promise; }; -export type TemplateAction = { +export type TemplateAction = { id: string; - handler: (ctx: ActionContext) => Promise; + parameterSchema?: Schema; + handler: (ctx: ActionContext) => Promise; }; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 357e5e815c..548083095c 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -18,11 +18,13 @@ import { PassThrough } from 'stream'; import { Logger } from 'winston'; import * as winston from 'winston'; import { JsonValue, JsonObject } from '@backstage/config'; +import { validate as validateJsonSchema } from 'jsonschema'; import { TaskBroker, Task } from './types'; import fs from 'fs-extra'; import path from 'path'; import { TemplateActionRegistry } from '../actions/TemplateActionRegistry'; import * as handlebars from 'handlebars'; +import { InputError } from '@backstage/backend-common'; type Options = { logger: Logger; @@ -111,6 +113,20 @@ export class TaskWorker { }, ); + if (action.parameterSchema) { + const validateResult = validateJsonSchema( + parameters, + action.parameterSchema, + { propertyName: 'parameters' }, + ); + if (!validateResult.valid) { + const errors = validateResult.errors.join(', '); + throw new InputError( + `Invalid parameters 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