Merge pull request #12417 from mknet3/mknet3/fix-empty-dirs-issue

Fix issues with optional directories and files

.changeset/orange-deers-marry.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Fix issues with optional directories and files

plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.test.ts

@@ -156,6 +156,99 @@ describe('fetch:template', () => {
       );
     });
 
+    describe('with optional directories / files', () => {
+      let context: ActionContext<FetchTemplateInput>;
+
+      beforeEach(async () => {
+        context = mockContext({
+          values: {
+            showDummyFile: false,
+            skipRootDirectory: true,
+            skipSubdirectory: true,
+            skipMultiplesDirectories: true,
+            skipFileInsideDirectory: 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',
+                      },
+                  },
+              },
+              subdir3: {
+                '${{ "fileSkippedInsideDirectory.txt" if not values.skipFileInsideDirectory else "" }}':
+                  'skipped file inside directory',
+              },
+            },
+          });
+
+          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 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(
+            `${workspacePath}/target/subdir2/multipleDirectorySkippedFile.txt`,
+          ),
+        ).resolves.toEqual(false);
+
+        await expect(
+          fs.pathExists(
+            `${workspacePath}/target/subdir2/dummy-subdir/dummy-subdir/multipleDirectorySkippedFile.txt`,
+          ),
+        ).resolves.toEqual(false);
+      });
+    });
+
     describe('with valid input', () => {
       let context: ActionContext<FetchTemplateInput>;
 
@@ -189,11 +282,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 +300,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'),

plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts

@@ -27,6 +27,7 @@ import {
   TemplateFilter,
   SecureTemplater,
 } from '../../../../lib/templating/SecureTemplater';
+import path from 'path';
 
 /**
  * Downloads a skeleton, templates variables into file and directory names and content.
@@ -206,13 +207,17 @@ export function createFetchTemplateAction(options: {
         } else {
           renderFilename = renderContents = !nonTemplatedEntries.has(location);
         }
+
         if (renderFilename) {
           localOutputPath = renderTemplate(localOutputPath, context);
         }
+
+        if (containsSkippedContent(localOutputPath)) {
+          continue;
+        }
+
         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 (fs.existsSync(outputPath)) {
           continue;
         }
 
@@ -257,3 +262,14 @@ export function createFetchTemplateAction(options: {
     },
   });
 }
+
+function containsSkippedContent(localOutputPath: string): boolean {
+  // 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 === '' ||
+    path.isAbsolute(localOutputPath) ||
+    localOutputPath.includes(`${path.sep}${path.sep}`)
+  );
+}