Private Cluster Bugfix - Issue with Multiple Files (#325)

* 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 8da79a8. * fixing labels test * pretty --------- Co-authored-by: David Gamero <[email protected]>
Azure · Jul 25, 2024 · 00795b0 · 00795b0
1 parent d565a17
commit 00795b0
Show file tree

Hide file tree

Showing 6 changed files with 124 additions and 67 deletions.
diff --git a/.github/workflows/run-integration-tests-private.yml b/.github/workflows/run-integration-tests-private.yml
@@ -43,7 +43,7 @@ jobs:
            run: |
               set +x
               # create cluster
-              az group create --location eastus --name ${{ env.NAMESPACE }}
+              az group create --location eastus2 --name ${{ env.NAMESPACE }}
               az aks create --name ${{ env.NAMESPACE }} --resource-group ${{ env.NAMESPACE }} --enable-private-cluster --generate-ssh-keys
               az aks get-credentials --resource-group ${{ env.NAMESPACE }} --name ${{ env.NAMESPACE }}
 
@@ -63,6 +63,7 @@ jobs:
               images: nginx:1.14.2
               manifests: |
                  test/integration/manifests/test.yml
+                 test/integration/manifests/test2.yml
               action: deploy
               private-cluster: true
               resource-group: ${{ env.NAMESPACE }}
@@ -73,6 +74,9 @@ jobs:
               python test/integration/k8s-deploy-test.py private=${{ env.NAMESPACE }} namespace=${{ env.NAMESPACE }} kind=Deployment name=nginx-deployment containerName=nginx:1.14.2 labels=app:nginx,workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Cluster_Integration_Tests_-_private_cluster selectorLabels=app:nginx
               python test/integration/k8s-deploy-test.py private=${{ env.NAMESPACE }} namespace=${{ env.NAMESPACE }} kind=Service name=nginx-service labels=workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Cluster_Integration_Tests_-_private_cluster selectorLabels=app:nginx
 
+              python test/integration/k8s-deploy-test.py private=${{ env.NAMESPACE }} namespace=${{ env.NAMESPACE }} kind=Deployment name=nginx-deployment2 containerName=nginx:1.14.2 labels=app:nginx2,workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Cluster_Integration_Tests_-_private_cluster selectorLabels=app:nginx2
+              python test/integration/k8s-deploy-test.py private=${{ env.NAMESPACE }} namespace=${{ env.NAMESPACE }} kind=Service name=nginx-service2 labels=workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Cluster_Integration_Tests_-_private_cluster selectorLabels=app:nginx2
+
          - name: Clean up AKS cluster
            if: ${{ always() }}
            run: |

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -27,7 +27,7 @@
       "@types/node": "^12.20.41",
       "@vercel/ncc": "^0.36.1",
       "jest": "^26.0.0",
-      "prettier": "^2.7.1",
+      "prettier": "^2.8.8",
       "ts-jest": "^26.0.0",
       "typescript": "3.9.5"
    }

