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

Supporting Multiple Files With the Same Name #327

Merged
merged 47 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
47 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
15a2e67
refactored getting tmp filename to use entire path, and refactored pr…
jaiveerk Jul 26, 2024
a8164f1
wip
jaiveerk Jul 26, 2024
5f7f8e2
merging master
jaiveerk Jul 30, 2024
c8973e2
merging master
jaiveerk Jul 30, 2024
9a66f13
this should fail
jaiveerk Jul 30, 2024
34db0f3
added UTs
jaiveerk Jul 30, 2024
f70d289
restructured plus UTs plus debug logs
jaiveerk Jul 31, 2024
3c4dde9
resolved dir not existing and UTs
jaiveerk Jul 31, 2024
bcc34e3
cleanup
jaiveerk Jul 31, 2024
48afb64
no silent failure
jaiveerk Jul 31, 2024
d61b384
Reverting private logic
jaiveerk Jul 31, 2024
bda065e
this might work
jaiveerk Aug 1, 2024
986b049
root level files for temp... bizarre issue
jaiveerk Aug 1, 2024
7285df0
need to actually write contents
jaiveerk Aug 1, 2024
8d665db
no more cwdir
jaiveerk Aug 2, 2024
270fda3
moving everything out of temp
jaiveerk Aug 2, 2024
b0454c2
deleting unused function
jaiveerk Aug 2, 2024
be2b57a
supporting windows filepaths for private cluster shallow path generation
jaiveerk Aug 6, 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
4 changes: 4 additions & 0 deletions .github/workflows/run-integration-tests-basic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,13 @@ jobs:
images: nginx:1.14.2
manifests: |
test/integration/manifests/test.yml
test/integration/manifests/manifest_test_dir/test.yml
action: deploy

- name: Checking if deployments and services were created
run: |
python test/integration/k8s-deploy-test.py namespace=${{ env.NAMESPACE }} kind=Deployment name=nginx-deployment containerName=nginx:1.14.2 labels=app:nginx,workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Minikube_Integration_Tests_-_basic selectorLabels=app:nginx
python test/integration/k8s-deploy-test.py namespace=${{ env.NAMESPACE }} kind=Service name=nginx-service labels=workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Minikube_Integration_Tests_-_basic selectorLabels=app:nginx

