Supporting Multiple Files With the Same Name (#327)

* changed ubuntu runner

* changed minikube action

* Version formatting

* nonedriveR

* update kube version

* installing conntrack'

* updated other actions

* update bg ingress api version

* prettify

* updated ingress backend for new api version

* Added path type

* prettify

* added logging

* added try catch logic to prevent future failures if annotations fail since failing annotations shouldn't affect users

* added nullcheck

* Added fallback filename if workflow fails to get github filepath due to runner issues

* cleanup

* added oliver's feedback + unit test demonstrating regex glitch and fix

* no longer using blank string for failed regex

* add tests and dont split so much

* testing

* file fix

* without fix

* Revert "without fix"

This reverts commit 8da79a8190.

* fixing labels test

* pretty

* refactored getting tmp filename to use entire path, and refactored private to use filepath relative to tmp dir

* wip

* merging master

* this should fail

* added UTs

* restructured plus UTs plus debug logs

* resolved dir not existing and UTs

* cleanup

* no silent failure

* Reverting private logic

* this might work

* root level files for temp... bizarre issue

* need to actually write contents

* no more cwdir

* moving everything out of temp

* deleting unused function

* supporting windows filepaths for private cluster shallow path generation

---------

Co-authored-by: David Gamero <david340804@gmail.com>

src/utilities/fileUtils.test.ts

@@ -1,20 +1,14 @@
-import {
-   getFilesFromDirectoriesAndURLs,
-   getTempDirectory,
-   urlFileKind,
-   writeYamlFromURLToFile
-} from './fileUtils'
+import * as fileUtils from './fileUtils'
 
 import * as yaml from 'js-yaml'
 import * as fs from 'fs'
 import * as path from 'path'
-import {succeeded} from '../types/errorable'
 
 const sampleYamlUrl =
    'https://raw.githubusercontent.com/kubernetes/website/main/content/en/examples/controllers/nginx-deployment.yaml'
 describe('File utils', () => {
    test('correctly parses a yaml file from a URL', async () => {
-      const tempFile = await writeYamlFromURLToFile(sampleYamlUrl, 0)
+      const tempFile = await fileUtils.writeYamlFromURLToFile(sampleYamlUrl, 0)
       const fileContents = fs.readFileSync(tempFile).toString()
       const inputObjects = yaml.safeLoadAll(fileContents)
       expect(inputObjects).toHaveLength(1)
@@ -30,34 +24,34 @@ describe('File utils', () => {
 
       const testPath = path.join('test', 'unit', 'manifests')
       await expect(
-         getFilesFromDirectoriesAndURLs([testPath, badUrl])
+         fileUtils.getFilesFromDirectoriesAndURLs([testPath, badUrl])
       ).rejects.toThrow()
    })
 
-   it('detects files in nested directories and ignores non-manifest files and empty dirs', async () => {
+   it('detects files in nested directories with the same name and ignores non-manifest files and empty dirs', async () => {
       const testPath = path.join('test', 'unit', 'manifests')
-      const testSearch: string[] = await getFilesFromDirectoriesAndURLs([
-         testPath,
-         sampleYamlUrl
-      ])
+      const testSearch: string[] =
+         await fileUtils.getFilesFromDirectoriesAndURLs([
+            testPath,
+            sampleYamlUrl
+         ])
 
       const expectedManifests = [
-         'test/unit/manifests/manifest_test_dir/another_layer/deep-ingress.yaml',
-         'test/unit/manifests/manifest_test_dir/another_layer/deep-service.yaml',
+         'test/unit/manifests/manifest_test_dir/another_layer/test-ingress.yaml',
+         'test/unit/manifests/manifest_test_dir/another_layer/nested-test-service.yaml',
          'test/unit/manifests/manifest_test_dir/nested-test-service.yaml',
          'test/unit/manifests/test-ingress.yml',
          'test/unit/manifests/test-ingress-new.yml',
          'test/unit/manifests/test-service.yml'
       ]
 
-      // is there a more efficient way to test equality w random order?
       expect(testSearch).toHaveLength(8)
       expectedManifests.forEach((fileName) => {
          if (fileName.startsWith('test/unit')) {
             expect(testSearch).toContain(fileName)
          } else {
-            expect(fileName.includes(urlFileKind)).toBe(true)
-            expect(fileName.startsWith(getTempDirectory()))
+            expect(fileName.includes(fileUtils.urlFileKind)).toBe(true)
+            expect(fileName.startsWith(fileUtils.getTempDirectory()))
          }
       })
    })
@@ -72,7 +66,7 @@ describe('File utils', () => {
       )
 
       expect(
-         getFilesFromDirectoriesAndURLs([badPath, goodPath])
+         fileUtils.getFilesFromDirectoriesAndURLs([badPath, goodPath])
       ).rejects.toThrowError()
    })
 
@@ -92,7 +86,7 @@ describe('File utils', () => {
       )
 
       expect(
-         await getFilesFromDirectoriesAndURLs([
+         await fileUtils.getFilesFromDirectoriesAndURLs([
             outerPath,
             fileAtOuter,
             innerPath
@@ -102,6 +96,24 @@ describe('File utils', () => {
 
    it('throws an error for an invalid URL', async () => {
       const badUrl = 'https://www.github.com'
-      await expect(writeYamlFromURLToFile(badUrl, 0)).rejects.toBeTruthy()
+      await expect(
+         fileUtils.writeYamlFromURLToFile(badUrl, 0)
+      ).rejects.toBeTruthy()
+   })
+})
+
+describe('moving files to temp', () => {
+   it('correctly moves the contents of a file to the temporary directory', () => {
+      jest.spyOn(fs, 'writeFileSync').mockImplementation(() => {})
+      jest.spyOn(fs, 'readFileSync').mockImplementation((filename) => {
+         return 'test contents'
+      })
+      const originalFilePath = path.join('path', 'in', 'repo')
+
+      const output = fileUtils.moveFileToTmpDir(originalFilePath)
+
+      expect(output).toEqual(
+         path.join(fileUtils.getTempDirectory(), '/path/in/repo')
+      )
    })
 })

src/utilities/fileUtils.ts

@@ -23,7 +23,7 @@ export function writeObjectsToFile(inputObjects: any[]): string[] {
          const inputObjectString = JSON.stringify(inputObject)
 
          if (inputObject?.metadata?.name) {
-            const fileName = getManifestFileName(
+            const fileName = getNewTempManifestFileName(
                inputObject.kind,
                inputObject.metadata.name
             )
@@ -52,7 +52,7 @@ export function writeManifestToFile(
 ): string {
    if (inputObjectString) {
       try {
-         const fileName = getManifestFileName(kind, name)
+         const fileName = getNewTempManifestFileName(kind, name)
          fs.writeFileSync(path.join(fileName), inputObjectString)
          return fileName
       } catch (ex) {
@@ -63,7 +63,27 @@ export function writeManifestToFile(
    }
 }
 
-function getManifestFileName(kind: string, name: string) {
+export function moveFileToTmpDir(originalFilepath: string) {
+   const tempDirectory = getTempDirectory()
+   const newPath = path.join(tempDirectory, originalFilepath)
+
+   core.debug(`reading original contents from path: ${originalFilepath}`)
+   const contents = fs.readFileSync(originalFilepath).toString()
+
+   const dirName = path.dirname(newPath)
+   if (!fs.existsSync(dirName)) {
+      core.debug(`path ${dirName} doesn't exist yet, making new dir...`)
+      fs.mkdirSync(dirName, {recursive: true})
+   }
+   core.debug(`writing contents to new path ${newPath}`)
+   fs.writeFileSync(path.join(newPath), contents)
+
+   core.debug(`moved contents from ${originalFilepath} to ${newPath}`)
+
+   return newPath
+}
+
+function getNewTempManifestFileName(kind: string, name: string) {
    const filePath = `${kind}_${name}_${getCurrentTime().toString()}`
    const tempDirectory = getTempDirectory()
    return path.join(tempDirectory, path.basename(filePath))
@@ -130,7 +150,7 @@ export async function writeYamlFromURLToFile(
                )
             }
 
-            const targetPath = getManifestFileName(
+            const targetPath = getNewTempManifestFileName(
                urlFileKind,
                fileNumber.toString()
             )

src/utilities/manifestUpdateUtils.test.ts

@@ -0,0 +1,28 @@
+import * as fileUtils from './fileUtils'
+import * as manifestUpdateUtils from './manifestUpdateUtils'
+import * as path from 'path'
+import * as fs from 'fs'
+
+describe('manifestUpdateUtils', () => {
+   jest.spyOn(fileUtils, 'moveFileToTmpDir').mockImplementation((filename) => {
+      return path.join('/tmp', filename)
+   })
+   jest.spyOn(fs, 'writeFileSync').mockImplementation(() => {})
+   jest.spyOn(fs, 'readFileSync').mockImplementation((filename) => {
+      return 'test contents'
+   })
+
+   it('should place all files within the temp dir with the same path that they have in the repo', () => {
+      const originalFilePaths: string[] = [
+         'path/in/repo/test.txt',
+         'path/deeper/in/repo/test.txt'
+      ]
+      const expected: string[] = [
+         '/tmp/path/in/repo/test.txt',
+         '/tmp/path/deeper/in/repo/test.txt'
+      ]
+      const newFilePaths =
+         manifestUpdateUtils.moveFilesToTmpDir(originalFilePaths)
+      expect(newFilePaths).toEqual(expected)
+   })
+})

src/utilities/manifestUpdateUtils.ts

@@ -3,7 +3,7 @@ import * as fs from 'fs'
 import * as yaml from 'js-yaml'
 import * as path from 'path'
 import * as fileHelper from './fileUtils'
-import {getTempDirectory} from './fileUtils'
+import {moveFileToTmpDir} from './fileUtils'
 import {
    InputObjectKindNotDefinedError,
    InputObjectMetadataNotDefinedError,
@@ -26,10 +26,14 @@ export function updateManifestFiles(manifestFilePaths: string[]) {
       throw new Error('Manifest files not provided')
    }
 
+   // move original set of input files to tmp dir
+   const manifestFilesInTempDir = moveFilesToTmpDir(manifestFilePaths)
+
    // update container images
    const containers: string[] = core.getInput('images').split('\n')
+
    const manifestFiles = updateContainerImagesInManifestFiles(
-      manifestFilePaths,
+      manifestFilesInTempDir,
       containers
    )
 
@@ -41,6 +45,12 @@ export function updateManifestFiles(manifestFilePaths: string[]) {
    return updateImagePullSecretsInManifestFiles(manifestFiles, imagePullSecrets)
 }
 
+export function moveFilesToTmpDir(filepaths: string[]): string[] {
+   return filepaths.map((filename) => {
+      return moveFileToTmpDir(filename)
+   })
+}
+
 export function UnsetClusterSpecificDetails(resource: any) {
    if (!resource) {
       return
@@ -70,12 +80,9 @@ function updateContainerImagesInManifestFiles(
 ): string[] {
    if (filePaths?.length <= 0) return filePaths
 
-   const newFilePaths = []
-
    // update container images
    filePaths.forEach((filePath: string) => {
       let contents = fs.readFileSync(filePath).toString()
-
       containers.forEach((container: string) => {
          let [imageName] = container.split(':')
          if (imageName.indexOf('@') > 0) {
@@ -91,13 +98,10 @@ function updateContainerImagesInManifestFiles(
       })
 
       // write updated files
-      const tempDirectory = getTempDirectory()
-      const fileName = path.join(tempDirectory, path.basename(filePath))
-      fs.writeFileSync(path.join(fileName), contents)
-      newFilePaths.push(fileName)
+      fs.writeFileSync(path.join(filePath), contents)
    })
 
-   return newFilePaths
+   return filePaths
 }
 
 /*