From 65e12248468f7e4fa5ea6469bb9885e7c9926686 Mon Sep 17 00:00:00 2001 From: Koushik Dey Date: Thu, 27 Aug 2020 17:14:59 +0530 Subject: [PATCH] Addressed review comments. --- __tests__/run.test.ts | 12 +-- lib/githubClient.js | 25 +++---- lib/kubectl-object-model.js | 22 ++---- .../strategy-helpers/deployment-helper.js | 6 +- lib/utilities/utility.js | 75 ++++++++++--------- src/constants.ts | 1 + src/githubClient.ts | 26 +++---- src/kubectl-object-model.ts | 16 ++-- .../strategy-helpers/deployment-helper.ts | 6 +- src/utilities/utility.ts | 75 ++++++++++--------- 10 files changed, 133 insertions(+), 131 deletions(-) diff --git a/__tests__/run.test.ts b/__tests__/run.test.ts index 6132a9fb..a130584b 100644 --- a/__tests__/run.test.ts +++ b/__tests__/run.test.ts @@ -312,11 +312,11 @@ test("deployment - deploy() - Annotate & label resources", async () => { kubeCtl.labelFiles = jest.fn(); //Invoke and assert await expect(deployment.deploy(kubeCtl, ['manifests/deployment.yaml'], undefined)).resolves.not.toThrowError(); - expect(kubeCtl.annotate).toHaveBeenNthCalledWith(1, 'namespace', 'default', [annotationKeyValStr], true); - expect(kubeCtl.annotateFiles).toBeCalledWith(["~/Deployment_testapp_currentTimestamp"], [annotationKeyValStr], true); + expect(kubeCtl.annotate).toHaveBeenNthCalledWith(1, 'namespace', 'default', annotationKeyValStr); + expect(kubeCtl.annotateFiles).toBeCalledWith(["~/Deployment_testapp_currentTimestamp"], annotationKeyValStr); expect(kubeCtl.annotate).toBeCalledTimes(2); expect(kubeCtl.labelFiles).toBeCalledWith(["~/Deployment_testapp_currentTimestamp"], - [`workflowFriendlyName=workflow.yml`, `workflow=${getWorkflowAnnotationKeyLabel(process.env.GITHUB_WORKFLOW)}`], true); + [`workflowFriendlyName=workflow.yml`, `workflow=${getWorkflowAnnotationKeyLabel(process.env.GITHUB_WORKFLOW)}`]); }); test("deployment - deploy() - Annotate & label resources for a new workflow", async () => { @@ -340,11 +340,11 @@ test("deployment - deploy() - Annotate & label resources for a new workflow", as kubeCtl.labelFiles = jest.fn(); //Invoke and assert await expect(deployment.deploy(kubeCtl, ['manifests/deployment.yaml'], undefined)).resolves.not.toThrowError(); - expect(kubeCtl.annotate).toHaveBeenNthCalledWith(1, 'namespace', 'default', [annotationKeyValStr], true); - expect(kubeCtl.annotateFiles).toBeCalledWith(["~/Deployment_testapp_currentTimestamp"], [annotationKeyValStr], true); + expect(kubeCtl.annotate).toHaveBeenNthCalledWith(1, 'namespace', 'default', annotationKeyValStr); + expect(kubeCtl.annotateFiles).toBeCalledWith(["~/Deployment_testapp_currentTimestamp"], annotationKeyValStr); expect(kubeCtl.annotate).toBeCalledTimes(2); expect(kubeCtl.labelFiles).toBeCalledWith(["~/Deployment_testapp_currentTimestamp"], - [`workflowFriendlyName=${process.env.GITHUB_WORKFLOW}`, `workflow=${getWorkflowAnnotationKeyLabel(process.env.GITHUB_WORKFLOW)}`], true); + [`workflowFriendlyName=${process.env.GITHUB_WORKFLOW}`, `workflow=${getWorkflowAnnotationKeyLabel(process.env.GITHUB_WORKFLOW)}`]); }); test("deployment - deploy() - Annotate resources failed", async () => { diff --git a/lib/githubClient.js b/lib/githubClient.js index 7de8bf6c..54d3f5c7 100644 --- a/lib/githubClient.js +++ b/lib/githubClient.js @@ -17,21 +17,18 @@ class GitHubClient { this._repository = repository; this._token = token; } - getWorkflows(force) { + getWorkflows() { return __awaiter(this, void 0, void 0, function* () { - if (force || !this._workflowsPromise) { - const getWorkflowFileNameUrl = `https://api.github.com/repos/${this._repository}/actions/workflows`; - const webRequest = new httpClient_1.WebRequest(); - webRequest.method = "GET"; - webRequest.uri = getWorkflowFileNameUrl; - webRequest.headers = { - Authorization: `Bearer ${this._token}` - }; - core.debug(`Getting workflows for repo: ${this._repository}`); - const response = yield httpClient_1.sendRequest(webRequest); - this._workflowsPromise = Promise.resolve(response); - } - return this._workflowsPromise; + const getWorkflowFileNameUrl = `https://api.github.com/repos/${this._repository}/actions/workflows`; + const webRequest = new httpClient_1.WebRequest(); + webRequest.method = "GET"; + webRequest.uri = getWorkflowFileNameUrl; + webRequest.headers = { + Authorization: `Bearer ${this._token}` + }; + core.debug(`Getting workflows for repo: ${this._repository}`); + const response = yield httpClient_1.sendRequest(webRequest); + return Promise.resolve(response); }); } } diff --git a/lib/kubectl-object-model.js b/lib/kubectl-object-model.js index b835b7e9..868143e6 100644 --- a/lib/kubectl-object-model.js +++ b/lib/kubectl-object-model.js @@ -37,30 +37,24 @@ class Kubectl { } return newReplicaSet; } - annotate(resourceType, resourceName, annotations, overwrite) { + annotate(resourceType, resourceName, annotation) { let args = ['annotate', resourceType, resourceName]; - args = args.concat(annotations); - if (!!overwrite) { - args.push(`--overwrite`); - } + args.push(annotation); + args.push(`--overwrite`); return this.execute(args); } - annotateFiles(files, annotations, overwrite) { + annotateFiles(files, annotation) { let args = ['annotate']; args = args.concat(['-f', this.createInlineArray(files)]); - args = args.concat(annotations); - if (!!overwrite) { - args.push(`--overwrite`); - } + args.push(annotation); + args.push(`--overwrite`); return this.execute(args); } - labelFiles(files, labels, overwrite) { + labelFiles(files, labels) { let args = ['label']; args = args.concat(['-f', this.createInlineArray(files)]); args = args.concat(labels); - if (!!overwrite) { - args.push(`--overwrite`); - } + args.push(`--overwrite`); return this.execute(args); } getAllPods() { diff --git a/lib/utilities/strategy-helpers/deployment-helper.js b/lib/utilities/strategy-helpers/deployment-helper.js index 0c2f6d8f..d482e853 100644 --- a/lib/utilities/strategy-helpers/deployment-helper.js +++ b/lib/utilities/strategy-helpers/deployment-helper.js @@ -122,8 +122,8 @@ function annotateResources(files, kubectl, resourceTypes, allPods, annotationKey const annotateResults = []; const lastSuccessSha = utility_1.getLastSuccessfulRunSha(kubectl, TaskInputParameters.namespace, annotationKey); let annotationKeyValStr = annotationKey + '=' + models.getWorkflowAnnotationsJson(lastSuccessSha); - annotateResults.push(kubectl.annotate('namespace', TaskInputParameters.namespace, [annotationKeyValStr], true)); - annotateResults.push(kubectl.annotateFiles(files, [annotationKeyValStr], true)); + annotateResults.push(kubectl.annotate('namespace', TaskInputParameters.namespace, annotationKeyValStr)); + annotateResults.push(kubectl.annotateFiles(files, annotationKeyValStr)); resourceTypes.forEach(resource => { if (resource.type.toUpperCase() !== models.KubernetesWorkload.pod.toUpperCase()) { utility_1.annotateChildPods(kubectl, resource.type, resource.name, annotationKeyValStr, allPods) @@ -137,7 +137,7 @@ function labelResources(files, kubectl, label) { workflowName = workflowName.startsWith('.github/workflows/') ? workflowName.replace(".github/workflows/", "") : workflowName; const labels = [`workflowFriendlyName=${workflowName}`, `workflow=${label}`]; - utility_1.checkForErrors([kubectl.labelFiles(files, labels, true)], true); + utility_1.checkForErrors([kubectl.labelFiles(files, labels)], true); } function updateResourceObjects(filePaths, imagePullSecrets, containers) { const newObjectsList = []; diff --git a/lib/utilities/utility.js b/lib/utilities/utility.js index fb71534c..0c419d3d 100644 --- a/lib/utilities/utility.js +++ b/lib/utilities/utility.js @@ -28,7 +28,7 @@ function isEqual(str1, str2, ignoreCase) { if (str1 == null || str2 == null) { return false; } - if (!!ignoreCase) { + if (ignoreCase) { return str1.toUpperCase() === str2.toUpperCase(); } else { @@ -40,7 +40,7 @@ function checkForErrors(execResults, warnIfError) { if (execResults.length !== 0) { let stderr = ''; execResults.forEach(result => { - if (!!result && !!result.stderr) { + if (result && result.stderr) { if (result.code !== 0) { stderr += result.stderr + '\n'; } @@ -50,7 +50,7 @@ function checkForErrors(execResults, warnIfError) { } }); if (stderr.length > 0) { - if (!!warnIfError) { + if (warnIfError) { core.warning(stderr.trim()); } else { @@ -61,25 +61,27 @@ function checkForErrors(execResults, warnIfError) { } exports.checkForErrors = checkForErrors; function getLastSuccessfulRunSha(kubectl, namespaceName, annotationKey) { - const result = kubectl.getResource('namespace', namespaceName); - if (!result) { - core.debug(`Failed to get commits from cluster.`); - return ''; + try { + const result = kubectl.getResource('namespace', namespaceName); + if (result) { + if (result.stderr) { + core.warning(`${result.stderr}`); + return process.env.GITHUB_SHA; + } + else if (result.stdout) { + const annotationsSet = JSON.parse(result.stdout).metadata.annotations; + if (annotationsSet && annotationsSet[annotationKey]) { + return JSON.parse(annotationsSet[annotationKey].replace(/'/g, '"')).commit; + } + else { + return 'NA'; + } + } + } } - else { - if (!!result.stderr) { - core.debug(`${result.stderr}`); - return process.env.GITHUB_SHA; - } - else if (!!result.stdout) { - const annotationsSet = JSON.parse(result.stdout).metadata.annotations; - if (!!annotationsSet && !!annotationsSet[annotationKey]) { - return JSON.parse(annotationsSet[annotationKey].replace(/'/g, '"')).commit; - } - else { - return 'NA'; - } - } + catch (ex) { + core.warning(`Failed to get commits from cluster. ${JSON.stringify(ex)}`); + return ''; } } exports.getLastSuccessfulRunSha = getLastSuccessfulRunSha; @@ -89,20 +91,25 @@ function getWorkflowFilePath(githubToken) { if (!workflowFilePath.startsWith('.github/workflows/')) { const githubClient = new githubClient_1.GitHubClient(process.env.GITHUB_REPOSITORY, githubToken); const response = yield githubClient.getWorkflows(); - if (response.statusCode == httpClient_1.StatusCodes.OK - && !!response.body - && !!response.body.total_count) { - if (response.body.total_count > 0) { - for (let workflow of response.body.workflows) { - if (process.env.GITHUB_WORKFLOW === workflow.name) { - workflowFilePath = workflow.path; - break; + if (response) { + if (response.statusCode == httpClient_1.StatusCodes.OK + && response.body + && response.body.total_count) { + if (response.body.total_count > 0) { + for (let workflow of response.body.workflows) { + if (process.env.GITHUB_WORKFLOW === workflow.name) { + workflowFilePath = workflow.path; + break; + } } } } + else if (response.statusCode != httpClient_1.StatusCodes.OK) { + core.debug(`An error occured while getting list of workflows on the repo. Statuscode: ${response.statusCode}, StatusMessage: ${response.statusMessage}`); + } } - else if (response.statusCode != httpClient_1.StatusCodes.OK) { - core.debug(`An error occured while getting list of workflows on the repo. Statuscode: ${response.statusCode}, StatusMessage: ${response.statusMessage}`); + else { + core.warning(`Failed to get response from workflow list API`); } } return Promise.resolve(workflowFilePath); @@ -115,13 +122,13 @@ function annotateChildPods(kubectl, resourceType, resourceName, annotationKeyVal if (resourceType.toLowerCase().indexOf('deployment') > -1) { owner = kubectl.getNewReplicaSet(resourceName); } - if (!!allPods && !!allPods.items && allPods.items.length > 0) { + if (allPods && allPods.items && allPods.items.length > 0) { allPods.items.forEach((pod) => { const owners = pod.metadata.ownerReferences; - if (!!owners) { + if (owners) { owners.forEach(ownerRef => { if (ownerRef.name === owner) { - commandExecutionResults.push(kubectl.annotate('pod', pod.metadata.name, [annotationKeyValStr], true)); + commandExecutionResults.push(kubectl.annotate('pod', pod.metadata.name, annotationKeyValStr)); } }); } diff --git a/src/constants.ts b/src/constants.ts index b2dc7e44..d29d3235 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -40,6 +40,7 @@ export function getWorkflowAnnotationsJson(lastSuccessRunSha: string): string { + `'provider': 'GitHub'` + `}`; } + export function getWorkflowAnnotationKeyLabel(workflowFilePath: string): string { const hashKey = require("crypto").createHash("MD5") .update(`${process.env.GITHUB_REPOSITORY}/${workflowFilePath}`) diff --git a/src/githubClient.ts b/src/githubClient.ts index b1cc4639..26c1c811 100644 --- a/src/githubClient.ts +++ b/src/githubClient.ts @@ -7,24 +7,20 @@ export class GitHubClient { this._token = token; } - public async getWorkflows(force?: boolean): Promise { - if (force || !this._workflowsPromise) { - const getWorkflowFileNameUrl = `https://api.github.com/repos/${this._repository}/actions/workflows`; - const webRequest = new WebRequest(); - webRequest.method = "GET"; - webRequest.uri = getWorkflowFileNameUrl; - webRequest.headers = { - Authorization: `Bearer ${this._token}` - }; + public async getWorkflows(): Promise { + const getWorkflowFileNameUrl = `https://api.github.com/repos/${this._repository}/actions/workflows`; + const webRequest = new WebRequest(); + webRequest.method = "GET"; + webRequest.uri = getWorkflowFileNameUrl; + webRequest.headers = { + Authorization: `Bearer ${this._token}` + }; - core.debug(`Getting workflows for repo: ${this._repository}`); - const response: WebResponse = await sendRequest(webRequest); - this._workflowsPromise = Promise.resolve(response); - } - return this._workflowsPromise; + core.debug(`Getting workflows for repo: ${this._repository}`); + const response: WebResponse = await sendRequest(webRequest); + return Promise.resolve(response); } private _repository: string; private _token: string; - private _workflowsPromise: Promise; } \ No newline at end of file diff --git a/src/kubectl-object-model.ts b/src/kubectl-object-model.ts index 65d354ea..eb452cf7 100644 --- a/src/kubectl-object-model.ts +++ b/src/kubectl-object-model.ts @@ -50,26 +50,26 @@ export class Kubectl { return newReplicaSet; } - public annotate(resourceType: string, resourceName: string, annotations: string[], overwrite?: boolean): IExecSyncResult { + public annotate(resourceType: string, resourceName: string, annotation: string): IExecSyncResult { let args = ['annotate', resourceType, resourceName]; - args = args.concat(annotations); - if (!!overwrite) { args.push(`--overwrite`); } + args.push(annotation); + args.push(`--overwrite`); return this.execute(args); } - public annotateFiles(files: string | string[], annotations: string[], overwrite?: boolean): IExecSyncResult { + public annotateFiles(files: string | string[], annotation: string): IExecSyncResult { let args = ['annotate']; args = args.concat(['-f', this.createInlineArray(files)]); - args = args.concat(annotations); - if (!!overwrite) { args.push(`--overwrite`); } + args.push(annotation); + args.push(`--overwrite`); return this.execute(args); } - public labelFiles(files: string | string[], labels: string[], overwrite?: boolean): IExecSyncResult { + public labelFiles(files: string | string[], labels: string[]): IExecSyncResult { let args = ['label']; args = args.concat(['-f', this.createInlineArray(files)]); args = args.concat(labels); - if (!!overwrite) { args.push(`--overwrite`); } + args.push(`--overwrite`); return this.execute(args); } diff --git a/src/utilities/strategy-helpers/deployment-helper.ts b/src/utilities/strategy-helpers/deployment-helper.ts index 9aaf6209..68c746a9 100644 --- a/src/utilities/strategy-helpers/deployment-helper.ts +++ b/src/utilities/strategy-helpers/deployment-helper.ts @@ -123,8 +123,8 @@ function annotateResources(files: string[], kubectl: Kubectl, resourceTypes: Res const annotateResults: IExecSyncResult[] = []; const lastSuccessSha = getLastSuccessfulRunSha(kubectl, TaskInputParameters.namespace, annotationKey); let annotationKeyValStr = annotationKey + '=' + models.getWorkflowAnnotationsJson(lastSuccessSha); - annotateResults.push(kubectl.annotate('namespace', TaskInputParameters.namespace, [annotationKeyValStr], true)); - annotateResults.push(kubectl.annotateFiles(files, [annotationKeyValStr], true)); + annotateResults.push(kubectl.annotate('namespace', TaskInputParameters.namespace, annotationKeyValStr)); + annotateResults.push(kubectl.annotateFiles(files, annotationKeyValStr)); resourceTypes.forEach(resource => { if (resource.type.toUpperCase() !== models.KubernetesWorkload.pod.toUpperCase()) { annotateChildPods(kubectl, resource.type, resource.name, annotationKeyValStr, allPods) @@ -139,7 +139,7 @@ function labelResources(files: string[], kubectl: Kubectl, label: string) { workflowName = workflowName.startsWith('.github/workflows/') ? workflowName.replace(".github/workflows/", "") : workflowName; const labels = [`workflowFriendlyName=${workflowName}`, `workflow=${label}`]; - checkForErrors([kubectl.labelFiles(files, labels, true)], true); + checkForErrors([kubectl.labelFiles(files, labels)], true); } function updateResourceObjects(filePaths: string[], imagePullSecrets: string[], containers: string[]): string[] { diff --git a/src/utilities/utility.ts b/src/utilities/utility.ts index fc8119f7..e75e1387 100644 --- a/src/utilities/utility.ts +++ b/src/utilities/utility.ts @@ -22,7 +22,7 @@ export function isEqual(str1: string, str2: string, ignoreCase?: boolean): boole return false; } - if (!!ignoreCase) { + if (ignoreCase) { return str1.toUpperCase() === str2.toUpperCase(); } else { return str1 === str2; @@ -33,7 +33,7 @@ export function checkForErrors(execResults: IExecSyncResult[], warnIfError?: boo if (execResults.length !== 0) { let stderr = ''; execResults.forEach(result => { - if (!!result && !!result.stderr) { + if (result && result.stderr) { if (result.code !== 0) { stderr += result.stderr + '\n'; } else { @@ -42,7 +42,7 @@ export function checkForErrors(execResults: IExecSyncResult[], warnIfError?: boo } }); if (stderr.length > 0) { - if (!!warnIfError) { + if (warnIfError) { core.warning(stderr.trim()); } else { throw new Error(stderr.trim()); @@ -52,25 +52,27 @@ export function checkForErrors(execResults: IExecSyncResult[], warnIfError?: boo } export function getLastSuccessfulRunSha(kubectl: Kubectl, namespaceName: string, annotationKey: string): string { - const result = kubectl.getResource('namespace', namespaceName); - if (!result) { - core.debug(`Failed to get commits from cluster.`); - return ''; + try { + const result = kubectl.getResource('namespace', namespaceName); + if (result) { + if (result.stderr) { + core.warning(`${result.stderr}`); + return process.env.GITHUB_SHA; + } + else if (result.stdout) { + const annotationsSet = JSON.parse(result.stdout).metadata.annotations; + if (annotationsSet && annotationsSet[annotationKey]) { + return JSON.parse(annotationsSet[annotationKey].replace(/'/g, '"')).commit; + } + else { + return 'NA'; + } + } + } } - else { - if (!!result.stderr) { - core.debug(`${result.stderr}`); - return process.env.GITHUB_SHA; - } - else if (!!result.stdout) { - const annotationsSet = JSON.parse(result.stdout).metadata.annotations; - if (!!annotationsSet && !!annotationsSet[annotationKey]) { - return JSON.parse(annotationsSet[annotationKey].replace(/'/g, '"')).commit; - } - else { - return 'NA'; - } - } + catch (ex) { + core.warning(`Failed to get commits from cluster. ${JSON.stringify(ex)}`); + return ''; } } @@ -79,20 +81,25 @@ export async function getWorkflowFilePath(githubToken: string): Promise if (!workflowFilePath.startsWith('.github/workflows/')) { const githubClient = new GitHubClient(process.env.GITHUB_REPOSITORY, githubToken); const response = await githubClient.getWorkflows(); - if (response.statusCode == StatusCodes.OK - && !!response.body - && !!response.body.total_count) { - if (response.body.total_count > 0) { - for (let workflow of response.body.workflows) { - if (process.env.GITHUB_WORKFLOW === workflow.name) { - workflowFilePath = workflow.path; - break; + if (response) { + if (response.statusCode == StatusCodes.OK + && response.body + && response.body.total_count) { + if (response.body.total_count > 0) { + for (let workflow of response.body.workflows) { + if (process.env.GITHUB_WORKFLOW === workflow.name) { + workflowFilePath = workflow.path; + break; + } } } } + else if (response.statusCode != StatusCodes.OK) { + core.debug(`An error occured while getting list of workflows on the repo. Statuscode: ${response.statusCode}, StatusMessage: ${response.statusMessage}`); + } } - else if (response.statusCode != StatusCodes.OK) { - core.debug(`An error occured while getting list of workflows on the repo. Statuscode: ${response.statusCode}, StatusMessage: ${response.statusMessage}`); + else { + core.warning(`Failed to get response from workflow list API`); } } return Promise.resolve(workflowFilePath); @@ -105,13 +112,13 @@ export function annotateChildPods(kubectl: Kubectl, resourceType: string, resour owner = kubectl.getNewReplicaSet(resourceName); } - if (!!allPods && !!allPods.items && allPods.items.length > 0) { + if (allPods && allPods.items && allPods.items.length > 0) { allPods.items.forEach((pod) => { const owners = pod.metadata.ownerReferences; - if (!!owners) { + if (owners) { owners.forEach(ownerRef => { if (ownerRef.name === owner) { - commandExecutionResults.push(kubectl.annotate('pod', pod.metadata.name, [annotationKeyValStr], true)); + commandExecutionResults.push(kubectl.annotate('pod', pod.metadata.name, annotationKeyValStr)); } }); }