Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private Cluster Bugfix - Issue with Multiple Files #325

Merged
merged 29 commits into from
Jul 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e896fce
changed ubuntu runner
jaiveerk Nov 22, 2022
117b901
changed minikube action
jaiveerk Nov 22, 2022
73abb8c
Version formatting
jaiveerk Nov 22, 2022
6754b69
nonedriveR
jaiveerk Nov 22, 2022
2f61ce3
update kube version
jaiveerk Nov 22, 2022
d94c1c2
installing conntrack'
jaiveerk Nov 22, 2022
f07d12f
updated other actions
jaiveerk Nov 22, 2022
eeab9fd
update bg ingress api version
jaiveerk Nov 22, 2022
bd9e2cd
prettify
jaiveerk Nov 22, 2022
ecc0ab0
updated ingress backend for new api version
jaiveerk Nov 22, 2022
a09e2b9
Added path type
jaiveerk Nov 22, 2022
9142d3b
prettify
jaiveerk Nov 22, 2022
7adeb4e
Merge branch 'main' of github.com:jaiveerk/k8s-deploy
jaiveerk Dec 7, 2022
724205a
added logging
jaiveerk Dec 7, 2022
6d9b280
added try catch logic to prevent future failures if annotations fail …
jaiveerk Dec 7, 2022
1403cdb
added nullcheck
jaiveerk Dec 7, 2022
55e9785
Added fallback filename if workflow fails to get github filepath due …
jaiveerk Dec 8, 2022
f507264
cleanup
jaiveerk Dec 8, 2022
86aca60
added oliver's feedback + unit test demonstrating regex glitch and fix
jaiveerk Dec 12, 2022
0b82897
no longer using blank string for failed regex
jaiveerk Dec 12, 2022
66eca59
add tests and dont split so much
davidgamero Jul 24, 2024
f846051
updating main
jaiveerk Jul 24, 2024
6bd6f66
testing
jaiveerk Jul 24, 2024
27ed52f
Merge remote-tracking branch 'upstream/fix-private-cluster-file-flag'
jaiveerk Jul 24, 2024
e437339
file fix
jaiveerk Jul 24, 2024
8da79a8
without fix
jaiveerk Jul 24, 2024
91606b8
Revert "without fix"
jaiveerk Jul 24, 2024
4b9af6f
fixing labels test
jaiveerk Jul 24, 2024
c082d00
pretty
jaiveerk Jul 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/run-integration-tests-private.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}

Expand All @@ -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 }}
Expand All @@ -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: |
Expand Down
19 changes: 10 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
22 changes: 18 additions & 4 deletions src/types/privatekubectl.test.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -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`
)
})

Expand Down
109 changes: 57 additions & 52 deletions src/types/privatekubectl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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 ')
}
Expand Down Expand Up @@ -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
}
33 changes: 33 additions & 0 deletions test/integration/manifests/test2.yml
Original file line number Diff line number Diff line change
@@ -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
Loading