diff --git a/src/types/privatekubectl.test.ts b/src/types/privatekubectl.test.ts
@@ -1,8 +1,12 @@
-import {PrivateKubectl} from './privatekubectl'
+import {
+   PrivateKubectl,
+   extractFileNames,
+   replaceFileNamesWithBaseNames
+} from './privatekubectl'
 import * as exec from '@actions/exec'
 
 describe('Private kubectl', () => {
-   const testString = `kubectl annotate -f test.yml,test2.yml,test3.yml -f test4.yml --filename test5.yml actions.github.com/k8s-deploy={"run":"3498366832","repository":"jaiveerk/k8s-deploy","workflow":"Minikube Integration Tests - private cluster","workflowFileName":"run-integration-tests-private.yml","jobName":"run-integration-test","createdBy":"jaiveerk","runUri":"https://github.com/jaiveerk/k8s-deploy/actions/runs/3498366832","commit":"c63b323186ea1320a31290de6dcc094c06385e75","lastSuccessRunCommit":"NA","branch":"refs/heads/main","deployTimestamp":1668787848577,"dockerfilePaths":{"nginx:1.14.2":""},"manifestsPaths":["https://github.com/jaiveerk/k8s-deploy/blob/c63b323186ea1320a31290de6dcc094c06385e75/test/integration/manifests/test.yml"],"helmChartPaths":[],"provider":"GitHub"} --overwrite --namespace test-3498366832`
+   const testString = `kubectl annotate -f testdir/test.yml,test2.yml,testdir/subdir/test3.yml -f test4.yml --filename test5.yml actions.github.com/k8s-deploy={"run":"3498366832","repository":"jaiveerk/k8s-deploy","workflow":"Minikube Integration Tests - private cluster","workflowFileName":"run-integration-tests-private.yml","jobName":"run-integration-test","createdBy":"jaiveerk","runUri":"https://github.com/jaiveerk/k8s-deploy/actions/runs/3498366832","commit":"c63b323186ea1320a31290de6dcc094c06385e75","lastSuccessRunCommit":"NA","branch":"refs/heads/main","deployTimestamp":1668787848577,"dockerfilePaths":{"nginx:1.14.2":""},"manifestsPaths":["https://github.com/jaiveerk/k8s-deploy/blob/c63b323186ea1320a31290de6dcc094c06385e75/test/integration/manifests/test.yml"],"helmChartPaths":[],"provider":"GitHub"} --overwrite --namespace test-3498366832`
    const mockKube = new PrivateKubectl(
       'kubectlPath',
       'namespace',
@@ -12,8 +16,18 @@ describe('Private kubectl', () => {
    )
 
    it('should extract filenames correctly', () => {
-      expect(mockKube.extractFilesnames(testString)).toEqual(
-         'test.yml test2.yml test3.yml test4.yml test5.yml'
+      expect(extractFileNames(testString)).toEqual([
+         'testdir/test.yml',
+         'test2.yml',
+         'testdir/subdir/test3.yml',
+         'test4.yml',
+         'test5.yml'
+      ])
+   })
+
+   it('should replace filenames with basenames correctly', () => {
+      expect(replaceFileNamesWithBaseNames(testString)).toEqual(
+         `kubectl annotate -f test.yml,test2.yml,test3.yml -f test4.yml --filename test5.yml actions.github.com/k8s-deploy={"run":"3498366832","repository":"jaiveerk/k8s-deploy","workflow":"Minikube Integration Tests - private cluster","workflowFileName":"run-integration-tests-private.yml","jobName":"run-integration-test","createdBy":"jaiveerk","runUri":"https://github.com/jaiveerk/k8s-deploy/actions/runs/3498366832","commit":"c63b323186ea1320a31290de6dcc094c06385e75","lastSuccessRunCommit":"NA","branch":"refs/heads/main","deployTimestamp":1668787848577,"dockerfilePaths":{"nginx:1.14.2":""},"manifestsPaths":["https://github.com/jaiveerk/k8s-deploy/blob/c63b323186ea1320a31290de6dcc094c06385e75/test/integration/manifests/test.yml"],"helmChartPaths":[],"provider":"GitHub"} --overwrite --namespace test-3498366832`
       )
    })
 

diff --git a/src/types/privatekubectl.ts b/src/types/privatekubectl.ts
@@ -19,7 +19,7 @@ export class PrivateKubectl extends Kubectl {
 
       if (this.containsFilenames(kubectlCmd)) {
          // For private clusters, files will referenced solely by their basename
-         kubectlCmd = this.replaceFilnamesWithBasenames(kubectlCmd)
+         kubectlCmd = replaceFileNamesWithBaseNames(kubectlCmd)
          addFileFlag = true
       }
 
@@ -43,21 +43,21 @@ export class PrivateKubectl extends Kubectl {
       ]
 
       if (addFileFlag) {
-         const filenames = this.extractFilesnames(kubectlCmd).split(' ')
+         const filenames = extractFileNames(kubectlCmd)
 
          const tempDirectory =
             process.env['runner.tempDirectory'] || os.tmpdir() + '/manifests'
          eo.cwd = tempDirectory
          privateClusterArgs.push(...['--file', '.'])
 
-         let filenamesArr = filenames[0].split(',')
-         for (let index = 0; index < filenamesArr.length; index++) {
-            const file = filenamesArr[index]
-
-            if (!file) {
-               continue
+         for (const filename of filenames) {
+            try {
+               this.moveFileToTempManifestDir(filename)
+            } catch (e) {
+               core.debug(
+                  `Error moving file ${filename} to temp directory: ${e}`
+               )
             }
-            this.moveFileToTempManifestDir(file)
          }
       }
 
@@ -95,49 +95,6 @@ export class PrivateKubectl extends Kubectl {
       } as ExecOutput
    }
 
-   private replaceFilnamesWithBasenames(kubectlCmd: string) {
-      let exFilenames = this.extractFilesnames(kubectlCmd)
-      let filenames = exFilenames.split(' ')
-      let filenamesArr = filenames[0].split(',')
-
-      for (let index = 0; index < filenamesArr.length; index++) {
-         filenamesArr[index] = path.basename(filenamesArr[index])
-      }
-
-      let baseFilenames = filenamesArr.join()
-
-      let result = kubectlCmd.replace(exFilenames, baseFilenames)
-      return result
-   }
-
-   public extractFilesnames(strToParse: string) {
-      const fileNames: string[] = []
-      const argv = minimist(strToParse.split(' '))
-      const fArg = 'f'
-      const filenameArg = 'filename'
-
-      fileNames.push(...this.extractFilesFromMinimist(argv, fArg))
-      fileNames.push(...this.extractFilesFromMinimist(argv, filenameArg))
-
-      return fileNames.join(' ')
-   }
-
-   private extractFilesFromMinimist(argv, arg: string): string[] {
-      if (!argv[arg]) {
-         return []
-      }
-      const toReturn: string[] = []
-      if (typeof argv[arg] === 'string') {
-         toReturn.push(...argv[arg].split(','))
-      } else {
-         for (const value of argv[arg] as string[]) {
-            toReturn.push(...value.split(','))
-         }
-      }
-
-      return toReturn
-   }
-
    private containsFilenames(str: string) {
       return str.includes('-f ') || str.includes('filename ')
    }
@@ -181,3 +138,51 @@ export class PrivateKubectl extends Kubectl {
       })
    }
 }
+
+export function replaceFileNamesWithBaseNames(kubectlCmd: string) {
+   let filenames = extractFileNames(kubectlCmd)
+   let basenames = filenames.map((filename) => path.basename(filename))
+
+   let result = kubectlCmd
+   if (filenames.length != basenames.length) {
+      throw Error(
+         'replacing filenames with basenames, ' +
+            filenames.length +
+            ' filenames != ' +
+            basenames.length +
+            'basenames'
+      )
+   }
+   for (let index = 0; index < filenames.length; index++) {
+      result = result.replace(filenames[index], basenames[index])
+   }
+   return result
+}
+
+export function extractFileNames(strToParse: string) {
+   const fileNames: string[] = []
+   const argv = minimist(strToParse.split(' '))
+   const fArg = 'f'
+   const filenameArg = 'filename'
+
+   fileNames.push(...extractFilesFromMinimist(argv, fArg))
+   fileNames.push(...extractFilesFromMinimist(argv, filenameArg))
+
+   return fileNames
+}
+
+export function extractFilesFromMinimist(argv, arg: string): string[] {
+   if (!argv[arg]) {
+      return []
+   }
+   const toReturn: string[] = []
+   if (typeof argv[arg] === 'string') {
+      toReturn.push(...argv[arg].split(','))
+   } else {
+      for (const value of argv[arg] as string[]) {
+         toReturn.push(...value.split(','))
+      }
+   }
+
+   return toReturn
+}
diff --git a/test/integration/manifests/test2.yml b/test/integration/manifests/test2.yml
@@ -0,0 +1,33 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+   name: nginx-deployment2
+   labels:
+      app: nginx2
+spec:
+   replicas: 1
+   selector:
+      matchLabels:
+         app: nginx2
+   template:
+      metadata:
+         labels:
+            app: nginx2
+      spec:
+         containers:
+            - name: nginx
+              image: nginx
+              ports:
+                 - containerPort: 80
+---
+apiVersion: v1
+kind: Service
+metadata:
+   name: nginx-service2
+spec:
+   selector:
+      app: nginx2
+   ports:
+      - protocol: TCP
+        port: 80
+        targetPort: 80