From 1d17902107520820a5a104e543946b6c3521b2cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Can=CC=83ete?= Date: Mon, 4 Jul 2022 11:07:29 +0200 Subject: [PATCH 01/11] Fix issues with empty directories and files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../actions/builtin/fetch/template.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 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 a1afabc39e..15e1a6fd75 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -206,16 +206,17 @@ export function createFetchTemplateAction(options: { } else { renderFilename = renderContents = !nonTemplatedEntries.has(location); } + if (renderFilename) { localOutputPath = renderTemplate(localOutputPath, context); } - const outputPath = resolveSafeChildPath(outputDir, localOutputPath); - // variables have been expanded to make an empty file name - // this is due to a conditional like if values.my_condition then file-name.txt else empty string so skip - if (outputDir === outputPath) { + + if (containsSkippedContent(localOutputPath)) { continue; } + const outputPath = resolveSafeChildPath(outputDir, localOutputPath); + if (!renderContents && !extension) { ctx.logger.info( `Copying file/directory ${location} without processing.`, @@ -257,3 +258,12 @@ export function createFetchTemplateAction(options: { }, }); } + +function containsSkippedContent(localOutputPath: string): boolean { + // if the path starts with / means that the root directory has been skipped + // if the path is empty means that there is a file skipped in the root + // if the path includes // means that there is a subdirectory skipped + return localOutputPath.startsWith('/') + || localOutputPath === '' + || localOutputPath.includes('//'); +} From 089d846962ec21edac2e552145a5b2cc147da4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Tue, 5 Jul 2022 10:15:29 +0200 Subject: [PATCH 02/11] add changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .changeset/orange-deers-marry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/orange-deers-marry.md diff --git a/.changeset/orange-deers-marry.md b/.changeset/orange-deers-marry.md new file mode 100644 index 0000000000..f344450537 --- /dev/null +++ b/.changeset/orange-deers-marry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Fix issues with optional directories and files From 7378e367efccd4e07bf35f68f0d398b49c673a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= <10815022+mknet3@users.noreply.github.com> Date: Tue, 5 Jul 2022 10:17:24 +0200 Subject: [PATCH 03/11] Update orange-deers-marry.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .changeset/orange-deers-marry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/orange-deers-marry.md b/.changeset/orange-deers-marry.md index f344450537..c565111b60 100644 --- a/.changeset/orange-deers-marry.md +++ b/.changeset/orange-deers-marry.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend': patch --- Fix issues with optional directories and files From 7a2dd9a37ac614da927a03f6d18de0b8441e7d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Tue, 5 Jul 2022 12:30:52 +0200 Subject: [PATCH 04/11] add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../actions/builtin/fetch/template.test.ts | 88 +++++++++++++++---- 1 file changed, 71 insertions(+), 17 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 06def076ea..f52cca504b 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 @@ -156,6 +156,77 @@ describe('fetch:template', () => { ); }); + describe('with optional directories / files', () => { + let context: ActionContext; + + beforeEach(async () => { + context = mockContext({ + values: { + showDummyFile: false, + skipRootDirectory: true, + skipSubdirectory: true, + skipMultiplesDirectories: true + }, + }); + + mockFetchContents.mockImplementation(({ outputPath }) => { + mockFs({ + ...realFiles, + [outputPath]: { + '{% if values.showDummyFile %}dummy-file.txt{% else %}{% endif %}': + 'dummy file', + '${{ "dummy-file2.txt" if values.showDummyFile else "" }}': + 'some dummy file', + '${{ "dummy-dir" if not values.skipRootDirectory else "" }}': { + 'file.txt': + 'file inside optional directory', + subdir: { + '${{ "dummy-subdir" if not values.skipSubdirectory else "" }}': + 'file inside optional subdirectory' + } + }, + subdir2: { + '${{ "dummy-subdir" if not values.skipMultiplesDirectories else "" }}': { + '${{ "dummy-subdir" if not values.skipMultiplesDirectories else "" }}': { + 'multipleDirectorySkippedFile.txt': + 'file inside multiple optional subdirectories' + } + } + } + }, + }); + + return Promise.resolve(); + }); + + await action.handler(context); + }); + + it('skips empty filename', async () => { + await expect( + fs.pathExists(`${workspacePath}/target/dummy-file.txt`), + ).resolves.toEqual(false); + }); + + it('skips empty filename syntax #2', async () => { + await expect( + fs.pathExists(`${workspacePath}/target/dummy-file2.txt`), + ).resolves.toEqual(false); + }); + + it('skips empty directory', async () => { + await expect( + fs.pathExists(`${workspacePath}/target/dummy-dir/dummy-file3.txt`), + ).resolves.toEqual(false); + }); + + it('skips content of empty directory', async () => { + await expect( + fs.pathExists(`${workspacePath}/target/subdir2/multipleDirectorySkippedFile.txt`), + ).resolves.toEqual(false); + }); + }); + describe('with valid input', () => { let context: ActionContext; @@ -189,11 +260,6 @@ describe('fetch:template', () => { symlink: mockFs.symlink({ path: 'a-binary-file.png', }), - - '{% if values.showDummyFile %}dummy-file.txt{% else %}{% endif %}': - 'dummy file', - '${{ "dummy-file2.txt" if values.showDummyFile else "" }}': - 'some dummy file', }, }); @@ -212,18 +278,6 @@ describe('fetch:template', () => { ); }); - it('skips empty filename', async () => { - await expect( - fs.pathExists(`${workspacePath}/target/dummy-file.txt`), - ).resolves.toEqual(false); - }); - - it('skips empty filename syntax #2', async () => { - await expect( - fs.pathExists(`${workspacePath}/target/dummy-file2.txt`), - ).resolves.toEqual(false); - }); - it('copies files with no templating in names or content successfully', async () => { await expect( fs.readFile(`${workspacePath}/target/static.txt`, 'utf-8'), From 1b9467ff32206ae9c300598f3c2c43db0bb6ad02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Tue, 5 Jul 2022 12:36:19 +0200 Subject: [PATCH 05/11] add test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../src/scaffolder/actions/builtin/fetch/template.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 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 f52cca504b..78c6255ba0 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 @@ -220,10 +220,14 @@ describe('fetch:template', () => { ).resolves.toEqual(false); }); - it('skips content of empty directory', async () => { + it('skips content of empty subdirectory', async () => { await expect( fs.pathExists(`${workspacePath}/target/subdir2/multipleDirectorySkippedFile.txt`), ).resolves.toEqual(false); + + await expect( + fs.pathExists(`${workspacePath}/target/subdir2/dummy-subdir/dummy-subdir/multipleDirectorySkippedFile.txt`), + ).resolves.toEqual(false); }); }); From 142bf701a832c16e36726b625c194f9d273553ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Wed, 6 Jul 2022 09:40:37 +0200 Subject: [PATCH 06/11] fix prettier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../actions/builtin/fetch/template.test.ts | 39 +++++++++++-------- .../actions/builtin/fetch/template.ts | 8 ++-- 2 files changed, 27 insertions(+), 20 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 78c6255ba0..a168f58184 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 @@ -165,7 +165,7 @@ describe('fetch:template', () => { showDummyFile: false, skipRootDirectory: true, skipSubdirectory: true, - skipMultiplesDirectories: true + skipMultiplesDirectories: true, }, }); @@ -178,21 +178,22 @@ describe('fetch:template', () => { '${{ "dummy-file2.txt" if values.showDummyFile else "" }}': 'some dummy file', '${{ "dummy-dir" if not values.skipRootDirectory else "" }}': { - 'file.txt': - 'file inside optional directory', - subdir: { - '${{ "dummy-subdir" if not values.skipSubdirectory else "" }}': - 'file inside optional subdirectory' - } + 'file.txt': 'file inside optional directory', + subdir: { + '${{ "dummy-subdir" if not values.skipSubdirectory else "" }}': + 'file inside optional subdirectory', + }, }, subdir2: { - '${{ "dummy-subdir" if not values.skipMultiplesDirectories else "" }}': { - '${{ "dummy-subdir" if not values.skipMultiplesDirectories else "" }}': { - 'multipleDirectorySkippedFile.txt': - 'file inside multiple optional subdirectories' - } - } - } + '${{ "dummy-subdir" if not values.skipMultiplesDirectories else "" }}': + { + '${{ "dummy-subdir" if not values.skipMultiplesDirectories else "" }}': + { + 'multipleDirectorySkippedFile.txt': + 'file inside multiple optional subdirectories', + }, + }, + }, }, }); @@ -222,12 +223,16 @@ describe('fetch:template', () => { it('skips content of empty subdirectory', async () => { await expect( - fs.pathExists(`${workspacePath}/target/subdir2/multipleDirectorySkippedFile.txt`), + fs.pathExists( + `${workspacePath}/target/subdir2/multipleDirectorySkippedFile.txt`, + ), ).resolves.toEqual(false); await expect( - fs.pathExists(`${workspacePath}/target/subdir2/dummy-subdir/dummy-subdir/multipleDirectorySkippedFile.txt`), - ).resolves.toEqual(false); + fs.pathExists( + `${workspacePath}/target/subdir2/dummy-subdir/dummy-subdir/multipleDirectorySkippedFile.txt`, + ), + ).resolves.toEqual(false); }); }); 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 15e1a6fd75..93ed7645c0 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -263,7 +263,9 @@ function containsSkippedContent(localOutputPath: string): boolean { // if the path starts with / means that the root directory has been skipped // if the path is empty means that there is a file skipped in the root // if the path includes // means that there is a subdirectory skipped - return localOutputPath.startsWith('/') - || localOutputPath === '' - || localOutputPath.includes('//'); + return ( + localOutputPath.startsWith('/') || + localOutputPath === '' || + localOutputPath.includes('//') + ); } From 42f4b0f425ec57f8b3b046d31635eb611fb765e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Wed, 6 Jul 2022 21:58:39 +0200 Subject: [PATCH 07/11] change the way to check skipped content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../src/scaffolder/actions/builtin/fetch/template.ts | 7 ++++--- 1 file changed, 4 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 93ed7645c0..5e076cabab 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -27,6 +27,7 @@ import { TemplateFilter, SecureTemplater, } from '../../../../lib/templating/SecureTemplater'; +import path from 'node:path'; /** * Downloads a skeleton, templates variables into file and directory names and content. @@ -260,12 +261,12 @@ export function createFetchTemplateAction(options: { } function containsSkippedContent(localOutputPath: string): boolean { - // if the path starts with / means that the root directory has been skipped + // if the path is absolute means that the root directory has been skipped // if the path is empty means that there is a file skipped in the root // if the path includes // means that there is a subdirectory skipped return ( - localOutputPath.startsWith('/') || localOutputPath === '' || - localOutputPath.includes('//') + path.isAbsolute(localOutputPath) || + localOutputPath.includes(`${path.sep}${path.sep}`) ); } From 7ba0cdb017042d0f7c646b845415ee2bd5949703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= <10815022+mknet3@users.noreply.github.com> Date: Fri, 8 Jul 2022 13:26:47 +0200 Subject: [PATCH 08/11] Update template.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../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 5e076cabab..119c870002 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -27,7 +27,7 @@ import { TemplateFilter, SecureTemplater, } from '../../../../lib/templating/SecureTemplater'; -import path from 'node:path'; +import path from 'path'; /** * Downloads a skeleton, templates variables into file and directory names and content. From 7d34991080383d5cf7b53580f4b17aa5e22ca203 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Fri, 8 Jul 2022 13:43:02 +0200 Subject: [PATCH 09/11] add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../src/scaffolder/actions/builtin/fetch/template.test.ts | 5 +++++ 1 file changed, 5 insertions(+) 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 a168f58184..c870d9a6b7 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 @@ -166,6 +166,7 @@ describe('fetch:template', () => { skipRootDirectory: true, skipSubdirectory: true, skipMultiplesDirectories: true, + skipFileInsideDirectory: true }, }); @@ -194,6 +195,10 @@ describe('fetch:template', () => { }, }, }, + subdir3: { + '${{ "fileSkippedInsideDirectory.txt" if not values.skipFileInsideDirectory else "" }}': + 'skipped file inside directory', + }, }, }); From bc59e8833baf5e15eca7691ff8c8965babbd095a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Sat, 9 Jul 2022 19:37:40 +0200 Subject: [PATCH 10/11] Fix issue when a file is skipped inside a directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../src/scaffolder/actions/builtin/fetch/template.test.ts | 6 ++++++ .../src/scaffolder/actions/builtin/fetch/template.ts | 3 +++ 2 files changed, 9 insertions(+) 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 c870d9a6b7..0b90db54e1 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 @@ -226,6 +226,12 @@ describe('fetch:template', () => { ).resolves.toEqual(false); }); + it('skips empty filename inside directory', async () => { + await expect( + fs.pathExists(`${workspacePath}/target/subdir3/fileSkippedInsideDirectory.txt`), + ).resolves.toEqual(false); + }); + it('skips content of empty subdirectory', async () => { await expect( fs.pathExists( 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 119c870002..25457f4775 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts @@ -217,6 +217,9 @@ export function createFetchTemplateAction(options: { } const outputPath = resolveSafeChildPath(outputDir, localOutputPath); + if (fs.existsSync(outputPath)) { + continue; + } if (!renderContents && !extension) { ctx.logger.info( From a15ff27c3925fa1a24f38df171cd736771da3056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Ca=C3=B1ete?= Date: Sat, 9 Jul 2022 19:59:45 +0200 Subject: [PATCH 11/11] fix prettier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Cañete --- .../scaffolder/actions/builtin/fetch/template.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 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 0b90db54e1..2557f365f3 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 @@ -166,7 +166,7 @@ describe('fetch:template', () => { skipRootDirectory: true, skipSubdirectory: true, skipMultiplesDirectories: true, - skipFileInsideDirectory: true + skipFileInsideDirectory: true, }, }); @@ -198,7 +198,7 @@ describe('fetch:template', () => { subdir3: { '${{ "fileSkippedInsideDirectory.txt" if not values.skipFileInsideDirectory else "" }}': 'skipped file inside directory', - }, + }, }, }); @@ -228,9 +228,11 @@ describe('fetch:template', () => { it('skips empty filename inside directory', async () => { await expect( - fs.pathExists(`${workspacePath}/target/subdir3/fileSkippedInsideDirectory.txt`), + fs.pathExists( + `${workspacePath}/target/subdir3/fileSkippedInsideDirectory.txt`, + ), ).resolves.toEqual(false); - }); + }); it('skips content of empty subdirectory', async () => { await expect(