From fde4307fc407988e96d84e77843e1101d12c9b3f Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 09:36:07 -0600 Subject: [PATCH 1/8] feat: [#8920] Add `nunjucksFilters` option for user-provided filters Signed-off-by: David Zemon --- .../src/lib/templating/SecureTemplater.ts | 9 ++++++++- .../actions/builtin/createBuiltinActions.ts | 12 ++++++++++-- .../src/scaffolder/actions/builtin/fetch/template.ts | 4 +++- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 2 ++ .../src/scaffolder/tasks/TaskWorker.ts | 3 +++ plugins/scaffolder-backend/src/service/router.ts | 4 ++++ 6 files changed, 30 insertions(+), 4 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 54c9166020..416bad3295 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -93,6 +93,9 @@ export interface SecureTemplaterOptions { /* Enables jinja compatibility and the "jsonify" filter */ cookiecutterCompat?: boolean; + + /* Extra user-provided nunjucks filters */ + additionalFilters?: Record any>; } export type SecureTemplateRenderer = ( @@ -102,7 +105,7 @@ export type SecureTemplateRenderer = ( export class SecureTemplater { static async loadRenderer(options: SecureTemplaterOptions = {}) { - const { parseRepoUrl, cookiecutterCompat } = options; + const { parseRepoUrl, cookiecutterCompat, additionalFilters } = options; let sandbox = undefined; if (parseRepoUrl) { @@ -110,6 +113,10 @@ export class SecureTemplater { parseRepoUrl: (url: string) => JSON.stringify(parseRepoUrl(url)), }; } + sandbox = { + ...sandbox, + ...additionalFilters, + }; const vm = new VM({ sandbox }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index ce1e2a7f49..492e1881b8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -53,9 +53,16 @@ export const createBuiltinActions = (options: { catalogClient: CatalogApi; containerRunner?: ContainerRunner; config: Config; + nunjucksFilters?: Record any>; }) => { - const { reader, integrations, containerRunner, catalogClient, config } = - options; + const { + reader, + integrations, + containerRunner, + catalogClient, + config, + nunjucksFilters, + } = options; const githubCredentialsProvider: GithubCredentialsProvider = DefaultGithubCredentialsProvider.fromIntegrations(integrations); @@ -67,6 +74,7 @@ export const createBuiltinActions = (options: { createFetchTemplateAction({ integrations, reader, + nunjucksFilters, }), createPublishGithubAction({ integrations, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts index 2e25547e61..7d68c4a129 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -44,8 +44,9 @@ export type FetchTemplateInput = { export function createFetchTemplateAction(options: { reader: UrlReader; integrations: ScmIntegrations; + nunjucksFilters?: Record any>; }) { - const { reader, integrations } = options; + const { reader, integrations, nunjucksFilters } = options; return createTemplateAction({ id: 'fetch:template', @@ -182,6 +183,7 @@ export function createFetchTemplateAction(options: { const renderTemplate = await SecureTemplater.loadRenderer({ cookiecutterCompat: ctx.input.cookiecutterCompat, + additionalFilters: nunjucksFilters, }); for (const location of allEntriesInTemplate) { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 46f87c1d00..1a0b9c9d16 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -44,6 +44,7 @@ type NunjucksWorkflowRunnerOptions = { actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; logger: winston.Logger; + additionalFilters?: Record any>; }; type TemplateContext = { @@ -190,6 +191,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { parseRepoUrl(url: string) { return parseRepoUrl(url, integrations); }, + additionalFilters: this.options.additionalFilters, }); try { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 8d89735361..adbcd75773 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -46,6 +46,7 @@ export type CreateWorkerOptions = { integrations: ScmIntegrations; workingDirectory: string; logger: Logger; + nunjucksFilters?: Record any>; }; /** @@ -63,6 +64,7 @@ export class TaskWorker { actionRegistry, integrations, workingDirectory, + nunjucksFilters, } = options; const legacyWorkflowRunner = new HandlebarsWorkflowRunner({ @@ -77,6 +79,7 @@ export class TaskWorker { integrations, logger, workingDirectory, + additionalFilters: nunjucksFilters, }); return new TaskWorker({ diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index f4f7398c8a..b12dace5fa 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -57,6 +57,7 @@ export interface RouterOptions { taskWorkers?: number; containerRunner?: ContainerRunner; taskBroker?: TaskBroker; + nunjucksFilters?: Record any>; } function isSupportedTemplate( @@ -83,6 +84,7 @@ export async function createRouter( actions, containerRunner, taskWorkers, + nunjucksFilters, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -110,6 +112,7 @@ export async function createRouter( integrations, logger, workingDirectory, + nunjucksFilters, }); workers.push(worker); } @@ -122,6 +125,7 @@ export async function createRouter( containerRunner, reader, config, + nunjucksFilters, }); actionsToRegister.forEach(action => actionRegistry.register(action)); From af6b692cb3ca685d97ef16b40b6e9dc99913e3d4 Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 09:37:05 -0600 Subject: [PATCH 2/8] feat: [8920] Define explicit type for filter; Add SecureTemplater tests Signed-off-by: David Zemon --- .../lib/templating/SecureTemplater.test.ts | 46 +++++++++++++++++++ .../src/lib/templating/SecureTemplater.ts | 37 +++++++++++---- .../actions/builtin/createBuiltinActions.ts | 3 +- .../actions/builtin/fetch/template.ts | 7 ++- .../tasks/NunjucksWorkflowRunner.ts | 3 +- .../src/scaffolder/tasks/TaskWorker.ts | 3 +- .../scaffolder-backend/src/service/router.ts | 3 +- 7 files changed, 87 insertions(+), 15 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts index 134b877efe..ce524384c7 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts @@ -99,6 +99,52 @@ describe('SecureTemplater', () => { ]); }); + it('should make additional filters available when requested', async () => { + const mockFilter1 = jest.fn(() => 'filtered text'); + const mockFilter2 = jest.fn((var1, var2) => `${var1} ${var2}`); + const mockFilter3 = jest.fn((var1, var2) => ({ var1, var2 })); + const renderWith = await SecureTemplater.loadRenderer({ + additionalFilters: { mockFilter1, mockFilter2, mockFilter3 }, + }); + const renderWithout = await SecureTemplater.loadRenderer(); + + const ctx = { inputValue: 'the input value' }; + + expect(renderWith('${{ inputValue | mockFilter1 }}', ctx)).toBe( + 'filtered text', + ); + expect( + renderWith('${{ inputValue | mockFilter2("extra arg") }}', ctx), + ).toBe('the input value extra arg'); + expect( + renderWith( + '${{ inputValue | mockFilter3("another extra arg") | dump }}', + ctx, + ), + ).toBe( + JSON.stringify({ + var1: 'the input value', + var2: 'another extra arg', + }), + ); + + expect(() => renderWithout('${{ inputValue | mockFilter1 }}', ctx)).toThrow( + /Error: filter not found: mockFilter1/, + ); + expect(() => + renderWithout('${{ inputValue | mockFilter2("extra arg") }}', ctx), + ).toThrow(/Error: filter not found: mockFilter2/); + expect(() => + renderWithout('${{ inputValue | mockFilter3("extra arg") }}', ctx), + ).toThrow(/Error: filter not found: mockFilter3/); + + expect(mockFilter1.mock.calls).toEqual([['the input value']]); + expect(mockFilter2.mock.calls).toEqual([['the input value', 'extra arg']]); + expect(mockFilter3.mock.calls).toEqual([ + ['the input value', 'another extra arg'], + ]); + }); + it('should not allow helpers to be rewritten', async () => { const render = await SecureTemplater.loadRenderer({ parseRepoUrl: () => ({ diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 416bad3295..b3e99cf1dd 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -17,8 +17,10 @@ import { VM } from 'vm2'; import { resolvePackagePath } from '@backstage/backend-common'; import fs from 'fs-extra'; +import { JsonValue } from '@backstage/types'; import { RepoSpec } from '../../scaffolder/actions/builtin/publish/util'; +// language=JavaScript const mkScript = (nunjucksSource: string) => ` const { render, renderCompat } = (() => { const module = {}; @@ -56,6 +58,16 @@ const { render, renderCompat } = (() => { }); } + if (typeof additionalFilters !== "undefined") { + Object.entries(additionalFilters) + .forEach(([filterName, filterFunction]) => { + env.addFilter( + filterName, + (...args) => JSON.parse(filterFunction.apply(null, args)) + ); + }); + } + let uninstallCompat = undefined; function render(str, values) { @@ -87,6 +99,8 @@ const { render, renderCompat } = (() => { })(); `; +export type NunjucksFilter = (...args: JsonValue[]) => JsonValue | undefined; + export interface SecureTemplaterOptions { /* Optional implementation of the parseRepoUrl filter */ parseRepoUrl?(repoUrl: string): RepoSpec; @@ -95,7 +109,7 @@ export interface SecureTemplaterOptions { cookiecutterCompat?: boolean; /* Extra user-provided nunjucks filters */ - additionalFilters?: Record any>; + additionalFilters?: Record; } export type SecureTemplateRenderer = ( @@ -106,17 +120,22 @@ export type SecureTemplateRenderer = ( export class SecureTemplater { static async loadRenderer(options: SecureTemplaterOptions = {}) { const { parseRepoUrl, cookiecutterCompat, additionalFilters } = options; - let sandbox = undefined; + const sandbox: Record = {}; if (parseRepoUrl) { - sandbox = { - parseRepoUrl: (url: string) => JSON.stringify(parseRepoUrl(url)), - }; + sandbox.parseRepoUrl = (url: string) => JSON.stringify(parseRepoUrl(url)); + } + + if (additionalFilters) { + sandbox.additionalFilters = Object.entries(additionalFilters) + .filter(([_, filterFunction]) => !!filterFunction) + .reduce((safeFilters, [filterName, filterFunction]) => { + const newSafeFilters = { ...safeFilters }; + newSafeFilters[filterName] = (...args) => + JSON.stringify(filterFunction.apply(null, args)); + return newSafeFilters; + }, {} as Record); } - sandbox = { - ...sandbox, - ...additionalFilters, - }; const vm = new VM({ sandbox }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index 492e1881b8..9cc60a6b1e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -46,6 +46,7 @@ import { createGithubActionsDispatchAction, createGithubWebhookAction, } from './github'; +import { NunjucksFilter } from '../../../lib/templating/SecureTemplater'; export const createBuiltinActions = (options: { reader: UrlReader; @@ -53,7 +54,7 @@ export const createBuiltinActions = (options: { catalogClient: CatalogApi; containerRunner?: ContainerRunner; config: Config; - nunjucksFilters?: Record any>; + nunjucksFilters?: Record; }) => { const { reader, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts index 7d68c4a129..c4c666abc1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -23,7 +23,10 @@ import { createTemplateAction } from '../../createTemplateAction'; import globby from 'globby'; import fs from 'fs-extra'; import { isBinaryFile } from 'isbinaryfile'; -import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; +import { + NunjucksFilter, + SecureTemplater, +} from '../../../../lib/templating/SecureTemplater'; type CookieCompatInput = { copyWithoutRender?: string[]; @@ -44,7 +47,7 @@ export type FetchTemplateInput = { export function createFetchTemplateAction(options: { reader: UrlReader; integrations: ScmIntegrations; - nunjucksFilters?: Record any>; + nunjucksFilters?: Record; }) { const { reader, integrations, nunjucksFilters } = options; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 1a0b9c9d16..5bcf83a231 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -35,6 +35,7 @@ import { validate as validateJsonSchema } from 'jsonschema'; import { parseRepoUrl } from '../actions/builtin/publish/util'; import { TemplateActionRegistry } from '../actions'; import { + NunjucksFilter, SecureTemplater, SecureTemplateRenderer, } from '../../lib/templating/SecureTemplater'; @@ -44,7 +45,7 @@ type NunjucksWorkflowRunnerOptions = { actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; logger: winston.Logger; - additionalFilters?: Record any>; + additionalFilters?: Record; }; type TemplateContext = { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index adbcd75773..8bf10b051a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -21,6 +21,7 @@ import { Logger } from 'winston'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; import { assertError } from '@backstage/errors'; +import { NunjucksFilter } from '../../lib/templating/SecureTemplater'; /** * TaskWorkerOptions @@ -46,7 +47,7 @@ export type CreateWorkerOptions = { integrations: ScmIntegrations; workingDirectory: string; logger: Logger; - nunjucksFilters?: Record any>; + nunjucksFilters?: Record; }; /** diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index b12dace5fa..3770f525a4 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -41,6 +41,7 @@ import { } from '../scaffolder'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { getEntityBaseUrl, getWorkingDirectory } from './helpers'; +import { NunjucksFilter } from '../lib/templating/SecureTemplater'; /** * RouterOptions @@ -57,7 +58,7 @@ export interface RouterOptions { taskWorkers?: number; containerRunner?: ContainerRunner; taskBroker?: TaskBroker; - nunjucksFilters?: Record any>; + nunjucksFilters?: Record; } function isSupportedTemplate( From a5600dba15a89684a365190c4837c73a69ff77c0 Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 09:37:59 -0600 Subject: [PATCH 3/8] feat: [8920] Define explicit type for filter; Add SecureTemplater tests Signed-off-by: David Zemon --- plugins/scaffolder-backend/src/index.ts | 2 +- plugins/scaffolder-backend/src/lib/index.ts | 18 ++++++++++++++++++ .../src/lib/templating/index.ts | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 plugins/scaffolder-backend/src/lib/index.ts create mode 100644 plugins/scaffolder-backend/src/lib/templating/index.ts diff --git a/plugins/scaffolder-backend/src/index.ts b/plugins/scaffolder-backend/src/index.ts index 76cda63eef..5978b6389b 100644 --- a/plugins/scaffolder-backend/src/index.ts +++ b/plugins/scaffolder-backend/src/index.ts @@ -22,5 +22,5 @@ export * from './scaffolder'; export * from './service/router'; -export * from './lib/catalog'; +export * from './lib'; export * from './processor'; diff --git a/plugins/scaffolder-backend/src/lib/index.ts b/plugins/scaffolder-backend/src/lib/index.ts new file mode 100644 index 0000000000..dbb4348a2c --- /dev/null +++ b/plugins/scaffolder-backend/src/lib/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2022 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. + */ + +export * from './catalog'; +export * from './templating'; diff --git a/plugins/scaffolder-backend/src/lib/templating/index.ts b/plugins/scaffolder-backend/src/lib/templating/index.ts new file mode 100644 index 0000000000..110d7de20b --- /dev/null +++ b/plugins/scaffolder-backend/src/lib/templating/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2022 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. + */ + +export type { NunjucksFilter } from './SecureTemplater'; From 723e5fc26a978800631e50e2d42f4dc269de893c Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 09:39:38 -0600 Subject: [PATCH 4/8] refactor: [#8920] Clean-up based on PR feedback Signed-off-by: David Zemon --- .../lib/templating/SecureTemplater.test.ts | 2 +- .../src/lib/templating/SecureTemplater.ts | 34 ++++++++----------- .../actions/builtin/fetch/template.ts | 2 +- .../tasks/NunjucksWorkflowRunner.ts | 4 +-- .../src/scaffolder/tasks/TaskWorker.ts | 2 +- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts index ce524384c7..e3d7a55b87 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts @@ -104,7 +104,7 @@ describe('SecureTemplater', () => { const mockFilter2 = jest.fn((var1, var2) => `${var1} ${var2}`); const mockFilter3 = jest.fn((var1, var2) => ({ var1, var2 })); const renderWith = await SecureTemplater.loadRenderer({ - additionalFilters: { mockFilter1, mockFilter2, mockFilter3 }, + nunjucksFilters: { mockFilter1, mockFilter2, mockFilter3 }, }); const renderWithout = await SecureTemplater.loadRenderer(); diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index b3e99cf1dd..1c1e2639a3 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -58,14 +58,10 @@ const { render, renderCompat } = (() => { }); } - if (typeof additionalFilters !== "undefined") { - Object.entries(additionalFilters) - .forEach(([filterName, filterFunction]) => { - env.addFilter( - filterName, - (...args) => JSON.parse(filterFunction.apply(null, args)) - ); - }); + if (typeof nunjucksFilters !== 'undefined') { + for (const [filterName, filterFn] of Object.entries(nunjucksFilters)) { + env.addFilter(filterName, (...args) => JSON.parse(filterFn(...args))); + } } let uninstallCompat = undefined; @@ -109,7 +105,7 @@ export interface SecureTemplaterOptions { cookiecutterCompat?: boolean; /* Extra user-provided nunjucks filters */ - additionalFilters?: Record; + nunjucksFilters?: Record; } export type SecureTemplateRenderer = ( @@ -119,22 +115,22 @@ export type SecureTemplateRenderer = ( export class SecureTemplater { static async loadRenderer(options: SecureTemplaterOptions = {}) { - const { parseRepoUrl, cookiecutterCompat, additionalFilters } = options; + const { parseRepoUrl, cookiecutterCompat, nunjucksFilters } = options; const sandbox: Record = {}; if (parseRepoUrl) { sandbox.parseRepoUrl = (url: string) => JSON.stringify(parseRepoUrl(url)); } - if (additionalFilters) { - sandbox.additionalFilters = Object.entries(additionalFilters) - .filter(([_, filterFunction]) => !!filterFunction) - .reduce((safeFilters, [filterName, filterFunction]) => { - const newSafeFilters = { ...safeFilters }; - newSafeFilters[filterName] = (...args) => - JSON.stringify(filterFunction.apply(null, args)); - return newSafeFilters; - }, {} as Record); + if (nunjucksFilters) { + sandbox.nunjucksFilters = Object.fromEntries( + Object.entries(nunjucksFilters) + .filter(([_, filterFunction]) => !!filterFunction) + .map(([filterName, filterFunction]) => [ + filterName, + (...args: JsonValue[]) => JSON.stringify(filterFunction(...args)), + ]), + ); } const vm = new VM({ sandbox }); diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts index c4c666abc1..58e96f4164 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -186,7 +186,7 @@ export function createFetchTemplateAction(options: { const renderTemplate = await SecureTemplater.loadRenderer({ cookiecutterCompat: ctx.input.cookiecutterCompat, - additionalFilters: nunjucksFilters, + nunjucksFilters: nunjucksFilters, }); for (const location of allEntriesInTemplate) { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 5bcf83a231..41b605e2e2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -45,7 +45,7 @@ type NunjucksWorkflowRunnerOptions = { actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; logger: winston.Logger; - additionalFilters?: Record; + nunjucksFilters?: Record; }; type TemplateContext = { @@ -192,7 +192,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { parseRepoUrl(url: string) { return parseRepoUrl(url, integrations); }, - additionalFilters: this.options.additionalFilters, + nunjucksFilters: this.options.nunjucksFilters, }); try { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 8bf10b051a..8439c4ed8e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -80,7 +80,7 @@ export class TaskWorker { integrations, logger, workingDirectory, - additionalFilters: nunjucksFilters, + nunjucksFilters: nunjucksFilters, }); return new TaskWorker({ From 0e4f141661bf99074df2ae7376b79c6459e83285 Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 09:51:14 -0600 Subject: [PATCH 5/8] refactor: [#8920] Minor import optimization Signed-off-by: David Zemon --- plugins/scaffolder-backend/src/service/router.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 3770f525a4..085b4b4d78 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -29,7 +29,7 @@ import express from 'express'; import Router from 'express-promise-router'; import { validate } from 'jsonschema'; import { Logger } from 'winston'; -import { CatalogEntityClient } from '../lib/catalog'; +import { CatalogEntityClient, NunjucksFilter } from '../lib'; import { createBuiltinActions, DatabaseTaskStore, @@ -41,7 +41,6 @@ import { } from '../scaffolder'; import { StorageTaskBroker } from '../scaffolder/tasks/StorageTaskBroker'; import { getEntityBaseUrl, getWorkingDirectory } from './helpers'; -import { NunjucksFilter } from '../lib/templating/SecureTemplater'; /** * RouterOptions From 2137b84c3225a8650949c25990bd6ed341a6742f Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 10:44:02 -0600 Subject: [PATCH 6/8] refactor: [#8920] Rename for inclusion of other templating engines Signed-off-by: David Zemon --- .../src/lib/templating/SecureTemplater.test.ts | 2 +- .../src/lib/templating/SecureTemplater.ts | 17 +++++++++-------- .../src/lib/templating/index.ts | 2 +- .../actions/builtin/createBuiltinActions.ts | 8 ++++---- .../actions/builtin/fetch/template.ts | 8 ++++---- .../scaffolder/tasks/NunjucksWorkflowRunner.ts | 6 +++--- .../src/scaffolder/tasks/TaskWorker.ts | 8 ++++---- .../scaffolder-backend/src/service/router.ts | 10 +++++----- 8 files changed, 31 insertions(+), 30 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts index e3d7a55b87..8b8d7847ac 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts @@ -104,7 +104,7 @@ describe('SecureTemplater', () => { const mockFilter2 = jest.fn((var1, var2) => `${var1} ${var2}`); const mockFilter3 = jest.fn((var1, var2) => ({ var1, var2 })); const renderWith = await SecureTemplater.loadRenderer({ - nunjucksFilters: { mockFilter1, mockFilter2, mockFilter3 }, + additionalTemplateFilters: { mockFilter1, mockFilter2, mockFilter3 }, }); const renderWithout = await SecureTemplater.loadRenderer(); diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 1c1e2639a3..6d3ce6f556 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -58,8 +58,8 @@ const { render, renderCompat } = (() => { }); } - if (typeof nunjucksFilters !== 'undefined') { - for (const [filterName, filterFn] of Object.entries(nunjucksFilters)) { + if (typeof additionalTemplateFilters !== 'undefined') { + for (const [filterName, filterFn] of Object.entries(additionalTemplateFilters)) { env.addFilter(filterName, (...args) => JSON.parse(filterFn(...args))); } } @@ -95,7 +95,7 @@ const { render, renderCompat } = (() => { })(); `; -export type NunjucksFilter = (...args: JsonValue[]) => JsonValue | undefined; +export type TemplateFilter = (...args: JsonValue[]) => JsonValue | undefined; export interface SecureTemplaterOptions { /* Optional implementation of the parseRepoUrl filter */ @@ -105,7 +105,7 @@ export interface SecureTemplaterOptions { cookiecutterCompat?: boolean; /* Extra user-provided nunjucks filters */ - nunjucksFilters?: Record; + additionalTemplateFilters?: Record; } export type SecureTemplateRenderer = ( @@ -115,16 +115,17 @@ export type SecureTemplateRenderer = ( export class SecureTemplater { static async loadRenderer(options: SecureTemplaterOptions = {}) { - const { parseRepoUrl, cookiecutterCompat, nunjucksFilters } = options; + const { parseRepoUrl, cookiecutterCompat, additionalTemplateFilters } = + options; const sandbox: Record = {}; if (parseRepoUrl) { sandbox.parseRepoUrl = (url: string) => JSON.stringify(parseRepoUrl(url)); } - if (nunjucksFilters) { - sandbox.nunjucksFilters = Object.fromEntries( - Object.entries(nunjucksFilters) + if (additionalTemplateFilters) { + sandbox.additionalTemplateFilters = Object.fromEntries( + Object.entries(additionalTemplateFilters) .filter(([_, filterFunction]) => !!filterFunction) .map(([filterName, filterFunction]) => [ filterName, diff --git a/plugins/scaffolder-backend/src/lib/templating/index.ts b/plugins/scaffolder-backend/src/lib/templating/index.ts index 110d7de20b..29d77291a1 100644 --- a/plugins/scaffolder-backend/src/lib/templating/index.ts +++ b/plugins/scaffolder-backend/src/lib/templating/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export type { NunjucksFilter } from './SecureTemplater'; +export type { TemplateFilter } from './SecureTemplater'; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index 9cc60a6b1e..481ab894ec 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -46,7 +46,7 @@ import { createGithubActionsDispatchAction, createGithubWebhookAction, } from './github'; -import { NunjucksFilter } from '../../../lib/templating/SecureTemplater'; +import { TemplateFilter } from '../../../lib'; export const createBuiltinActions = (options: { reader: UrlReader; @@ -54,7 +54,7 @@ export const createBuiltinActions = (options: { catalogClient: CatalogApi; containerRunner?: ContainerRunner; config: Config; - nunjucksFilters?: Record; + additionalTemplateFilters?: Record; }) => { const { reader, @@ -62,7 +62,7 @@ export const createBuiltinActions = (options: { containerRunner, catalogClient, config, - nunjucksFilters, + additionalTemplateFilters, } = options; const githubCredentialsProvider: GithubCredentialsProvider = DefaultGithubCredentialsProvider.fromIntegrations(integrations); @@ -75,7 +75,7 @@ export const createBuiltinActions = (options: { createFetchTemplateAction({ integrations, reader, - nunjucksFilters, + additionalTemplateFilters, }), createPublishGithubAction({ integrations, diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts index 58e96f4164..32a3a84070 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -24,7 +24,7 @@ import globby from 'globby'; import fs from 'fs-extra'; import { isBinaryFile } from 'isbinaryfile'; import { - NunjucksFilter, + TemplateFilter, SecureTemplater, } from '../../../../lib/templating/SecureTemplater'; @@ -47,9 +47,9 @@ export type FetchTemplateInput = { export function createFetchTemplateAction(options: { reader: UrlReader; integrations: ScmIntegrations; - nunjucksFilters?: Record; + additionalTemplateFilters?: Record; }) { - const { reader, integrations, nunjucksFilters } = options; + const { reader, integrations, additionalTemplateFilters } = options; return createTemplateAction({ id: 'fetch:template', @@ -186,7 +186,7 @@ export function createFetchTemplateAction(options: { const renderTemplate = await SecureTemplater.loadRenderer({ cookiecutterCompat: ctx.input.cookiecutterCompat, - nunjucksFilters: nunjucksFilters, + additionalTemplateFilters, }); for (const location of allEntriesInTemplate) { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 41b605e2e2..5089998834 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -35,7 +35,7 @@ import { validate as validateJsonSchema } from 'jsonschema'; import { parseRepoUrl } from '../actions/builtin/publish/util'; import { TemplateActionRegistry } from '../actions'; import { - NunjucksFilter, + TemplateFilter, SecureTemplater, SecureTemplateRenderer, } from '../../lib/templating/SecureTemplater'; @@ -45,7 +45,7 @@ type NunjucksWorkflowRunnerOptions = { actionRegistry: TemplateActionRegistry; integrations: ScmIntegrations; logger: winston.Logger; - nunjucksFilters?: Record; + additionalTemplateFilters?: Record; }; type TemplateContext = { @@ -192,7 +192,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { parseRepoUrl(url: string) { return parseRepoUrl(url, integrations); }, - nunjucksFilters: this.options.nunjucksFilters, + additionalTemplateFilters: this.options.additionalTemplateFilters, }); try { diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 8439c4ed8e..8de9359806 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -21,7 +21,7 @@ import { Logger } from 'winston'; import { TemplateActionRegistry } from '../actions'; import { ScmIntegrations } from '@backstage/integration'; import { assertError } from '@backstage/errors'; -import { NunjucksFilter } from '../../lib/templating/SecureTemplater'; +import { TemplateFilter } from '../../lib/templating/SecureTemplater'; /** * TaskWorkerOptions @@ -47,7 +47,7 @@ export type CreateWorkerOptions = { integrations: ScmIntegrations; workingDirectory: string; logger: Logger; - nunjucksFilters?: Record; + additionalTemplateFilters?: Record; }; /** @@ -65,7 +65,7 @@ export class TaskWorker { actionRegistry, integrations, workingDirectory, - nunjucksFilters, + additionalTemplateFilters, } = options; const legacyWorkflowRunner = new HandlebarsWorkflowRunner({ @@ -80,7 +80,7 @@ export class TaskWorker { integrations, logger, workingDirectory, - nunjucksFilters: nunjucksFilters, + additionalTemplateFilters, }); return new TaskWorker({ diff --git a/plugins/scaffolder-backend/src/service/router.ts b/plugins/scaffolder-backend/src/service/router.ts index 085b4b4d78..85ed8a60ab 100644 --- a/plugins/scaffolder-backend/src/service/router.ts +++ b/plugins/scaffolder-backend/src/service/router.ts @@ -29,7 +29,7 @@ import express from 'express'; import Router from 'express-promise-router'; import { validate } from 'jsonschema'; import { Logger } from 'winston'; -import { CatalogEntityClient, NunjucksFilter } from '../lib'; +import { CatalogEntityClient, TemplateFilter } from '../lib'; import { createBuiltinActions, DatabaseTaskStore, @@ -57,7 +57,7 @@ export interface RouterOptions { taskWorkers?: number; containerRunner?: ContainerRunner; taskBroker?: TaskBroker; - nunjucksFilters?: Record; + additionalTemplateFilters?: Record; } function isSupportedTemplate( @@ -84,7 +84,7 @@ export async function createRouter( actions, containerRunner, taskWorkers, - nunjucksFilters, + additionalTemplateFilters, } = options; const logger = parentLogger.child({ plugin: 'scaffolder' }); @@ -112,7 +112,7 @@ export async function createRouter( integrations, logger, workingDirectory, - nunjucksFilters, + additionalTemplateFilters, }); workers.push(worker); } @@ -125,7 +125,7 @@ export async function createRouter( containerRunner, reader, config, - nunjucksFilters, + additionalTemplateFilters, }); actionsToRegister.forEach(action => actionRegistry.register(action)); From b16ad3f8c98303e1b62a48e06243dc2f9dfb070b Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 10:49:31 -0600 Subject: [PATCH 7/8] chore: [#8920] Update API report Signed-off-by: David Zemon --- plugins/scaffolder-backend/api-report.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 12b29cae39..c33aa91bee 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -76,6 +76,7 @@ export const createBuiltinActions: (options: { catalogClient: CatalogApi; containerRunner?: ContainerRunner; config: Config; + additionalTemplateFilters?: Record; }) => TemplateAction[]; // Warning: (ae-missing-release-tag) "createCatalogRegisterAction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -112,6 +113,7 @@ export function createFetchPlainAction(options: { export function createFetchTemplateAction(options: { reader: UrlReader; integrations: ScmIntegrations; + additionalTemplateFilters?: Record; }): TemplateAction; // Warning: (ae-missing-release-tag) "createFilesystemDeleteAction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -219,6 +221,7 @@ export type CreateWorkerOptions = { integrations: ScmIntegrations; workingDirectory: string; logger: Logger_2; + additionalTemplateFilters?: Record; }; // @public @@ -304,6 +307,8 @@ export interface RouterOptions { // (undocumented) actions?: TemplateAction[]; // (undocumented) + additionalTemplateFilters?: Record; + // (undocumented) catalogClient: CatalogApi; // (undocumented) config: Config; @@ -545,5 +550,10 @@ export class TemplateActionRegistry { ): void; } +// Warning: (ae-missing-release-tag) "TemplateFilter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export type TemplateFilter = (...args: JsonValue[]) => JsonValue | undefined; + export { TemplateMetadata }; ``` From 0d5e846a78f2fb9a2c1f7bc9c15f0b3e942b5ec9 Mon Sep 17 00:00:00 2001 From: David Zemon Date: Fri, 21 Jan 2022 15:16:33 -0600 Subject: [PATCH 8/8] chore: [#8920] Add changeset Signed-off-by: David Zemon --- .changeset/rude-clouds-chew.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/rude-clouds-chew.md diff --git a/.changeset/rude-clouds-chew.md b/.changeset/rude-clouds-chew.md new file mode 100644 index 0000000000..f39a7bdaac --- /dev/null +++ b/.changeset/rude-clouds-chew.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Expose a new option to provide additional template filters via `@backstage/scaffolder-backend`'s `createRouter()` function.