From 156fefcd82c29b4737ba1908fcb915001cfeb137 Mon Sep 17 00:00:00 2001 From: blam Date: Sun, 27 Jun 2021 03:09:05 +0200 Subject: [PATCH 01/35] sample templater Signed-off-by: blam --- plugins/scaffolder-backend/package.json | 2 + .../actions/builtin/createBuiltinActions.ts | 10 +- .../scaffolder/actions/builtin/fetch/index.ts | 1 + .../actions/builtin/fetch/template.ts | 121 ++++++++++++++++++ yarn.lock | 21 ++- 5 files changed, 153 insertions(+), 2 deletions(-) create mode 100644 plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index ab7ab1f552..4ce37999d6 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -40,6 +40,7 @@ "@octokit/rest": "^18.5.3", "@types/express": "^4.17.6", "@types/git-url-parse": "^9.0.0", + "@types/nunjucks": "^3.1.4", "azure-devops-node-api": "^10.1.1", "command-exists": "^1.2.9", "compression": "^1.7.4", @@ -58,6 +59,7 @@ "lodash": "^4.17.21", "luxon": "^1.26.0", "morgan": "^1.10.0", + "nunjucks": "^3.2.3", "octokit-plugin-create-pull-request": "^3.9.3", "uuid": "^8.2.0", "winston": "^3.2.1", diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts index 46b8db72b4..71042129db 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/createBuiltinActions.ts @@ -23,7 +23,11 @@ import { createCatalogWriteAction, } from './catalog'; import { createDebugLogAction } from './debug'; -import { createFetchCookiecutterAction, createFetchPlainAction } from './fetch'; +import { + createFetchCookiecutterAction, + createFetchPlainAction, + createFetchTemplateAction, +} from './fetch'; import { createFilesystemDeleteAction, createFilesystemRenameAction, @@ -54,6 +58,10 @@ export const createBuiltinActions = (options: { integrations, templaters, }), + createFetchTemplateAction({ + integrations, + reader, + }), createPublishGithubAction({ integrations, }), diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/index.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/index.ts index 8e0f93f3ab..e06de4a2b5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/index.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/index.ts @@ -16,3 +16,4 @@ export { createFetchPlainAction } from './plain'; export { createFetchCookiecutterAction } from './cookiecutter'; +export { createFetchTemplateAction } from './template'; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts new file mode 100644 index 0000000000..ea376e76ab --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -0,0 +1,121 @@ +/* + * 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 path, { resolve as resolvePath } from 'path'; +import { UrlReader } from '@backstage/backend-common'; +import { InputError } from '@backstage/errors'; +import { ScmIntegrations } from '@backstage/integration'; +import { fetchContents } from './helpers'; +import { createTemplateAction } from '../../createTemplateAction'; +import globby from 'globby'; +import nunjucks from 'nunjucks'; +import fs from 'fs-extra'; + +export function createFetchTemplateAction(options: { + reader: UrlReader; + integrations: ScmIntegrations; +}) { + const { reader, integrations } = options; + + return createTemplateAction<{ + url: string; + targetPath?: string; + values: any; + }>({ + id: 'fetch:template', + description: + "Downloads a skeleton and will template variables into the skeleton and places the result in the workspace, or optionally in a subdirectory specified by the 'targetPath' input option.", + schema: { + input: { + 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 the templating engine', + type: 'object', + }, + }, + }, + }, + async handler(ctx) { + ctx.logger.info('Fetching template content from remote URL'); + const workDir = await ctx.createTemporaryDirectory(); + const templateDir = resolvePath(workDir, 'template'); + + // Finally move the template result into the task workspace + const targetPath = ctx.input.targetPath ?? './'; + const outputPath = path.resolve(ctx.workspacePath, targetPath); + if (!outputPath.startsWith(ctx.workspacePath)) { + throw new InputError( + `Fetch action targetPath may not specify a path outside the working directory`, + ); + } + + await fetchContents({ + reader, + integrations, + baseUrl: ctx.baseUrl, + fetchUrl: ctx.input.url, + outputPath: templateDir, + }); + + ctx.logger.info( + 'Fetched template, beginning templating process with values', + ctx.input.values, + ); + + const allFilesInTemplates = await globby(`*`, { cwd: templateDir }); + nunjucks.installJinjaCompat(); + const templater = nunjucks.configure({ + tags: { + variableStart: '${', + variableEnd: '}', + }, + autoescape: false, + }); + + templater.addFilter('jsonify', s => JSON.stringify(s)); + + for (const location of allFilesInTemplates) { + console.log(location); + + const filepath = templater.renderString(location, ctx.input.values); + console.log(filepath); + console.log('is', resolvePath(outputPath, filepath)); + await fs.writeFile( + resolvePath(outputPath, filepath), + templater.renderString( + await fs.readFile(resolvePath(templateDir, location), 'utf-8'), + ctx.input.values, + ), + ); + } + }, + }); +} diff --git a/yarn.lock b/yarn.lock index 27a6fcc250..96299709e4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6242,6 +6242,11 @@ resolved "https://registry.npmjs.org/@types/npmlog/-/npmlog-4.1.2.tgz#d070fe6a6b78755d1092a3dc492d34c3d8f871c4" integrity sha512-4QQmOF5KlwfxJ5IGXFIudkeLCdMABz03RcUXu+LCb24zmln8QW6aDjuGl4d4XPVLf2j+FnjelHTP7dvceAFbhA== +"@types/nunjucks@^3.1.4": + version "3.1.4" + resolved "https://registry.npmjs.org/@types/nunjucks/-/nunjucks-3.1.4.tgz#2f80e2fa5e7982b2131201d0b4474ab4d03d9de1" + integrity sha512-cR65PLlHKW/qxxj840dbNb3ICO+iAVQzaNKJ8TcKOVKFi+QcAkhw9SCY8VFAyU41SmJMs+2nrIN2JGhX+jYb7A== + "@types/oauth@*": version "0.9.1" resolved "https://registry.npmjs.org/@types/oauth/-/oauth-0.9.1.tgz#e17221e7f7936b0459ae7d006255dff61adca305" @@ -7130,6 +7135,11 @@ JSONStream@^1.0.4: jsonparse "^1.2.0" through ">=2.2.7 <3" +a-sync-waterfall@^1.0.0: + version "1.0.1" + resolved "https://registry.npmjs.org/a-sync-waterfall/-/a-sync-waterfall-1.0.1.tgz#75b6b6aa72598b497a125e7a2770f14f4c8a1fa7" + integrity sha512-RYTOHHdWipFUliRFMCS4X2Yn2X8M87V/OpSqWzKKOGhzqyUxzyVmhHDH9sAvG+ZuQf/TAOFsLCpMw09I1ufUnA== + abab@^2.0.0, abab@^2.0.3: version "2.0.3" resolved "https://registry.npmjs.org/abab/-/abab-2.0.3.tgz#623e2075e02eb2d3f2475e49f99c91846467907a" @@ -7842,7 +7852,7 @@ arrify@^2.0.0, arrify@^2.0.1: resolved "https://registry.npmjs.org/arrify/-/arrify-2.0.1.tgz#c9655e9331e0abcd588d2a7cad7e9956f66701fa" integrity sha512-3duEwti880xqi4eAMN8AyR4a0ByT90zoYdLlevfrvU43vb0YZwZVfxOgxWrLXXXpyugL0hNZc9G6BiB5B3nUug== -asap@^2.0.0, asap@~2.0.3: +asap@^2.0.0, asap@^2.0.3, asap@~2.0.3: version "2.0.6" resolved "https://registry.npmjs.org/asap/-/asap-2.0.6.tgz#e50347611d7e690943208bbdafebcbc2fb866d46" integrity sha1-5QNHYR1+aQlDIIu9r+vLwvuGbUY= @@ -19532,6 +19542,15 @@ number-is-nan@^1.0.0: resolved "https://registry.npmjs.org/number-is-nan/-/number-is-nan-1.0.1.tgz#097b602b53422a522c1afb8790318336941a011d" integrity sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0= +nunjucks@^3.2.3: + version "3.2.3" + resolved "https://registry.npmjs.org/nunjucks/-/nunjucks-3.2.3.tgz#1b33615247290e94e28263b5d855ece765648a31" + integrity sha512-psb6xjLj47+fE76JdZwskvwG4MYsQKXUtMsPh6U0YMvmyjRtKRFcxnlXGWglNybtNTNVmGdp94K62/+NjF5FDQ== + dependencies: + a-sync-waterfall "^1.0.0" + asap "^2.0.3" + commander "^5.1.0" + nwsapi@^2.0.7, nwsapi@^2.2.0: version "2.2.0" resolved "https://registry.npmjs.org/nwsapi/-/nwsapi-2.2.0.tgz#204879a9e3d068ff2a55139c2c772780681a38b7" From 9b3c9c16d54a7bf191c0abf86ca64d61a6257a02 Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 29 Jun 2021 16:02:48 +0200 Subject: [PATCH 02/35] chore: updating stuff Signed-off-by: blam --- .../scaffolder/actions/builtin/fetch/template.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 ea376e76ab..445f36acbc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -90,24 +90,26 @@ export function createFetchTemplateAction(options: { ctx.input.values, ); + // Grab some files const allFilesInTemplates = await globby(`*`, { cwd: templateDir }); + + // Nice for Cookiecutter compat nunjucks.installJinjaCompat(); + + // Create a templater const templater = nunjucks.configure({ tags: { - variableStart: '${', - variableEnd: '}', + variableStart: '${{', + variableEnd: '}}', }, autoescape: false, }); + // Need to work out how to autoescape but not this templater.addFilter('jsonify', s => JSON.stringify(s)); for (const location of allFilesInTemplates) { - console.log(location); - const filepath = templater.renderString(location, ctx.input.values); - console.log(filepath); - console.log('is', resolvePath(outputPath, filepath)); await fs.writeFile( resolvePath(outputPath, filepath), templater.renderString( From 01e13e173d49829b92be42ca49eb065ea1a6304b Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Fri, 2 Jul 2021 10:05:22 +0100 Subject: [PATCH 03/35] Scaffolder: add some todos and comments to fetch:template action Co-authored-by: Himanshu Mishra Signed-off-by: Mike Lewis --- .../actions/builtin/fetch/template.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) 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 445f36acbc..705f7aa138 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -52,7 +52,7 @@ export function createFetchTemplateAction(options: { targetPath: { title: 'Target Path', description: - 'Target path within the working directory to download the contents to.', + 'Target path within the working directory to download the contents to. Defaults to the working directory root.', type: 'string', }, values: { @@ -60,6 +60,10 @@ export function createFetchTemplateAction(options: { description: 'Values to pass on to the templating engine', type: 'object', }, + // TODO(mtlewis/orkohunter): add copyWithoutRender support + // + // TODO(mtlewis/orkohunter): do we need to replicate the template extensions support + // from fetch:cookiecutter? }, }, }, @@ -85,15 +89,21 @@ export function createFetchTemplateAction(options: { outputPath: templateDir, }); + // at this point the templateDir contains the unprocessed contents of the skeleton directory + ctx.logger.info( 'Fetched template, beginning templating process with values', ctx.input.values, ); // Grab some files + // + // TODO(mtlewis/orkohunter) handle subdirectories const allFilesInTemplates = await globby(`*`, { cwd: templateDir }); // Nice for Cookiecutter compat + // + // TODO(mtlewis/orkohunter): parameterize all jinja2/cookiecutter compat nunjucks.installJinjaCompat(); // Create a templater @@ -105,11 +115,14 @@ export function createFetchTemplateAction(options: { autoescape: false, }); - // Need to work out how to autoescape but not this + // TODO(mtlewis/orkohunter) Need to work out how to autoescape but not this templater.addFilter('jsonify', s => JSON.stringify(s)); for (const location of allFilesInTemplates) { + // handle variables in filenames const filepath = templater.renderString(location, ctx.input.values); + + // write file await fs.writeFile( resolvePath(outputPath, filepath), templater.renderString( @@ -118,6 +131,8 @@ export function createFetchTemplateAction(options: { ), ); } + + // TODO(mtlewis/orkohunter) log success }, }); } From 333db5f4dda6c3787277c77fb302a090a12e951c Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Fri, 2 Jul 2021 11:31:19 +0200 Subject: [PATCH 04/35] scaffolder: fix templating all files in sub directories for nunjucks action Co-authored-by: Mike Lewis Signed-off-by: Himanshu Mishra --- .../src/scaffolder/actions/builtin/fetch/template.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 705f7aa138..93a158ca37 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -98,8 +98,8 @@ export function createFetchTemplateAction(options: { // Grab some files // - // TODO(mtlewis/orkohunter) handle subdirectories - const allFilesInTemplates = await globby(`*`, { cwd: templateDir }); + // TODO(mtlewis/orkohunter) test whether empty directories are templated + const allFilesInTemplates = await globby(`**/*`, { cwd: templateDir }); // Nice for Cookiecutter compat // @@ -123,7 +123,7 @@ export function createFetchTemplateAction(options: { const filepath = templater.renderString(location, ctx.input.values); // write file - await fs.writeFile( + await fs.outputFile( resolvePath(outputPath, filepath), templater.renderString( await fs.readFile(resolvePath(templateDir, location), 'utf-8'), From 3fc8f9ff5b242369312ac14d866b708c757cafd4 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Fri, 2 Jul 2021 12:19:17 +0200 Subject: [PATCH 05/35] scaffolder: add some explanations and todos on nunjucks action Co-authored-by: Mike Lewis Signed-off-by: Himanshu Mishra --- .../src/scaffolder/actions/builtin/fetch/template.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 93a158ca37..5a12191010 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -108,14 +108,19 @@ export function createFetchTemplateAction(options: { // Create a templater const templater = nunjucks.configure({ + // TODO(mtlewis/orkohunter): Document Why we are changing the literals? Not here, but on scaffolder docs. ADR? tags: { variableStart: '${{', variableEnd: '}}', }, + // We don't want this builtin auto-escaping since it is escaping as HTML which will often be incorrect e.g. adds things like " autoescape: false, }); - // TODO(mtlewis/orkohunter) Need to work out how to autoescape but not this + // TODO(mtlewis/orkohunter) Evaluate whether this behavior is still appropriate when using nunjucks. + // As of now jsonify seems to be the most reliable way to do escaping, + // but is there a builtin filter to do this inside nunjucks + // (other than `autoescape` inside `configure` which escapes strings as HTML, which isn't right.). templater.addFilter('jsonify', s => JSON.stringify(s)); for (const location of allFilesInTemplates) { @@ -131,8 +136,6 @@ export function createFetchTemplateAction(options: { ), ); } - - // TODO(mtlewis/orkohunter) log success }, }); } From cc43a3135eafea5324fd4022eaca5da67d4caef8 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Fri, 2 Jul 2021 18:14:08 +0100 Subject: [PATCH 06/35] scaffolder: stop ignoring dotfiles in template directories globby ignores files/directories starting with a dot by default, which we don't want. cf. https://github.com/mrmlnc/fast-glob#dot Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 5a12191010..ee33465d5b 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -99,7 +99,10 @@ export function createFetchTemplateAction(options: { // Grab some files // // TODO(mtlewis/orkohunter) test whether empty directories are templated - const allFilesInTemplates = await globby(`**/*`, { cwd: templateDir }); + const allFilesInTemplates = await globby(`**/*`, { + cwd: templateDir, + dot: true, + }); // Nice for Cookiecutter compat // From cc8cf9e89c2e1cf4216cfd57d262d22122e8d574 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Fri, 2 Jul 2021 18:17:43 +0100 Subject: [PATCH 07/35] scaffolder: add support for copyWithoutRender to fetch:template Signed-off-by: Mike Lewis --- .../actions/builtin/fetch/template.ts | 61 +++++++++++++++---- 1 file changed, 49 insertions(+), 12 deletions(-) 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 ee33465d5b..4f6f20cedb 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -34,6 +34,7 @@ export function createFetchTemplateAction(options: { url: string; targetPath?: string; values: any; + copyWithoutRender?: string[]; }>({ id: 'fetch:template', description: @@ -60,8 +61,15 @@ export function createFetchTemplateAction(options: { description: 'Values to pass on to the templating engine', type: 'object', }, - // TODO(mtlewis/orkohunter): add copyWithoutRender support - // + copyWithoutRender: { + title: 'Copy Without Render', + description: + 'Avoid rendering directories and files in the template', + type: 'array', + items: { + type: 'string', + }, + }, // TODO(mtlewis/orkohunter): do we need to replicate the template extensions support // from fetch:cookiecutter? }, @@ -69,18 +77,28 @@ export function createFetchTemplateAction(options: { }, async handler(ctx) { ctx.logger.info('Fetching template content from remote URL'); + const workDir = await ctx.createTemporaryDirectory(); const templateDir = resolvePath(workDir, 'template'); - // Finally move the template result into the task workspace const targetPath = ctx.input.targetPath ?? './'; const outputPath = path.resolve(ctx.workspacePath, targetPath); + if (!outputPath.startsWith(ctx.workspacePath)) { throw new InputError( `Fetch action targetPath may not specify a path outside the working directory`, ); } + if ( + ctx.input.copyWithoutRender && + !Array.isArray(ctx.input.copyWithoutRender) + ) { + throw new InputError( + 'Fetch action input copyWithoutRender must be an Array', + ); + } + await fetchContents({ reader, integrations, @@ -103,6 +121,15 @@ export function createFetchTemplateAction(options: { cwd: templateDir, dot: true, }); + const nonTemplatedFiles = new Set( + ( + await Promise.all( + (ctx.input.copyWithoutRender || []).map(pattern => + globby(pattern, { cwd: templateDir, dot: true }), + ), + ) + ).flat(), + ); // Nice for Cookiecutter compat // @@ -116,7 +143,9 @@ export function createFetchTemplateAction(options: { variableStart: '${{', variableEnd: '}}', }, - // We don't want this builtin auto-escaping since it is escaping as HTML which will often be incorrect e.g. adds things like " + // We don't want this builtin auto-escaping, since uses HTML escape sequences + // like `"` - the correct way to escape strings in our case depends on + // the file type. autoescape: false, }); @@ -127,16 +156,24 @@ export function createFetchTemplateAction(options: { templater.addFilter('jsonify', s => JSON.stringify(s)); for (const location of allFilesInTemplates) { - // handle variables in filenames - const filepath = templater.renderString(location, ctx.input.values); + const isTemplated = !nonTemplatedFiles.has(location); + const outputFilePath = resolvePath( + outputPath, + isTemplated + ? templater.renderString(location, ctx.input.values) + : location, + ); + + const inputFileContents = await fs.readFile( + resolvePath(templateDir, location), + 'utf-8', + ); - // write file await fs.outputFile( - resolvePath(outputPath, filepath), - templater.renderString( - await fs.readFile(resolvePath(templateDir, location), 'utf-8'), - ctx.input.values, - ), + outputFilePath, + isTemplated + ? templater.renderString(inputFileContents, ctx.input.values) + : inputFileContents, ); } }, From 161e9e659bed11a0d199d5ded8929c800dea2290 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Fri, 2 Jul 2021 19:00:33 +0100 Subject: [PATCH 08/35] scaffolder: create empty directories in fetch:template action Signed-off-by: Mike Lewis --- .../actions/builtin/fetch/template.ts | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) 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 4f6f20cedb..6d906284bf 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -82,9 +82,9 @@ export function createFetchTemplateAction(options: { const templateDir = resolvePath(workDir, 'template'); const targetPath = ctx.input.targetPath ?? './'; - const outputPath = path.resolve(ctx.workspacePath, targetPath); + const outputDir = path.resolve(ctx.workspacePath, targetPath); - if (!outputPath.startsWith(ctx.workspacePath)) { + if (!outputDir.startsWith(ctx.workspacePath)) { throw new InputError( `Fetch action targetPath may not specify a path outside the working directory`, ); @@ -115,17 +115,22 @@ export function createFetchTemplateAction(options: { ); // Grab some files - // - // TODO(mtlewis/orkohunter) test whether empty directories are templated - const allFilesInTemplates = await globby(`**/*`, { + const allEntriesInTemplate = await globby(`**/*`, { cwd: templateDir, dot: true, + onlyFiles: false, + markDirectories: true, }); - const nonTemplatedFiles = new Set( + const nonTemplatedEntries = new Set( ( await Promise.all( (ctx.input.copyWithoutRender || []).map(pattern => - globby(pattern, { cwd: templateDir, dot: true }), + globby(pattern, { + cwd: templateDir, + dot: true, + onlyFiles: false, + markDirectories: true, + }), ), ) ).flat(), @@ -155,26 +160,30 @@ export function createFetchTemplateAction(options: { // (other than `autoescape` inside `configure` which escapes strings as HTML, which isn't right.). templater.addFilter('jsonify', s => JSON.stringify(s)); - for (const location of allFilesInTemplates) { - const isTemplated = !nonTemplatedFiles.has(location); - const outputFilePath = resolvePath( - outputPath, + for (const location of allEntriesInTemplate) { + const isTemplated = !nonTemplatedEntries.has(location); + const outputPath = resolvePath( + outputDir, isTemplated ? templater.renderString(location, ctx.input.values) : location, ); - const inputFileContents = await fs.readFile( - resolvePath(templateDir, location), - 'utf-8', - ); + if (location.endsWith('/')) { + await fs.ensureDir(outputPath); + } else { + const inputFileContents = await fs.readFile( + resolvePath(templateDir, location), + 'utf-8', + ); - await fs.outputFile( - outputFilePath, - isTemplated - ? templater.renderString(inputFileContents, ctx.input.values) - : inputFileContents, - ); + await fs.outputFile( + outputPath, + isTemplated + ? templater.renderString(inputFileContents, ctx.input.values) + : inputFileContents, + ); + } } }, }); From 1caf2a24f4cda8c8e5ba6ac996657f240157dc3a Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Fri, 2 Jul 2021 19:01:11 +0100 Subject: [PATCH 09/35] scaffolder: add compat TODO Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.ts | 2 ++ 1 file changed, 2 insertions(+) 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 6d906284bf..adb45c45c3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -139,6 +139,8 @@ export function createFetchTemplateAction(options: { // Nice for Cookiecutter compat // // TODO(mtlewis/orkohunter): parameterize all jinja2/cookiecutter compat + // TODO(mtlewis): introduce "cookiecutter" prefix for input variables when + // compat is enabled? nunjucks.installJinjaCompat(); // Create a templater From 5144e7617d18dd860735a4833e6202dcda539f86 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:03:42 +0100 Subject: [PATCH 10/35] scaffolder: input type from fetch:template action factory Signed-off-by: Mike Lewis --- .../scaffolder/actions/builtin/fetch/template.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) 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 adb45c45c3..f4cfacb175 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -24,18 +24,20 @@ import globby from 'globby'; import nunjucks from 'nunjucks'; import fs from 'fs-extra'; +export type FetchTemplateInput = { + url: string; + targetPath?: string; + values: any; + copyWithoutRender?: string[]; +}; + export function createFetchTemplateAction(options: { reader: UrlReader; integrations: ScmIntegrations; }) { const { reader, integrations } = options; - return createTemplateAction<{ - url: string; - targetPath?: string; - values: any; - copyWithoutRender?: string[]; - }>({ + return createTemplateAction({ id: 'fetch:template', description: "Downloads a skeleton and will template variables into the skeleton and places the result in the workspace, or optionally in a subdirectory specified by the 'targetPath' input option.", From 9fba3e22b18c2b4bc8db7e9e63b2f77af5c0cfc6 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:02:57 +0100 Subject: [PATCH 11/35] scaffolder: add test suite for fetch:template Signed-off-by: Mike Lewis --- .../actions/builtin/fetch/template.test.ts | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts new file mode 100644 index 0000000000..2bced11900 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -0,0 +1,221 @@ +/* + * 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 fs from 'fs-extra'; +import mockFs from 'mock-fs'; +import { getVoidLogger, UrlReader } from '@backstage/backend-common'; +import { ScmIntegrations } from '@backstage/integration'; +import { PassThrough } from 'stream'; +import { fetchContents } from './helpers'; +import { ActionContext, TemplateAction } from '../../types'; +import { createFetchTemplateAction, FetchTemplateInput } from './template'; + +jest.mock('./helpers', () => ({ + fetchContents: jest.fn(), +})); + +const mockFetchContents = fetchContents as jest.MockedFunction< + typeof fetchContents +>; + +describe('fetch:template', () => { + let action: TemplateAction; + + const workspacePath = os.tmpdir(); + const createTemporaryDirectory: jest.MockedFunction< + ActionContext['createTemporaryDirectory'] + > = jest.fn(() => + Promise.resolve( + `${workspacePath}/${createTemporaryDirectory.mock.calls.length}`, + ), + ); + + const logger = getVoidLogger(); + + const mockContext = (inputPatch: Partial = {}) => ({ + baseUrl: 'base-url', + input: { + url: './skeleton', + targetPath: './target', + values: { + test: 'value', + }, + ...inputPatch, + }, + output: jest.fn(), + logStream: new PassThrough(), + logger, + workspacePath, + createTemporaryDirectory, + }); + + beforeEach(() => { + mockFs(); + + action = createFetchTemplateAction({ + reader: (Symbol('UrlReader') as unknown) as UrlReader, + integrations: (Symbol('Integrations') as unknown) as ScmIntegrations, + }); + }); + + afterEach(() => { + mockFs.restore(); + }); + + it(`returns a TemplateAction with the id 'fetch:template'`, () => { + expect(action.id).toEqual('fetch:template'); + }); + + describe('handler', () => { + it('throws if output directory is outside the workspace', async () => { + await expect(() => + action.handler(mockContext({ targetPath: '../' })), + ).rejects.toThrowError(/outside the working directory/i); + }); + + it('throws if copyWithoutRender parameter is not an array', async () => { + await expect(() => + action.handler( + mockContext({ copyWithoutRender: ('abc' as unknown) as string[] }), + ), + ).rejects.toThrowError(/copyWithoutRender must be an array/i); + }); + + describe('with valid input', () => { + let context: ActionContext; + + beforeEach(async () => { + context = mockContext({ + values: { + name: 'test-project', + count: 1234, + itemList: ['first', 'second', 'third'], + }, + }); + + mockFetchContents.mockImplementation(({ outputPath }) => { + mockFs({ + [outputPath]: { + 'empty-dir-${{ count }}': {}, + 'static.txt': 'static content', + '${{ name }}.txt': 'static content', + subdir: { + 'templated-content.txt': '${{ name }}: ${{ count }}', + }, + '.${{ name }}': '${{ itemList | dump }}', + }, + }); + + return Promise.resolve(); + }); + + await action.handler(context); + }); + + it('uses fetchContents to retrieve the template content', () => { + expect(mockFetchContents).toHaveBeenCalledWith( + expect.objectContaining({ + baseUrl: context.baseUrl, + fetchUrl: context.input.url, + }), + ); + }); + + it('copies files with no templating in names or content successfully', async () => { + await expect( + fs.readFile(`${workspacePath}/target/static.txt`, 'utf-8'), + ).resolves.toEqual('static content'); + }); + + it('copies files with templated names successfully', async () => { + await expect( + fs.readFile(`${workspacePath}/target/test-project.txt`, 'utf-8'), + ).resolves.toEqual('static content'); + }); + + it('copies files with templated content successfully', async () => { + await expect( + fs.readFile( + `${workspacePath}/target/subdir/templated-content.txt`, + 'utf-8', + ), + ).resolves.toEqual('test-project: 1234'); + }); + + it('processes dotfiles', async () => { + await expect( + fs.readFile(`${workspacePath}/target/.test-project`, 'utf-8'), + ).resolves.toEqual('["first","second","third"]'); + }); + + it('copies empty directories', async () => { + await expect( + fs.readdir(`${workspacePath}/target/empty-dir-1234`, 'utf-8'), + ).resolves.toEqual([]); + }); + }); + + describe('copyWithoutRender', () => { + let context: ActionContext; + + beforeEach(async () => { + context = mockContext({ + values: { + name: 'test-project', + count: 1234, + }, + copyWithoutRender: ['.unprocessed'], + }); + + mockFetchContents.mockImplementation(({ outputPath }) => { + mockFs({ + [outputPath]: { + processed: { + 'templated-content-${{ name }}.txt': '${{ count }}', + }, + '.unprocessed': { + 'templated-content-${{ name }}.txt': '${{ count }}', + }, + }, + }); + + return Promise.resolve(); + }); + + await action.handler(context); + }); + + it('ignores template syntax in files matched in copyWithoutRender', async () => { + await expect( + fs.readFile( + `${workspacePath}/target/.unprocessed/templated-content-\${{ name }}.txt`, + 'utf-8', + ), + ).resolves.toEqual('${{ count }}'); + }); + + it('processes files not matched in copyWithoutRender', async () => { + await expect( + fs.readFile( + `${workspacePath}/target/processed/templated-content-test-project.txt`, + 'utf-8', + ), + ).resolves.toEqual('1234'); + }); + }); + }); +}); From a8ded61923d8010d0b0f9514b33b34896c4e6bc6 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:30:55 +0100 Subject: [PATCH 12/35] scaffolder: add cookiecutter compat mode to fetch:template Signed-off-by: Mike Lewis --- .../actions/builtin/fetch/template.test.ts | 54 ++++++++++++++ .../actions/builtin/fetch/template.ts | 73 +++++++++++++------ 2 files changed, 106 insertions(+), 21 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 2bced11900..2fa00336df 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -218,4 +218,58 @@ describe('fetch:template', () => { }); }); }); + + describe('cookiecutter compatibility mode', () => { + let context: ActionContext; + + beforeEach(async () => { + context = mockContext({ + values: { + name: 'test-project', + count: 1234, + itemList: ['first', 'second', 'third'], + }, + cookieCutterCompat: true, + }); + + mockFetchContents.mockImplementation(({ outputPath }) => { + mockFs({ + [outputPath]: { + '{{ cookiecutter.name }}.txt': 'static content', + subdir: { + 'templated-content.txt': + '{{ cookiecutter.name }}: {{ cookiecutter.count }}', + }, + '{{ cookiecutter.name }}.json': + '{{ cookiecutter.itemList | jsonify }}', + }, + }); + + return Promise.resolve(); + }); + + await action.handler(context); + }); + + it('copies files with cookiecutter-style templated names successfully', async () => { + await expect( + fs.readFile(`${workspacePath}/target/test-project.txt`, 'utf-8'), + ).resolves.toEqual('static content'); + }); + + it('copies files with cookiecutter-style templated content successfully', async () => { + await expect( + fs.readFile( + `${workspacePath}/target/subdir/templated-content.txt`, + 'utf-8', + ), + ).resolves.toEqual('test-project: 1234'); + }); + + it('includes the jsonify filter', async () => { + await expect( + fs.readFile(`${workspacePath}/target/test-project.json`, 'utf-8'), + ).resolves.toEqual('["first","second","third"]'); + }); + }); }); 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 f4cfacb175..e2f0047542 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -24,11 +24,25 @@ import globby from 'globby'; import nunjucks from 'nunjucks'; import fs from 'fs-extra'; +/* + * Maximise compatibility with Jinja (and therefore cookiecutter) + * using nunjucks jinja compat mode. Since this method mutates + * the global nunjucks instance, we can't enable this per-template, + * or only for templates with cookiecutter compat enabled, so the + * next best option is to explicitly enable it globally and allow + * folks to rely on jinja compatibility behaviour in fetch:template + * templates if they wish. + * + * cf. https://mozilla.github.io/nunjucks/api.html#installjinjacompat + */ +nunjucks.installJinjaCompat(); + export type FetchTemplateInput = { url: string; targetPath?: string; values: any; copyWithoutRender?: string[]; + cookieCutterCompat?: boolean; }; export function createFetchTemplateAction(options: { @@ -72,6 +86,13 @@ export function createFetchTemplateAction(options: { type: 'string', }, }, + cookieCutterCompat: { + title: 'Cookiecutter compatibility mode', + // TODO(mtlewis): documentation for cookiecutter compat mode + description: + 'Enable features to maximise compatibility with templates built for fetch:cookiecutter', + type: 'boolean', + }, // TODO(mtlewis/orkohunter): do we need to replicate the template extensions support // from fetch:cookiecutter? }, @@ -109,8 +130,6 @@ export function createFetchTemplateAction(options: { outputPath: templateDir, }); - // at this point the templateDir contains the unprocessed contents of the skeleton directory - ctx.logger.info( 'Fetched template, beginning templating process with values', ctx.input.values, @@ -138,39 +157,51 @@ export function createFetchTemplateAction(options: { ).flat(), ); - // Nice for Cookiecutter compat - // - // TODO(mtlewis/orkohunter): parameterize all jinja2/cookiecutter compat - // TODO(mtlewis): introduce "cookiecutter" prefix for input variables when - // compat is enabled? - nunjucks.installJinjaCompat(); + ctx.logger.info(allEntriesInTemplate); // Create a templater const templater = nunjucks.configure({ // TODO(mtlewis/orkohunter): Document Why we are changing the literals? Not here, but on scaffolder docs. ADR? - tags: { - variableStart: '${{', - variableEnd: '}}', - }, + ...(ctx.input.cookieCutterCompat + ? {} + : { + tags: { + variableStart: '${{', + variableEnd: '}}', + }, + }), // We don't want this builtin auto-escaping, since uses HTML escape sequences // like `"` - the correct way to escape strings in our case depends on // the file type. autoescape: false, }); - // TODO(mtlewis/orkohunter) Evaluate whether this behavior is still appropriate when using nunjucks. - // As of now jsonify seems to be the most reliable way to do escaping, - // but is there a builtin filter to do this inside nunjucks - // (other than `autoescape` inside `configure` which escapes strings as HTML, which isn't right.). - templater.addFilter('jsonify', s => JSON.stringify(s)); + if (ctx.input.cookieCutterCompat) { + // The "jsonify" filter built into cookiecutter is common + // in fetch:cookiecutter templates, so when compat mode + // is enabled we alias the "dump" filter from nunjucks as + // jsonify. Dump accepts an optional `spaces` parameter + // which enables indented output, but when this parameter + // is not supplied it works identically to jsonify. + // + // cf. https://cookiecutter.readthedocs.io/en/latest/advanced/template_extensions.html?highlight=jsonify#jsonify-extension + // cf. https://mozilla.github.io/nunjucks/templating.html#dump + templater.addFilter('jsonify', templater.getFilter('dump')); + } + + // Cookiecutter prefixes all parameters in templates with + // `cookiecutter.`. To replicate this, we wrap our parameters + // in an object with a `cookiecutter` property when compat + // mode is enabled. + const parameters = ctx.input.cookieCutterCompat + ? { cookiecutter: ctx.input.values } + : ctx.input.values; for (const location of allEntriesInTemplate) { const isTemplated = !nonTemplatedEntries.has(location); const outputPath = resolvePath( outputDir, - isTemplated - ? templater.renderString(location, ctx.input.values) - : location, + isTemplated ? templater.renderString(location, parameters) : location, ); if (location.endsWith('/')) { @@ -184,7 +215,7 @@ export function createFetchTemplateAction(options: { await fs.outputFile( outputPath, isTemplated - ? templater.renderString(inputFileContents, ctx.input.values) + ? templater.renderString(inputFileContents, parameters) : inputFileContents, ); } From a26bc320327fe179906d2488ef3c45699bb256a3 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:35:13 +0100 Subject: [PATCH 13/35] scaffolder: update api-report.md Signed-off-by: Mike Lewis --- plugins/scaffolder-backend/api-report.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/plugins/scaffolder-backend/api-report.md b/plugins/scaffolder-backend/api-report.md index 7528388a47..1f3415cc23 100644 --- a/plugins/scaffolder-backend/api-report.md +++ b/plugins/scaffolder-backend/api-report.md @@ -143,6 +143,12 @@ export function createFetchPlainAction(options: { integrations: ScmIntegrations; }): TemplateAction; +// @public (undocumented) +export function createFetchTemplateAction(options: { + reader: UrlReader; + integrations: ScmIntegrations; +}): TemplateAction; + // @public (undocumented) export const createFilesystemDeleteAction: () => TemplateAction; From ab61778b877bce5d90fd32a49f2290e5ec48c1ce Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:36:55 +0100 Subject: [PATCH 14/35] scaffolder: remove fetch:template extensions todo cf. https://github.com/backstage/backstage/pull/6322/commits/944fda210ea807f52ece3272736b090ecd1a3a57#r663007257 Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.ts | 2 -- 1 file changed, 2 deletions(-) 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 e2f0047542..38771292d4 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -93,8 +93,6 @@ export function createFetchTemplateAction(options: { 'Enable features to maximise compatibility with templates built for fetch:cookiecutter', type: 'boolean', }, - // TODO(mtlewis/orkohunter): do we need to replicate the template extensions support - // from fetch:cookiecutter? }, }, }, From b4680036b60b1ea38abfb432a829e028df8a7a64 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:52:04 +0100 Subject: [PATCH 15/35] scaffolder: adjust case of cookieCutterCompat flag in fetch:template Realized that `cookiecutter` is more idiomatic than `cookieCutter`, so renamed the `cookieCutterCompat` flag on the new `fetch:template` action to `cookiecutterCompat`. Signed-off-by: Mike Lewis --- .../scaffolder/actions/builtin/fetch/template.test.ts | 2 +- .../src/scaffolder/actions/builtin/fetch/template.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 2fa00336df..48dcb8a06a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -229,7 +229,7 @@ describe('fetch:template', () => { count: 1234, itemList: ['first', 'second', 'third'], }, - cookieCutterCompat: true, + cookiecutterCompat: true, }); mockFetchContents.mockImplementation(({ outputPath }) => { 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 38771292d4..54e9888184 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -42,7 +42,7 @@ export type FetchTemplateInput = { targetPath?: string; values: any; copyWithoutRender?: string[]; - cookieCutterCompat?: boolean; + cookiecutterCompat?: boolean; }; export function createFetchTemplateAction(options: { @@ -86,7 +86,7 @@ export function createFetchTemplateAction(options: { type: 'string', }, }, - cookieCutterCompat: { + cookiecutterCompat: { title: 'Cookiecutter compatibility mode', // TODO(mtlewis): documentation for cookiecutter compat mode description: @@ -160,7 +160,7 @@ export function createFetchTemplateAction(options: { // Create a templater const templater = nunjucks.configure({ // TODO(mtlewis/orkohunter): Document Why we are changing the literals? Not here, but on scaffolder docs. ADR? - ...(ctx.input.cookieCutterCompat + ...(ctx.input.cookiecutterCompat ? {} : { tags: { @@ -174,7 +174,7 @@ export function createFetchTemplateAction(options: { autoescape: false, }); - if (ctx.input.cookieCutterCompat) { + if (ctx.input.cookiecutterCompat) { // The "jsonify" filter built into cookiecutter is common // in fetch:cookiecutter templates, so when compat mode // is enabled we alias the "dump" filter from nunjucks as @@ -191,7 +191,7 @@ export function createFetchTemplateAction(options: { // `cookiecutter.`. To replicate this, we wrap our parameters // in an object with a `cookiecutter` property when compat // mode is enabled. - const parameters = ctx.input.cookieCutterCompat + const parameters = ctx.input.cookiecutterCompat ? { cookiecutter: ctx.input.values } : ctx.input.values; From b3583b4108b3872f0da0fff65a96513ae8011b39 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 17:57:23 +0100 Subject: [PATCH 16/35] scaffolder: move @types/nunjucks to devDeps Signed-off-by: Mike Lewis --- plugins/scaffolder-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index 4ce37999d6..d7829197e3 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -40,7 +40,6 @@ "@octokit/rest": "^18.5.3", "@types/express": "^4.17.6", "@types/git-url-parse": "^9.0.0", - "@types/nunjucks": "^3.1.4", "azure-devops-node-api": "^10.1.1", "command-exists": "^1.2.9", "compression": "^1.7.4", @@ -71,6 +70,7 @@ "@types/command-exists": "^1.2.0", "@types/fs-extra": "^9.0.1", "@types/mock-fs": "^4.13.0", + "@types/nunjucks": "^3.1.4", "@types/supertest": "^2.0.8", "jest-when": "^3.1.0", "mock-fs": "^4.13.0", From ef07bce298a1fbf8ece1b27635fa9c8504561ca6 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Mon, 5 Jul 2021 18:10:58 +0100 Subject: [PATCH 17/35] scaffolder: improve logging in fetch:template Signed-off-by: Mike Lewis --- .../actions/builtin/fetch/template.ts | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) 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 54e9888184..0c9b997746 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -128,18 +128,14 @@ export function createFetchTemplateAction(options: { outputPath: templateDir, }); - ctx.logger.info( - 'Fetched template, beginning templating process with values', - ctx.input.values, - ); - - // Grab some files + ctx.logger.info('Listing files and directories in template'); const allEntriesInTemplate = await globby(`**/*`, { cwd: templateDir, dot: true, onlyFiles: false, markDirectories: true, }); + const nonTemplatedEntries = new Set( ( await Promise.all( @@ -155,15 +151,13 @@ export function createFetchTemplateAction(options: { ).flat(), ); - ctx.logger.info(allEntriesInTemplate); - // Create a templater const templater = nunjucks.configure({ - // TODO(mtlewis/orkohunter): Document Why we are changing the literals? Not here, but on scaffolder docs. ADR? ...(ctx.input.cookiecutterCompat ? {} : { tags: { + // TODO(mtlewis/orkohunter): Document Why we are changing the literals? Not here, but on scaffolder docs. ADR? variableStart: '${{', variableEnd: '}}', }, @@ -195,6 +189,11 @@ export function createFetchTemplateAction(options: { ? { cookiecutter: ctx.input.values } : ctx.input.values; + ctx.logger.info( + `Processing ${allEntriesInTemplate.length} template files/directories with input values`, + ctx.input.values, + ); + for (const location of allEntriesInTemplate) { const isTemplated = !nonTemplatedEntries.has(location); const outputPath = resolvePath( @@ -202,6 +201,12 @@ export function createFetchTemplateAction(options: { isTemplated ? templater.renderString(location, parameters) : location, ); + ctx.logger.info( + `Writing${isTemplated ? ' ' : ' un-templated '}${ + location.endsWith('/') ? 'directory' : 'file' + } ${location} to template output path`, + ); + if (location.endsWith('/')) { await fs.ensureDir(outputPath); } else { @@ -218,6 +223,8 @@ export function createFetchTemplateAction(options: { ); } } + + ctx.logger.info(`Template result written to ${outputDir}`); }, }); } From a8c83700ebf0269cc155824ae7444ff99be60f0d Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 6 Jul 2021 13:51:59 +0200 Subject: [PATCH 18/35] fetch:template - skip running templater on binary files Co-authored-by: Mike Lewis Signed-off-by: Himanshu Mishra --- plugins/scaffolder-backend/package.json | 1 + .../src/scaffolder/actions/builtin/fetch/template.ts | 11 ++++++++++- yarn.lock | 5 +++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index d7829197e3..534fcba169 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -52,6 +52,7 @@ "globby": "^11.0.0", "handlebars": "^4.7.6", "helmet": "^4.0.0", + "isbinaryfile": "^4.0.8", "isomorphic-git": "^1.8.0", "jsonschema": "^1.2.6", "knex": "^0.95.1", 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 0c9b997746..58a14a3da5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -23,6 +23,7 @@ import { createTemplateAction } from '../../createTemplateAction'; import globby from 'globby'; import nunjucks from 'nunjucks'; import fs from 'fs-extra'; +import { isBinaryFile } from 'isbinaryfile'; /* * Maximise compatibility with Jinja (and therefore cookiecutter) @@ -196,6 +197,7 @@ export function createFetchTemplateAction(options: { for (const location of allEntriesInTemplate) { const isTemplated = !nonTemplatedEntries.has(location); + const outputPath = resolvePath( outputDir, isTemplated ? templater.renderString(location, parameters) : location, @@ -215,9 +217,16 @@ export function createFetchTemplateAction(options: { 'utf-8', ); + const isBinary = await isBinaryFile(Buffer.from(inputFileContents)); + if (isBinary) { + ctx.logger.info( + `Not running templater on contents of ${location} since it is a binary file.`, + ); + } + await fs.outputFile( outputPath, - isTemplated + isTemplated && !isBinary ? templater.renderString(inputFileContents, parameters) : inputFileContents, ); diff --git a/yarn.lock b/yarn.lock index 96299709e4..accbe9389c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15996,6 +15996,11 @@ isarray@^2.0.5: resolved "https://registry.npmjs.org/isarray/-/isarray-2.0.5.tgz#8af1e4c1221244cc62459faf38940d4e644a5723" integrity sha512-xHjhDr3cNBK0BzdUJSPXZntQUx/mwMS5Rw4A7lPJ90XGAO6ISP/ePDNuo0vhqOZU+UD5JoodwCAAoZQd3FeAKw== +isbinaryfile@^4.0.8: + version "4.0.8" + resolved "https://registry.npmjs.org/isbinaryfile/-/isbinaryfile-4.0.8.tgz#5d34b94865bd4946633ecc78a026fc76c5b11fcf" + integrity sha512-53h6XFniq77YdW+spoRrebh0mnmTxRPTlcuIArO57lmMdq4uBKFKaeTjnb92oYWrSn/LVL+LT+Hap2tFQj8V+w== + isexe@^2.0.0: version "2.0.0" resolved "https://registry.npmjs.org/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10" From 5f7e7db19dd9834fbd4d3dfc97963a8e55056476 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 6 Jul 2021 14:47:53 +0200 Subject: [PATCH 19/35] fetch:template - binary files should be copied to output path as-is Co-authored-by: Mike Lewis Signed-off-by: Himanshu Mishra --- .../actions/builtin/fetch/template.ts | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) 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 58a14a3da5..fd2aeac70e 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -81,7 +81,7 @@ export function createFetchTemplateAction(options: { copyWithoutRender: { title: 'Copy Without Render', description: - 'Avoid rendering directories and files in the template', + 'An array of glob patterns. Any files or directories which match are copied without being processed as templates.', type: 'array', items: { type: 'string', @@ -203,33 +203,37 @@ export function createFetchTemplateAction(options: { isTemplated ? templater.renderString(location, parameters) : location, ); - ctx.logger.info( - `Writing${isTemplated ? ' ' : ' un-templated '}${ - location.endsWith('/') ? 'directory' : 'file' - } ${location} to template output path`, - ); + if (isTemplated) { + ctx.logger.info( + `Copying file/directory ${location} without processing since it matches a pattern in "copyWithoutRender".`, + ); + } if (location.endsWith('/')) { + ctx.logger.info( + `Writing directory ${location} to template output path.`, + ); await fs.ensureDir(outputPath); } else { - const inputFileContents = await fs.readFile( - resolvePath(templateDir, location), - 'utf-8', - ); + const inputFilePath = resolvePath(templateDir, location); - const isBinary = await isBinaryFile(Buffer.from(inputFileContents)); - if (isBinary) { + if (await isBinaryFile(inputFilePath)) { ctx.logger.info( - `Not running templater on contents of ${location} since it is a binary file.`, + `Copying binary file ${location} to template output path.`, + ); + await fs.copy(inputFilePath, outputPath); + } else { + ctx.logger.info( + `Writing file ${location} to template output path.`, + ); + const inputFileContents = await fs.readFile(inputFilePath, 'utf-8'); + await fs.outputFile( + outputPath, + isTemplated + ? templater.renderString(inputFileContents, parameters) + : inputFileContents, ); } - - await fs.outputFile( - outputPath, - isTemplated && !isBinary - ? templater.renderString(inputFileContents, parameters) - : inputFileContents, - ); } } From f6eff971cb04972723453cfcbf2754e8373216b6 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 6 Jul 2021 14:51:58 +0200 Subject: [PATCH 20/35] fetch:template - rename isTemplated to shouldCopyWithoutRender (fix logging bug) and add test todo Co-authored-by: Mike Lewis Signed-off-by: Himanshu Mishra --- .../actions/builtin/fetch/template.test.ts | 2 ++ .../scaffolder/actions/builtin/fetch/template.ts | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 48dcb8a06a..179626581a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -24,6 +24,8 @@ import { fetchContents } from './helpers'; import { ActionContext, TemplateAction } from '../../types'; import { createFetchTemplateAction, FetchTemplateInput } from './template'; +// TODO(mtlewis/orkohunter): Test handling binary files + jest.mock('./helpers', () => ({ fetchContents: jest.fn(), })); 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 fd2aeac70e..cbe74dc472 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -196,14 +196,16 @@ export function createFetchTemplateAction(options: { ); for (const location of allEntriesInTemplate) { - const isTemplated = !nonTemplatedEntries.has(location); + const shouldCopyWithoutRender = nonTemplatedEntries.has(location); const outputPath = resolvePath( outputDir, - isTemplated ? templater.renderString(location, parameters) : location, + shouldCopyWithoutRender + ? location + : templater.renderString(location, parameters), ); - if (isTemplated) { + if (shouldCopyWithoutRender) { ctx.logger.info( `Copying file/directory ${location} without processing since it matches a pattern in "copyWithoutRender".`, ); @@ -229,9 +231,9 @@ export function createFetchTemplateAction(options: { const inputFileContents = await fs.readFile(inputFilePath, 'utf-8'); await fs.outputFile( outputPath, - isTemplated - ? templater.renderString(inputFileContents, parameters) - : inputFileContents, + shouldCopyWithoutRender + ? inputFileContents + : templater.renderString(inputFileContents, parameters), ); } } From 6aeb2b93ded8dd84e03ff7e69a862ee08a5d7808 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 6 Jul 2021 15:48:14 +0200 Subject: [PATCH 21/35] fetch:template - test binary files are copied over Signed-off-by: Himanshu Mishra --- .../public/react-logo192.png | Bin 0 -> 5347 bytes .../actions/builtin/fetch/template.test.ts | 17 +++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 plugins/scaffolder-backend/fixtures/test-nested-template/public/react-logo192.png diff --git a/plugins/scaffolder-backend/fixtures/test-nested-template/public/react-logo192.png b/plugins/scaffolder-backend/fixtures/test-nested-template/public/react-logo192.png new file mode 100644 index 0000000000000000000000000000000000000000..fc44b0a3796c0e0a64c3d858ca038bd4570465d9 GIT binary patch literal 5347 zcmZWtbyO6NvR-oO24RV%BvuJ&=?+<7=`LvyB&A_#M7mSDYw1v6DJkiYl9XjT!%$dLEBTQ8R9|wd3008in6lFF3GV-6mLi?MoP_y~}QUnaDCHI#t z7w^m$@6DI)|C8_jrT?q=f8D?0AM?L)Z}xAo^e^W>t$*Y0KlT5=@bBjT9kxb%-KNdk zeOS1tKO#ChhG7%{ApNBzE2ZVNcxbrin#E1TiAw#BlUhXllzhN$qWez5l;h+t^q#Eav8PhR2|T}y5kkflaK`ba-eoE+Z2q@o6P$)=&` z+(8}+-McnNO>e#$Rr{32ngsZIAX>GH??tqgwUuUz6kjns|LjsB37zUEWd|(&O!)DY zQLrq%Y>)Y8G`yYbYCx&aVHi@-vZ3|ebG!f$sTQqMgi0hWRJ^Wc+Ibv!udh_r%2|U) zPi|E^PK?UE!>_4`f`1k4hqqj_$+d!EB_#IYt;f9)fBOumGNyglU(ofY`yHq4Y?B%- zp&G!MRY<~ajTgIHErMe(Z8JG*;D-PJhd@RX@QatggM7+G(Lz8eZ;73)72Hfx5KDOE zkT(m}i2;@X2AT5fW?qVp?@WgN$aT+f_6eo?IsLh;jscNRp|8H}Z9p_UBO^SJXpZew zEK8fz|0Th%(Wr|KZBGTM4yxkA5CFdAj8=QSrT$fKW#tweUFqr0TZ9D~a5lF{)%-tTGMK^2tz(y2v$i%V8XAxIywrZCp=)83p(zIk6@S5AWl|Oa2hF`~~^W zI;KeOSkw1O#TiQ8;U7OPXjZM|KrnN}9arP)m0v$c|L)lF`j_rpG(zW1Qjv$=^|p*f z>)Na{D&>n`jOWMwB^TM}slgTEcjxTlUby89j1)|6ydRfWERn3|7Zd2&e7?!K&5G$x z`5U3uFtn4~SZq|LjFVrz$3iln-+ucY4q$BC{CSm7Xe5c1J<=%Oagztj{ifpaZk_bQ z9Sb-LaQMKp-qJA*bP6DzgE3`}*i1o3GKmo2pn@dj0;He}F=BgINo};6gQF8!n0ULZ zL>kC0nPSFzlcB7p41doao2F7%6IUTi_+!L`MM4o*#Y#0v~WiO8uSeAUNp=vA2KaR&=jNR2iVwG>7t%sG2x_~yXzY)7K& zk3p+O0AFZ1eu^T3s};B%6TpJ6h-Y%B^*zT&SN7C=N;g|#dGIVMSOru3iv^SvO>h4M=t-N1GSLLDqVTcgurco6)3&XpU!FP6Hlrmj}f$ zp95;b)>M~`kxuZF3r~a!rMf4|&1=uMG$;h^g=Kl;H&Np-(pFT9FF@++MMEx3RBsK?AU0fPk-#mdR)Wdkj)`>ZMl#^<80kM87VvsI3r_c@_vX=fdQ`_9-d(xiI z4K;1y1TiPj_RPh*SpDI7U~^QQ?%0&!$Sh#?x_@;ag)P}ZkAik{_WPB4rHyW#%>|Gs zdbhyt=qQPA7`?h2_8T;-E6HI#im9K>au*(j4;kzwMSLgo6u*}-K`$_Gzgu&XE)udQ zmQ72^eZd|vzI)~!20JV-v-T|<4@7ruqrj|o4=JJPlybwMg;M$Ud7>h6g()CT@wXm` zbq=A(t;RJ^{Xxi*Ff~!|3!-l_PS{AyNAU~t{h;(N(PXMEf^R(B+ZVX3 z8y0;0A8hJYp@g+c*`>eTA|3Tgv9U8#BDTO9@a@gVMDxr(fVaEqL1tl?md{v^j8aUv zm&%PX4^|rX|?E4^CkplWWNv*OKM>DxPa z!RJ)U^0-WJMi)Ksc!^ixOtw^egoAZZ2Cg;X7(5xZG7yL_;UJ#yp*ZD-;I^Z9qkP`} zwCTs0*%rIVF1sgLervtnUo&brwz?6?PXRuOCS*JI-WL6GKy7-~yi0giTEMmDs_-UX zo=+nFrW_EfTg>oY72_4Z0*uG>MnXP=c0VpT&*|rvv1iStW;*^={rP1y?Hv+6R6bxFMkxpWkJ>m7Ba{>zc_q zEefC3jsXdyS5??Mz7IET$Kft|EMNJIv7Ny8ZOcKnzf`K5Cd)&`-fTY#W&jnV0l2vt z?Gqhic}l}mCv1yUEy$%DP}4AN;36$=7aNI^*AzV(eYGeJ(Px-j<^gSDp5dBAv2#?; zcMXv#aj>%;MiG^q^$0MSg-(uTl!xm49dH!{X0){Ew7ThWV~Gtj7h%ZD zVN-R-^7Cf0VH!8O)uUHPL2mO2tmE*cecwQv_5CzWeh)ykX8r5Hi`ehYo)d{Jnh&3p z9ndXT$OW51#H5cFKa76c<%nNkP~FU93b5h-|Cb}ScHs@4Q#|}byWg;KDMJ#|l zE=MKD*F@HDBcX@~QJH%56eh~jfPO-uKm}~t7VkHxHT;)4sd+?Wc4* z>CyR*{w@4(gnYRdFq=^(#-ytb^5ESD?x<0Skhb%Pt?npNW1m+Nv`tr9+qN<3H1f<% zZvNEqyK5FgPsQ`QIu9P0x_}wJR~^CotL|n zk?dn;tLRw9jJTur4uWoX6iMm914f0AJfB@C74a;_qRrAP4E7l890P&{v<}>_&GLrW z)klculcg`?zJO~4;BBAa=POU%aN|pmZJn2{hA!d!*lwO%YSIzv8bTJ}=nhC^n}g(ld^rn#kq9Z3)z`k9lvV>y#!F4e{5c$tnr9M{V)0m(Z< z#88vX6-AW7T2UUwW`g<;8I$Jb!R%z@rCcGT)-2k7&x9kZZT66}Ztid~6t0jKb&9mm zpa}LCb`bz`{MzpZR#E*QuBiZXI#<`5qxx=&LMr-UUf~@dRk}YI2hbMsAMWOmDzYtm zjof16D=mc`^B$+_bCG$$@R0t;e?~UkF?7<(vkb70*EQB1rfUWXh$j)R2)+dNAH5%R zEBs^?N;UMdy}V};59Gu#0$q53$}|+q7CIGg_w_WlvE}AdqoS<7DY1LWS9?TrfmcvT zaypmplwn=P4;a8-%l^e?f`OpGb}%(_mFsL&GywhyN(-VROj`4~V~9bGv%UhcA|YW% zs{;nh@aDX11y^HOFXB$a7#Sr3cEtNd4eLm@Y#fc&j)TGvbbMwze zXtekX_wJqxe4NhuW$r}cNy|L{V=t#$%SuWEW)YZTH|!iT79k#?632OFse{+BT_gau zJwQcbH{b}dzKO?^dV&3nTILYlGw{27UJ72ZN){BILd_HV_s$WfI2DC<9LIHFmtyw? zQ;?MuK7g%Ym+4e^W#5}WDLpko%jPOC=aN)3!=8)s#Rnercak&b3ESRX3z{xfKBF8L z5%CGkFmGO@x?_mPGlpEej!3!AMddChabyf~nJNZxx!D&{@xEb!TDyvqSj%Y5@A{}9 zRzoBn0?x}=krh{ok3Nn%e)#~uh;6jpezhA)ySb^b#E>73e*frBFu6IZ^D7Ii&rsiU z%jzygxT-n*joJpY4o&8UXr2s%j^Q{?e-voloX`4DQyEK+DmrZh8A$)iWL#NO9+Y@!sO2f@rI!@jN@>HOA< z?q2l{^%mY*PNx2FoX+A7X3N}(RV$B`g&N=e0uvAvEN1W^{*W?zT1i#fxuw10%~))J zjx#gxoVlXREWZf4hRkgdHx5V_S*;p-y%JtGgQ4}lnA~MBz-AFdxUxU1RIT$`sal|X zPB6sEVRjGbXIP0U+?rT|y5+ev&OMX*5C$n2SBPZr`jqzrmpVrNciR0e*Wm?fK6DY& zl(XQZ60yWXV-|Ps!A{EF;=_z(YAF=T(-MkJXUoX zI{UMQDAV2}Ya?EisdEW;@pE6dt;j0fg5oT2dxCi{wqWJ<)|SR6fxX~5CzblPGr8cb zUBVJ2CQd~3L?7yfTpLNbt)He1D>*KXI^GK%<`bq^cUq$Q@uJifG>p3LU(!H=C)aEL zenk7pVg}0{dKU}&l)Y2Y2eFMdS(JS0}oZUuVaf2+K*YFNGHB`^YGcIpnBlMhO7d4@vV zv(@N}(k#REdul8~fP+^F@ky*wt@~&|(&&meNO>rKDEnB{ykAZ}k>e@lad7to>Ao$B zz<1(L=#J*u4_LB=8w+*{KFK^u00NAmeNN7pr+Pf+N*Zl^dO{LM-hMHyP6N!~`24jd zXYP|Ze;dRXKdF2iJG$U{k=S86l@pytLx}$JFFs8e)*Vi?aVBtGJ3JZUj!~c{(rw5>vuRF$`^p!P8w1B=O!skwkO5yd4_XuG^QVF z`-r5K7(IPSiKQ2|U9+`@Js!g6sfJwAHVd|s?|mnC*q zp|B|z)(8+mxXyxQ{8Pg3F4|tdpgZZSoU4P&9I8)nHo1@)9_9u&NcT^FI)6|hsAZFk zZ+arl&@*>RXBf-OZxhZerOr&dN5LW9@gV=oGFbK*J+m#R-|e6(Loz(;g@T^*oO)0R zN`N=X46b{7yk5FZGr#5&n1!-@j@g02g|X>MOpF3#IjZ_4wg{dX+G9eqS+Es9@6nC7 zD9$NuVJI}6ZlwtUm5cCAiYv0(Yi{%eH+}t)!E^>^KxB5^L~a`4%1~5q6h>d;paC9c zTj0wTCKrhWf+F#5>EgX`sl%POl?oyCq0(w0xoL?L%)|Q7d|Hl92rUYAU#lc**I&^6p=4lNQPa0 znQ|A~i0ip@`B=FW-Q;zh?-wF;Wl5!+q3GXDu-x&}$gUO)NoO7^$BeEIrd~1Dh{Tr` z8s<(Bn@gZ(mkIGnmYh_ehXnq78QL$pNDi)|QcT*|GtS%nz1uKE+E{7jdEBp%h0}%r zD2|KmYGiPa4;md-t_m5YDz#c*oV_FqXd85d@eub?9N61QuYcb3CnVWpM(D-^|CmkL z(F}L&N7qhL2PCq)fRh}XO@U`Yn<?TNGR4L(mF7#4u29{i~@k;pLsgl({YW5`Mo+p=zZn3L*4{JU;++dG9 X@eDJUQo;Ye2mwlRs ({ fetchContents: jest.fn(), })); +const aBinaryFile = fs.readFileSync( + resolvePath( + 'src', + '../fixtures/test-nested-template/public/react-logo192.png', + ), +); + const mockFetchContents = fetchContents as jest.MockedFunction< typeof fetchContents >; @@ -119,6 +125,7 @@ describe('fetch:template', () => { 'templated-content.txt': '${{ name }}: ${{ count }}', }, '.${{ name }}': '${{ itemList | dump }}', + 'a-binary-file.png': aBinaryFile, }, }); @@ -169,6 +176,12 @@ describe('fetch:template', () => { fs.readdir(`${workspacePath}/target/empty-dir-1234`, 'utf-8'), ).resolves.toEqual([]); }); + + it('copies binary files as it-is without processing them', async () => { + await expect( + fs.readFile(`${workspacePath}/target/a-binary-file.png`), + ).resolves.toEqual(aBinaryFile); + }); }); describe('copyWithoutRender', () => { From 9d759702b1608fe2ac760ae7e45093061cf81737 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Tue, 6 Jul 2021 15:04:55 +0100 Subject: [PATCH 22/35] docs: update docs to refer to new fetch:template action Signed-off-by: Mike Lewis --- .../software-catalog/descriptor-format.md | 2 +- .../software-templates/adding-templates.md | 2 +- .../software-templates/builtin-actions.md | 36 +++++++++++++++++++ .../software-templates/writing-templates.md | 30 ++++++++-------- .../scaffolder/actions/builtin/debug/log.ts | 2 +- .../actions/builtin/fetch/cookiecutter.ts | 2 +- .../actions/builtin/fetch/template.ts | 2 +- 7 files changed, 56 insertions(+), 20 deletions(-) diff --git a/docs/features/software-catalog/descriptor-format.md b/docs/features/software-catalog/descriptor-format.md index 988af89e47..fb17f4a434 100644 --- a/docs/features/software-catalog/descriptor-format.md +++ b/docs/features/software-catalog/descriptor-format.md @@ -654,7 +654,7 @@ spec: steps: - id: fetch-base name: Fetch Base - action: fetch:cookiecutter + action: fetch:template input: url: ./template values: diff --git a/docs/features/software-templates/adding-templates.md b/docs/features/software-templates/adding-templates.md index d185558949..2cdf6f1b35 100644 --- a/docs/features/software-templates/adding-templates.md +++ b/docs/features/software-templates/adding-templates.md @@ -51,7 +51,7 @@ spec: steps: - id: fetch-base name: Fetch Base - action: fetch:cookiecutter + action: fetch:template input: url: ./template values: diff --git a/docs/features/software-templates/builtin-actions.md b/docs/features/software-templates/builtin-actions.md index 14dc7ef861..a165a0a274 100644 --- a/docs/features/software-templates/builtin-actions.md +++ b/docs/features/software-templates/builtin-actions.md @@ -14,3 +14,39 @@ Azure, GitLab and Bitbucket. A list of all registered actions can be found under `/create/actions`. For local development you should be able to reach them at `http://localhost:3000/create/actions`. + +### Migrating from `fetch:cookiecutter` to `fetch:template` + +The `fetch:template` action is a new action with a similar API to +`fetch:cookiecutter` but no dependency on `cookiecutter`. There are two options +for migrating templates that use `fetch:cookiecutter` to use `fetch:template`: + +#### Using `cookiecutterCompat` mode + +The new `fetch:template` action has a `cookiecutterCompat` flag which should +allow most templates built for `fetch:cookiecutter` to work without any changes. + +1. Update action name in `template.yaml`. The name should be changed from + `fetch:cookiecutter` to `fetch:template`. +2. Set `cookieCutterCompat` to `true` in the `fetch:template` step input in + `template.yaml`. + +#### Manual migration + +If you prefer, you can manually migrate your templates to avoid the need for +enabling cookiecutter compatibility mode, which will result in slightly less +verbose template variables expressions. + +1. Update action name in `template.yaml`. The name should be changed from + `fetch:cookiecutter` to `fetch:template`. +2. Update variable syntax in file names and content. `fetch:cookiecutter` + expects variables to be enclosed in `{{` `}}` and prefixed with + `cookiecutter.`, while `fetch:template` doesn't require prefixing and expects + variables to be enclosed in `${{` `}}`. For example, a reference to variable + `myInputVariable` would need to be migrated from + `{{ cookiecutter.myInputVariable }}` to `${{ myInputVariable }}`. +3. Replace uses of `jsonify` with `dump`. The `jsonify` filter is built in to + `cookiecutter`, and is not available by default when using `fetch:template`. + The `dump` filter is equivalent, so an expression like + `{{ myAwesomeList | jsonify }}` should be migrated to + `${{ myAwesomeList | dump }}`. diff --git a/docs/features/software-templates/writing-templates.md b/docs/features/software-templates/writing-templates.md index 529376eb89..7b475f2fa1 100644 --- a/docs/features/software-templates/writing-templates.md +++ b/docs/features/software-templates/writing-templates.md @@ -62,7 +62,7 @@ spec: steps: - id: fetch-base name: Fetch Base - action: fetch:cookiecutter + action: fetch:template input: url: ./template values: @@ -289,8 +289,8 @@ template. These follow the same standard format: - id: fetch-base # A unique id for the step name: Fetch Base # A title displayed in the frontend if: '{{ parameters.name }}' # Optional condition, skip the step if not truthy - action: fetch:cookiecutter # an action to call - input: # input that is passed as arguments to the action handler + action: fetch:template # An action to call + input: # Input that is passed as arguments to the action handler url: ./template values: name: '{{ parameters.name }}' @@ -317,20 +317,20 @@ output: ### The templating syntax -You might have noticed in the examples that there are `{{ }}`, and these are a -`handlebars` templates for linking and glueing all these different parts of -`yaml` together. All the form inputs from the `parameters` section, when passed -to the steps will be available by using the template syntax -`{{ parameters.something }}`. This is great for passing the values from the form -into different steps and reusing these input variables. To pass arrays or -objects use the syntax `{{ json paramaters.something }}` where -`paramaters.something` is of type `object` or `array` in the `jsonSchema`, such -as the `nicknames` parameter in the previous example. +You might have noticed variables wrapped in `{{ }}` in the examples. These are +`handlebars` template strings for linking and gluing the different parts of the +template together. All the form inputs from the `parameters` section will be +available by using this template syntax (for example, +`{{ parameters.firstName }}` inserts the value of `firstName` from the +parameters). This is great for passing the values from the form into different +steps and reusing these input variables. To pass arrays or objects use the +`json` custom [helper](https://handlebarsjs.com/guide/expressions.html#helpers). +For example, `{{ json parameters.nicknames }}` will insert the result of calling +`JSON.stringify` on the value of the `nicknames` parameter. As you can see above in the `Outputs` section, `actions` and `steps` can also -output things. So you can grab that output by using -`steps.$stepId.output.$property`. +output things. You can grab that output using `steps.$stepId.output.$property`. You can read more about all the `inputs` and `outputs` defined in the actions in -code part of the `JSONSchema` or you can read more about our built in ones +code part of the `JSONSchema`, or you can read more about our built in ones [here](./builtin-actions.md). 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 9b160b1dc3..557190dbbc 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts @@ -26,7 +26,7 @@ export function createDebugLogAction() { return createTemplateAction<{ message?: string; listWorkspace?: boolean }>({ id: 'debug:log', description: - 'Writes a message into the log or list all files in the workspace.', + 'Writes a message into the log or lists all files in the workspace.', schema: { input: { type: 'object', 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 62dfc20125..d179270303 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/cookiecutter.ts @@ -41,7 +41,7 @@ export function createFetchCookiecutterAction(options: { }>({ id: 'fetch:cookiecutter', description: - 'Downloads a template from the given URL into the workspace, and runs cookiecutter on it.', + "Downloads a template from the given URL into the workspace, and runs cookiecutter on it. This action is deprecated in favor of 'fetch:template'. See https://backstage.io/docs/features/software-templates/builtin-actions#migrating-from-fetch-cookiecutter-to-fetch-template for more details.", schema: { input: { type: 'object', 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 cbe74dc472..05e7ebaaa6 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -55,7 +55,7 @@ export function createFetchTemplateAction(options: { return createTemplateAction({ id: 'fetch:template', description: - "Downloads a skeleton and will template variables into the skeleton and places the result in the workspace, or optionally in a subdirectory specified by the 'targetPath' input option.", + "Downloads a skeleton, templates variables into file and directory names and content, and places the result in the workspace, or optionally in a subdirectory specified by the 'targetPath' input option.", schema: { input: { type: 'object', From c22fc45ffae50c79fb9682e9c57305347d230576 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Tue, 6 Jul 2021 15:30:04 +0100 Subject: [PATCH 23/35] scaffolder: use resolveSafeChildPath in fetch:template Signed-off-by: Mike Lewis --- .../scaffolder/actions/builtin/fetch/template.test.ts | 4 +++- .../src/scaffolder/actions/builtin/fetch/template.ts | 10 ++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 3ab96a367e..dbdb3feff1 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -92,7 +92,9 @@ describe('fetch:template', () => { it('throws if output directory is outside the workspace', async () => { await expect(() => action.handler(mockContext({ targetPath: '../' })), - ).rejects.toThrowError(/outside the working directory/i); + ).rejects.toThrowError( + /relative path is not allowed to refer to a directory outside its parent/i, + ); }); it('throws if copyWithoutRender parameter is not an array', async () => { 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 05e7ebaaa6..c45f429929 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -15,7 +15,7 @@ */ import path, { resolve as resolvePath } from 'path'; -import { UrlReader } from '@backstage/backend-common'; +import { resolveSafeChildPath, UrlReader } from '@backstage/backend-common'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; import { fetchContents } from './helpers'; @@ -104,13 +104,7 @@ export function createFetchTemplateAction(options: { const templateDir = resolvePath(workDir, 'template'); const targetPath = ctx.input.targetPath ?? './'; - const outputDir = path.resolve(ctx.workspacePath, targetPath); - - if (!outputDir.startsWith(ctx.workspacePath)) { - throw new InputError( - `Fetch action targetPath may not specify a path outside the working directory`, - ); - } + const outputDir = resolveSafeChildPath(ctx.workspacePath, targetPath); if ( ctx.input.copyWithoutRender && From b40370272b6c2667041c9fa0b2298824617b71fa Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Tue, 6 Jul 2021 15:42:22 +0100 Subject: [PATCH 24/35] scaffolder: remove unused import in fetch:template Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c45f429929..ac7051fe69 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import path, { resolve as resolvePath } from 'path'; +import { resolve as resolvePath } from 'path'; import { resolveSafeChildPath, UrlReader } from '@backstage/backend-common'; import { InputError } from '@backstage/errors'; import { ScmIntegrations } from '@backstage/integration'; From 31de5f27f737738ca42669c30e365ec574dea711 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Tue, 6 Jul 2021 16:14:17 +0100 Subject: [PATCH 25/35] scaffolder: add changeset for new fetch:template action Signed-off-by: Mike Lewis --- .changeset/poor-otters-buy.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/poor-otters-buy.md diff --git a/.changeset/poor-otters-buy.md b/.changeset/poor-otters-buy.md new file mode 100644 index 0000000000..ae1f5f2ef1 --- /dev/null +++ b/.changeset/poor-otters-buy.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Add new `fetch:template` action which handles the same responsibilities as `fetch:cookiecutter` without the external dependency on `cookiecutter`. For information on migrating from `fetch:cookiecutter` to `fetch:template`, see the [migration guide](https://backstage.io/docs/features/software-templates/builtin-actions#migrating-from-fetch-cookiecutter-to-fetch-template) in the docs. From 19666c787209baf31a41abe38c550ea5f44e2685 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 09:43:53 +0100 Subject: [PATCH 26/35] Update fetch:template action changeset to patch Signed-off-by: Mike Lewis --- .changeset/poor-otters-buy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/poor-otters-buy.md b/.changeset/poor-otters-buy.md index ae1f5f2ef1..b60cbddc1b 100644 --- a/.changeset/poor-otters-buy.md +++ b/.changeset/poor-otters-buy.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend': patch --- Add new `fetch:template` action which handles the same responsibilities as `fetch:cookiecutter` without the external dependency on `cookiecutter`. For information on migrating from `fetch:cookiecutter` to `fetch:template`, see the [migration guide](https://backstage.io/docs/features/software-templates/builtin-actions#migrating-from-fetch-cookiecutter-to-fetch-template) in the docs. From 886844aa30eedbfc0ea13a6d89de80446008540e Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 11:00:14 +0100 Subject: [PATCH 27/35] scaffolder: platform-agnostic path joining in fetch:template test suite Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index dbdb3feff1..98ac01edec 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -15,7 +15,7 @@ */ import os from 'os'; -import { resolve as resolvePath } from 'path'; +import { join as joinPath, resolve as resolvePath } from 'path'; import fs from 'fs-extra'; import mockFs from 'mock-fs'; import { getVoidLogger, UrlReader } from '@backstage/backend-common'; @@ -48,7 +48,9 @@ describe('fetch:template', () => { ActionContext['createTemporaryDirectory'] > = jest.fn(() => Promise.resolve( - `${workspacePath}/${createTemporaryDirectory.mock.calls.length}`, + joinPath( + `${workspacePath}/${createTemporaryDirectory.mock.calls.length}`, + ), ), ); From 51513c64208f39d699e488e5977d8ce36591399e Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 11:20:10 +0100 Subject: [PATCH 28/35] scaffolder: fix use of path.join in fetch:template tests Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 98ac01edec..56fffe0189 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -48,9 +48,7 @@ describe('fetch:template', () => { ActionContext['createTemporaryDirectory'] > = jest.fn(() => Promise.resolve( - joinPath( - `${workspacePath}/${createTemporaryDirectory.mock.calls.length}`, - ), + joinPath(workspacePath, `${createTemporaryDirectory.mock.calls.length}`), ), ); From 56b61c2bc05c7d1d932c85632737f7eba2bc2690 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 11:26:00 +0100 Subject: [PATCH 29/35] scaffolder: prefix input values in fetch:template templates Signed-off-by: Mike Lewis --- .../software-templates/builtin-actions.md | 6 +++--- .../actions/builtin/fetch/template.test.ts | 19 +++++++++++-------- .../actions/builtin/fetch/template.ts | 11 ++++++----- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/docs/features/software-templates/builtin-actions.md b/docs/features/software-templates/builtin-actions.md index a165a0a274..5cd6b005d9 100644 --- a/docs/features/software-templates/builtin-actions.md +++ b/docs/features/software-templates/builtin-actions.md @@ -41,10 +41,10 @@ verbose template variables expressions. `fetch:cookiecutter` to `fetch:template`. 2. Update variable syntax in file names and content. `fetch:cookiecutter` expects variables to be enclosed in `{{` `}}` and prefixed with - `cookiecutter.`, while `fetch:template` doesn't require prefixing and expects - variables to be enclosed in `${{` `}}`. For example, a reference to variable + `cookiecutter.`, while `fetch:template` expects variables to be enclosed in + `${{` `}}` and prefixed with `values.`. For example, a reference to variable `myInputVariable` would need to be migrated from - `{{ cookiecutter.myInputVariable }}` to `${{ myInputVariable }}`. + `{{ cookiecutter.myInputVariable }}` to `${{ values.myInputVariable }}`. 3. Replace uses of `jsonify` with `dump`. The `jsonify` filter is built in to `cookiecutter`, and is not available by default when using `fetch:template`. The `dump` filter is equivalent, so an expression like diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 56fffe0189..79cc5be524 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -120,13 +120,14 @@ describe('fetch:template', () => { mockFetchContents.mockImplementation(({ outputPath }) => { mockFs({ [outputPath]: { - 'empty-dir-${{ count }}': {}, + 'empty-dir-${{ values.count }}': {}, 'static.txt': 'static content', - '${{ name }}.txt': 'static content', + '${{ values.name }}.txt': 'static content', subdir: { - 'templated-content.txt': '${{ name }}: ${{ count }}', + 'templated-content.txt': + '${{ values.name }}: ${{ values.count }}', }, - '.${{ name }}': '${{ itemList | dump }}', + '.${{ values.name }}': '${{ values.itemList | dump }}', 'a-binary-file.png': aBinaryFile, }, }); @@ -202,10 +203,12 @@ describe('fetch:template', () => { mockFs({ [outputPath]: { processed: { - 'templated-content-${{ name }}.txt': '${{ count }}', + 'templated-content-${{ values.name }}.txt': + '${{ values.count }}', }, '.unprocessed': { - 'templated-content-${{ name }}.txt': '${{ count }}', + 'templated-content-${{ values.name }}.txt': + '${{ values.count }}', }, }, }); @@ -219,10 +222,10 @@ describe('fetch:template', () => { it('ignores template syntax in files matched in copyWithoutRender', async () => { await expect( fs.readFile( - `${workspacePath}/target/.unprocessed/templated-content-\${{ name }}.txt`, + `${workspacePath}/target/.unprocessed/templated-content-\${{ values.name }}.txt`, 'utf-8', ), - ).resolves.toEqual('${{ count }}'); + ).resolves.toEqual('${{ values.count }}'); }); it('processes files not matched in copyWithoutRender', async () => { 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 ac7051fe69..4fd505f9b0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -180,9 +180,10 @@ export function createFetchTemplateAction(options: { // `cookiecutter.`. To replicate this, we wrap our parameters // in an object with a `cookiecutter` property when compat // mode is enabled. - const parameters = ctx.input.cookiecutterCompat - ? { cookiecutter: ctx.input.values } - : ctx.input.values; + const { cookiecutterCompat, values } = ctx.input; + const context = { + [cookiecutterCompat ? 'cookiecutter' : 'values']: values, + }; ctx.logger.info( `Processing ${allEntriesInTemplate.length} template files/directories with input values`, @@ -196,7 +197,7 @@ export function createFetchTemplateAction(options: { outputDir, shouldCopyWithoutRender ? location - : templater.renderString(location, parameters), + : templater.renderString(location, context), ); if (shouldCopyWithoutRender) { @@ -227,7 +228,7 @@ export function createFetchTemplateAction(options: { outputPath, shouldCopyWithoutRender ? inputFileContents - : templater.renderString(inputFileContents, parameters), + : templater.renderString(inputFileContents, context), ); } } From fdac00fb3fe6810e8ad603fd907cf164f3aa44b2 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 13:10:36 +0100 Subject: [PATCH 30/35] scaffolder: fix typo in fetch:template migration docs Signed-off-by: Mike Lewis --- docs/features/software-templates/builtin-actions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/software-templates/builtin-actions.md b/docs/features/software-templates/builtin-actions.md index 5cd6b005d9..8c80eaedae 100644 --- a/docs/features/software-templates/builtin-actions.md +++ b/docs/features/software-templates/builtin-actions.md @@ -28,7 +28,7 @@ allow most templates built for `fetch:cookiecutter` to work without any changes. 1. Update action name in `template.yaml`. The name should be changed from `fetch:cookiecutter` to `fetch:template`. -2. Set `cookieCutterCompat` to `true` in the `fetch:template` step input in +2. Set `cookiecutterCompat` to `true` in the `fetch:template` step input in `template.yaml`. #### Manual migration From 3ee0caf82e6a99052f7f01b93e5cd992f8a597aa Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 13:13:25 +0100 Subject: [PATCH 31/35] scaffolder: add example diff in fetch:template migration docs Signed-off-by: Mike Lewis --- docs/features/software-templates/builtin-actions.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/features/software-templates/builtin-actions.md b/docs/features/software-templates/builtin-actions.md index 8c80eaedae..cdf5f5fb2d 100644 --- a/docs/features/software-templates/builtin-actions.md +++ b/docs/features/software-templates/builtin-actions.md @@ -31,6 +31,18 @@ allow most templates built for `fetch:cookiecutter` to work without any changes. 2. Set `cookiecutterCompat` to `true` in the `fetch:template` step input in `template.yaml`. +```diff + steps: + - id: fetch-base + name: Fetch Base +- action: fetch:cookiecutter ++ action: fetch:template + input: + url: ./skeleton ++ cookiecutterCompat: true + values: +``` + #### Manual migration If you prefer, you can manually migrate your templates to avoid the need for From b2db928644c0f467f68dc021e4a7dc066d75eabb Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 13:17:39 +0100 Subject: [PATCH 32/35] scaffolder: improve fetch:template migration docs Signed-off-by: Mike Lewis --- docs/features/software-templates/builtin-actions.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/features/software-templates/builtin-actions.md b/docs/features/software-templates/builtin-actions.md index cdf5f5fb2d..af3ae97b21 100644 --- a/docs/features/software-templates/builtin-actions.md +++ b/docs/features/software-templates/builtin-actions.md @@ -57,8 +57,11 @@ verbose template variables expressions. `${{` `}}` and prefixed with `values.`. For example, a reference to variable `myInputVariable` would need to be migrated from `{{ cookiecutter.myInputVariable }}` to `${{ values.myInputVariable }}`. -3. Replace uses of `jsonify` with `dump`. The `jsonify` filter is built in to - `cookiecutter`, and is not available by default when using `fetch:template`. - The `dump` filter is equivalent, so an expression like - `{{ myAwesomeList | jsonify }}` should be migrated to - `${{ myAwesomeList | dump }}`. +3. Replace uses of `jsonify` with `dump`. The + [`jsonify` filter](https://cookiecutter.readthedocs.io/en/latest/advanced/template_extensions.html#jsonify-extension) + is built in to `cookiecutter`, and is not available by default when using + `fetch:template`. The + [`dump` filter](https://mozilla.github.io/nunjucks/templating.html#dump) is + the equivalent filter in nunjucks, so an expression like + `{{ cookiecutter.myAwesomeList | jsonify }}` should be migrated to + `${{ values.myAwesomeList | dump }}`. From b4035aa463366ba744928db9ff1873f913912af4 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 13:21:24 +0100 Subject: [PATCH 33/35] scaffolder: tweak test description in fetch:template suite Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts index 79cc5be524..bd2beae47a 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts @@ -180,7 +180,7 @@ describe('fetch:template', () => { ).resolves.toEqual([]); }); - it('copies binary files as it-is without processing them', async () => { + it('copies binary files as-is without processing them', async () => { await expect( fs.readFile(`${workspacePath}/target/a-binary-file.png`), ).resolves.toEqual(aBinaryFile); From 13d9940cde373ba0e070ee15a8df84bc033197aa Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 13:22:44 +0100 Subject: [PATCH 34/35] scaffolder: remove completed TODO in fetch:template Signed-off-by: Mike Lewis --- .../src/scaffolder/actions/builtin/fetch/template.ts | 1 - 1 file changed, 1 deletion(-) 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 4fd505f9b0..87e67f76be 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -89,7 +89,6 @@ export function createFetchTemplateAction(options: { }, cookiecutterCompat: { title: 'Cookiecutter compatibility mode', - // TODO(mtlewis): documentation for cookiecutter compat mode description: 'Enable features to maximise compatibility with templates built for fetch:cookiecutter', type: 'boolean', From eece4003bf0ddd64e775df1ac64afc1f3ce943b2 Mon Sep 17 00:00:00 2001 From: Mike Lewis Date: Wed, 7 Jul 2021 17:29:01 +0100 Subject: [PATCH 35/35] add nunjucks to dictionary Signed-off-by: Mike Lewis --- .github/styles/vocab.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index f6826cbefe..6cc276428f 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -169,6 +169,7 @@ nohoist nonces noop npm +nunjucks nvarchar nvm OAuth