python test/integration/k8s-deploy-test.py namespace=${{ env.NAMESPACE }} kind=Deployment name=nginx-deployment3 containerName=nginx:1.14.2 labels=app:nginx3,workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Minikube_Integration_Tests_-_basic selectorLabels=app:nginx3
python test/integration/k8s-deploy-test.py namespace=${{ env.NAMESPACE }} kind=Service name=nginx-service3 labels=workflow:actions.github.com-k8s-deploy,workflowFriendlyName:Minikube_Integration_Tests_-_basic selectorLabels=app:nginx3
8 changes: 1 addition & 7 deletions src/strategyHelpers/deploymentHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ import {Kubectl, Resource} from '../types/kubectl'
import {deployPodCanary} from './canary/podCanaryHelper'
import {deploySMICanary} from './canary/smiCanaryHelper'
import {DeploymentConfig} from '../types/deploymentConfig'
import {
deployBlueGreen,
deployBlueGreenIngress,
deployBlueGreenService
} from './blueGreen/deploy'
import {deployBlueGreenSMI} from './blueGreen/deploy'
import {deployBlueGreen} from './blueGreen/deploy'
import {DeploymentStrategy} from '../types/deploymentStrategy'
import * as core from '@actions/core'
import {
Expand All @@ -39,7 +34,6 @@ import {
normalizeWorkflowStrLabel
} from '../utilities/githubUtils'
import {getDeploymentConfig} from '../utilities/dockerUtils'
import {deploy} from '../actions/deploy'
import {DeployResult} from '../types/deployResult'

export async function deployManifests(
Expand Down
35 changes: 25 additions & 10 deletions src/types/privatekubectl.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import * as fileUtils from '../utilities/fileUtils'
import * as fs from 'fs'
import {
PrivateKubectl,
extractFileNames,
replaceFileNamesWithBaseNames
replaceFileNamesWithShallowNamesRelativeToTemp
} from './privatekubectl'
import * as exec from '@actions/exec'

describe('Private kubectl', () => {
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 testString = `kubectl annotate -f /tmp/testdir/test.yml,/tmp/test2.yml,/tmp/testdir/subdir/test3.yml -f /tmp/test4.yml --filename /tmp/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/test.yml"],"helmChartPaths":[],"provider":"GitHub"} --overwrite --namespace test-3498366832`
const mockKube = new PrivateKubectl(
'kubectlPath',
'namespace',
Expand All @@ -15,19 +17,32 @@ describe('Private kubectl', () => {
'resourceName'
)

const spy = jest
.spyOn(fileUtils, 'getTempDirectory')
.mockImplementation(() => {
return '/tmp'
})

jest.spyOn(fs, 'writeFileSync').mockImplementation(() => {})
jest.spyOn(fs, 'readFileSync').mockImplementation((filename) => {
return 'test contents'
})

it('should extract filenames correctly', () => {
expect(extractFileNames(testString)).toEqual([
'testdir/test.yml',
'test2.yml',
'testdir/subdir/test3.yml',
'test4.yml',
'test5.yml'
'/tmp/testdir/test.yml',
'/tmp/test2.yml',
'/tmp/testdir/subdir/test3.yml',
'/tmp/test4.yml',
'/tmp/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`
it('should replace filenames with shallow names for relative locations in tmp correctly', () => {
expect(
replaceFileNamesWithShallowNamesRelativeToTemp(testString)
).toEqual(
`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/test.yml"],"helmChartPaths":[],"provider":"GitHub"} --overwrite --namespace test-3498366832`
)
})

Expand Down
97 changes: 38 additions & 59 deletions src/types/privatekubectl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as core from '@actions/core'
import * as os from 'os'
import * as fs from 'fs'
import * as path from 'path'
import {getTempDirectory} from '../utilities/fileUtils'

export class PrivateKubectl extends Kubectl {
protected async execute(args: string[], silent: boolean = false) {
Expand All @@ -18,8 +19,7 @@ export class PrivateKubectl extends Kubectl {
}

if (this.containsFilenames(kubectlCmd)) {
// For private clusters, files will referenced solely by their basename
kubectlCmd = replaceFileNamesWithBaseNames(kubectlCmd)
kubectlCmd = replaceFileNamesWithShallowNamesRelativeToTemp(kubectlCmd)
addFileFlag = true
}

Expand All @@ -43,22 +43,9 @@ export class PrivateKubectl extends Kubectl {
]

if (addFileFlag) {
const filenames = extractFileNames(kubectlCmd)

const tempDirectory =
process.env['runner.tempDirectory'] || os.tmpdir() + '/manifests'
eo.cwd = tempDirectory
const tempDirectory = getTempDirectory()
eo.cwd = path.join(tempDirectory, 'manifests')
privateClusterArgs.push(...['--file', '.'])

for (const filename of filenames) {
jaiveerk marked this conversation as resolved.
Show resolved Hide resolved
try {
this.moveFileToTempManifestDir(filename)
} catch (e) {
core.debug(
`Error moving file ${filename} to temp directory: ${e}`
)
}
}
}

core.debug(
Expand Down Expand Up @@ -98,63 +85,55 @@ export class PrivateKubectl extends Kubectl {
private containsFilenames(str: string) {
return str.includes('-f ') || str.includes('filename ')
}
}

private createTempManifestsDirectory() {
const manifestsDir = '/tmp/manifests'
if (!fs.existsSync('/tmp/manifests')) {
fs.mkdirSync('/tmp/manifests', {recursive: true})
}
function createTempManifestsDirectory(): string {
const manifestsDirPath = path.join(getTempDirectory(), 'manifests')
if (!fs.existsSync(manifestsDirPath)) {
fs.mkdirSync(manifestsDirPath, {recursive: true})
}

private moveFileToTempManifestDir(file: string) {
this.createTempManifestsDirectory()
if (!fs.existsSync('/tmp/' + file)) {
core.debug(
'/tmp/' +
file +
' does not exist, and therefore cannot be moved to the manifest directory'
)
}

fs.copyFile('/tmp/' + file, '/tmp/manifests/' + file, function (err) {
if (err) {
core.debug(
'Could not rename ' +
'/tmp/' +
file +
' to ' +
'/tmp/manifests/' +
file +
' ERROR: ' +
err
)
return
}
core.debug(
"Successfully moved file '" +
file +
"' from /tmp to /tmp/manifest directory"
)
})
}
return manifestsDirPath
}

export function replaceFileNamesWithBaseNames(kubectlCmd: string) {
export function replaceFileNamesWithShallowNamesRelativeToTemp(
kubectlCmd: string
) {
let filenames = extractFileNames(kubectlCmd)
let basenames = filenames.map((filename) => path.basename(filename))
core.debug(`filenames originally provided in kubectl command: ${filenames}`)
let relativeShallowNames = filenames.map((filename) => {
jaiveerk marked this conversation as resolved.
Show resolved Hide resolved
const relativeName = path.relative(getTempDirectory(), filename)
const shallowName = relativeName.replace(/\//g, '-')
jaiveerk marked this conversation as resolved.
Show resolved Hide resolved

// make manifests dir in temp if it doesn't already exist
const manifestsTempDir = createTempManifestsDirectory()

const shallowPath = path.join(manifestsTempDir, shallowName)
core.debug(
`moving contents from ${filename} to shallow location at ${shallowPath}`
)

core.debug(`reading contents from ${filename}`)
const contents = fs.readFileSync(filename).toString()

core.debug(`writing contents to new path ${shallowPath}`)
fs.writeFileSync(shallowPath, contents)

return shallowName
})

let result = kubectlCmd
if (filenames.length != basenames.length) {
if (filenames.length != relativeShallowNames.length) {
throw Error(
'replacing filenames with basenames, ' +
'replacing filenames with relative path from temp dir, ' +
filenames.length +
' filenames != ' +
basenames.length +
relativeShallowNames.length +
'basenames'
)
}
for (let index = 0; index < filenames.length; index++) {
result = result.replace(filenames[index], basenames[index])
result = result.replace(filenames[index], relativeShallowNames[index])
}
return result
}
Expand Down
56 changes: 34 additions & 22 deletions src/utilities/fileUtils.test.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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()))
}
})
})
Expand All @@ -72,7 +66,7 @@ describe('File utils', () => {
)

expect(
getFilesFromDirectoriesAndURLs([badPath, goodPath])
fileUtils.getFilesFromDirectoriesAndURLs([badPath, goodPath])
).rejects.toThrowError()
})

Expand All @@ -92,7 +86,7 @@ describe('File utils', () => {
)

expect(
await getFilesFromDirectoriesAndURLs([
await fileUtils.getFilesFromDirectoriesAndURLs([
outerPath,
fileAtOuter,
innerPath
Expand All @@ -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')
)
})
})
Loading
Loading