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 8da79a8190.

* fixing labels test

* pretty

---------

Co-authored-by: David Gamero <david340804@gmail.com>
This commit is contained in:
Jaiveer Katariya
2024-07-25 14:47:27 -04:00
committed by GitHub
parent d565a17533
commit 00795b0b56
6 changed files with 124 additions and 67 deletions
+18 -4
View File
@@ -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`
)
})
+57 -52
View File
@@ -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
}