[KubernetesManifest] Replacement logic fix (#11883)

* [KubernetesManifest] Replacement logic fix * Resolving comments and some extra logic * Adding list support/fixing logic
microsoft · Dec 9, 2019 · 273f54b · 273f54b
1 parent d9b719a
commit 273f54b
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 89 deletions.
diff --git a/Tasks/KubernetesManifestV0/Tests/L0.ts b/Tasks/KubernetesManifestV0/Tests/L0.ts
@@ -5,7 +5,7 @@ import * as ttm from 'azure-pipelines-task-lib/mock-test';
 import * as tl from 'azure-pipelines-task-lib';
 import * as shared from './TestShared';
 import * as utils from '../src/utils/utilities';
-import { updateImagePullSecrets } from '../src/utils/KubernetesObjectUtility';
+import { updateImagePullSecrets, updateImageDetails } from '../src/utils/KubernetesObjectUtility';
 import * as yaml from 'js-yaml';
 import { IExecSyncResult } from 'azure-pipelines-task-lib/toolrunner';
 
@@ -283,32 +283,15 @@ describe('Kubernetes Manifests Suite', function () {
     it('Run should successfully add container image tags', (done: MochaDone) => {
         const testFile = path.join(__dirname, './manifests/', 'deployment-image-substitution.yaml');
         const deploymentFile = fs.readFileSync(testFile).toString();
-
-        const bigNameEditFirst = utils.substituteImageNameInSpecFile(deploymentFile, 'nginx-init', 'nginx-init:42.1');
-        const smallNameEditSecond = utils.substituteImageNameInSpecFile(bigNameEditFirst, 'nginx', 'nginx:42');
-        const smallSecondYaml = yaml.load(smallNameEditSecond);
-
-        assert(smallSecondYaml.spec.template.spec.containers[0].image === 'nginx:42', 'nginx image not tagged correctly');
-        assert(smallSecondYaml.spec.template.spec.initContainers[0].image === 'nginx-init:42.1', 'nginx-init image not tagged correctly');
-
-        const smallNameEditFirst = utils.substituteImageNameInSpecFile(deploymentFile, 'nginx', 'nginx:42');
-        const bigNameEditSecond = utils.substituteImageNameInSpecFile(smallNameEditFirst, 'nginx-init', 'nginx-init:42.1');
-        const bigSecondYaml = yaml.load(bigNameEditSecond);
-
-        assert(bigSecondYaml.spec.template.spec.containers[0].image === 'nginx:42', 'nginx image not tagged correctly');
-        assert(bigSecondYaml.spec.template.spec.initContainers[0].image === 'nginx-init:42.1', 'nginx-init image not tagged correctly');
-
-        const untaggedImages = utils.substituteImageNameInSpecFile(deploymentFile, 'mysql', 'mysql:8.0');
-        const untaggedImagesYaml = yaml.load(untaggedImages);
-        assert(untaggedImagesYaml.spec.template.spec.containers[2].image === 'mysql:8.0', 'untagged image not tagged correctly');
-
-        const untaggedImagesWithRegistry = utils.substituteImageNameInSpecFile(deploymentFile, 'myacr.azurecr.io/myimage', 'myacr.azurecr.io/myimage:1');
-        const untaggedImagesWithRegistryYaml = yaml.load(untaggedImagesWithRegistry);
-        assert(untaggedImagesWithRegistryYaml.spec.template.spec.containers[3].image === 'myacr.azurecr.io/myimage:1', 'untagged image with registry not tagged correctly');
-
-        const untaggedImagesWithComment = utils.substituteImageNameInSpecFile(deploymentFile, 'myimagewithcomment', 'myimagewithcomment:1');
-        const untaggedImagesWithCommentYaml = yaml.load(untaggedImagesWithComment);
-        assert(untaggedImagesWithCommentYaml.spec.template.spec.containers[4].image === 'myimagewithcomment:1', 'untagged image with comment not tagged correctly');
+        const deploymentObject = yaml.load(deploymentFile);
+
+        updateImageDetails(deploymentObject, ['nginx:42', 'mysql:8.0', 'imagewithhyphen:1', 'myacr.azurecr.io/myimage:1', 'myimagewithcomment:1', 'nginx-init:42.1']);
+        assert(deploymentObject.spec.template.spec.containers[0].image === 'nginx:42', 'nginx image not tagged correctly');
+        assert(deploymentObject.spec.template.spec.containers[2].image === 'mysql:8.0', 'untagged image not tagged correctly');
+        assert(deploymentObject.spec.template.spec.containers[3].image === 'myacr.azurecr.io/myimage:1', 'untagged image with registry not tagged correctly');
+        assert(deploymentObject.spec.template.spec.containers[4].image === 'myimagewithcomment:1', 'untagged image with comment not tagged correctly');
+        assert(deploymentObject.spec.template.spec.containers[5].image === 'imagewithhyphen:1', 'manifest regex should work correctly');
+        assert(deploymentObject.spec.template.spec.initContainers[0].image === 'nginx-init:42.1', 'nginx-init image not tagged correctly');
         done();
     });
 

diff --git a/Tasks/KubernetesManifestV0/Tests/manifests/deployment-image-substitution.yaml b/Tasks/KubernetesManifestV0/Tests/manifests/deployment-image-substitution.yaml
@@ -27,6 +27,8 @@ spec:
         image: myacr.azurecr.io/myimage 
       - name: untaggedwithcomment
         image: myimagewithcomment # this is a comment
+      - image: imagewithhyphen # this is a comment
+        name: imagewithhyphen
       imagePullSecrets:
       - name: key1
       initContainers:

diff --git a/Tasks/KubernetesManifestV0/src/utils/DeploymentHelper.ts b/Tasks/KubernetesManifestV0/src/utils/DeploymentHelper.ts
@@ -26,11 +26,8 @@ export async function deploy(kubectl: Kubectl, manifestFilePaths: string[], depl
     // get manifest files
     let inputManifestFiles: string[] = getManifestFiles(manifestFilePaths);
 
-    // artifact substitution
-    inputManifestFiles = updateContainerImagesInManifestFiles(inputManifestFiles, TaskInputParameters.containers);
-
-    // imagePullSecrets addition
-    inputManifestFiles = updateImagePullSecretsInManifestFiles(inputManifestFiles, TaskInputParameters.imagePullSecrets);
+    // imagePullSecrets addition & artifact substitution
+    inputManifestFiles = updateResourceObjects(inputManifestFiles, TaskInputParameters.imagePullSecrets, TaskInputParameters.containers);
 
     // deployment
     const deployedManifestFiles = deployManifests(inputManifestFiles, kubectl, isCanaryDeploymentStrategy(deploymentStrategy));
@@ -49,9 +46,8 @@ export async function deploy(kubectl: Kubectl, manifestFilePaths: string[], depl
     let allPods: any;
     try {
         allPods = JSON.parse((kubectl.getAllPods()).stdout);
-    }
-    catch (e) {
-        tl.debug("Unable to parse pods; Error: "+ e);
+    } catch (e) {
+        tl.debug("Unable to parse pods; Error: " + e);
     }
 
     annotateResources(deployedManifestFiles, kubectl, resourceTypes, allPods);
@@ -61,8 +57,7 @@ export async function deploy(kubectl: Kubectl, manifestFilePaths: string[], depl
         try {
             const clusterInfo = kubectl.getClusterInfo().stdout;
             captureAndPushDeploymentMetadata(inputManifestFiles, allPods, deploymentStrategy, clusterInfo, manifestFilePaths);
-        }
-        catch (e) {
+        } catch (e) {
             tl.warning("Capturing deployment metadata failed with error: " + e);
         }
     }
@@ -93,8 +88,7 @@ function deployManifests(files: string[], kubectl: Kubectl, isCanaryDeploymentSt
         if (canaryDeploymentHelper.isSMICanaryStrategy()) {
             const updatedManifests = appendStableVersionLabelToResource(files, kubectl);
             result = kubectl.apply(updatedManifests);
-        }
-        else {
+        } else {
             result = kubectl.apply(files);
         }
     }
@@ -141,56 +135,37 @@ function annotateResources(files: string[], kubectl: Kubectl, resourceTypes: Res
     utils.checkForErrors(annotateResults, true);
 }
 
-function updateContainerImagesInManifestFiles(filePaths: string[], containers: string[]): string[] {
-    if (!!containers && containers.length > 0) {
-        const newFilePaths = [];
-        const tempDirectory = fileHelper.getTempDirectory();
-        filePaths.forEach((filePath: string) => {
-            let contents = fs.readFileSync(filePath).toString();
-            containers.forEach((container: string) => {
-                let imageName = container.split(':')[0];
-                if (imageName.indexOf('@') > 0) {
-                    imageName = imageName.split('@')[0];
-                }
-                if (contents.indexOf(imageName) > 0) {
-                    contents = utils.substituteImageNameInSpecFile(contents, imageName, container);
-                }
-            });
-
-            const fileName = path.join(tempDirectory, path.basename(filePath));
-            fs.writeFileSync(
-                path.join(fileName),
-                contents
-            );
-            newFilePaths.push(fileName);
-        });
-
-        return newFilePaths;
+function updateResourceObjects(filePaths: string[], imagePullSecrets: string[], containers: string[]): string[] {
+    const newObjectsList = [];
+    const updateResourceObject = (inputObject) => {
+        if (!!imagePullSecrets && imagePullSecrets.length > 0) {
+            KubernetesObjectUtility.updateImagePullSecrets(inputObject, imagePullSecrets, false);
+        }
+        if (!!containers && containers.length > 0) {
+            KubernetesObjectUtility.updateImageDetails(inputObject, containers);
+        }
     }
-
-    return filePaths;
-}
-
-function updateImagePullSecretsInManifestFiles(filePaths: string[], imagePullSecrets: string[]): string[] {
-    if (!!imagePullSecrets && imagePullSecrets.length > 0) {
-        const newObjectsList = [];
-        filePaths.forEach((filePath: string) => {
-            const fileContents = fs.readFileSync(filePath).toString();
-            yaml.safeLoadAll(fileContents, function (inputObject: any) {
-                if (!!inputObject && !!inputObject.kind) {
-                    const kind = inputObject.kind;
-                    if (KubernetesObjectUtility.isWorkloadEntity(kind)) {
-                        KubernetesObjectUtility.updateImagePullSecrets(inputObject, imagePullSecrets, false);
+    filePaths.forEach((filePath: string) => {
+        const fileContents = fs.readFileSync(filePath).toString();
+        yaml.safeLoadAll(fileContents, function (inputObject: any) {
+            if (inputObject && inputObject.kind) {
+                const kind = inputObject.kind;
+                if (KubernetesObjectUtility.isWorkloadEntity(kind)) {
+                    updateResourceObject(inputObject);
+                }
+                else if (isEqual(kind, 'list', StringComparer.OrdinalIgnoreCase)) {
+                    let items = inputObject.items;
+                    if (items.length > 0) {
+                        items.forEach((item) => updateResourceObject(item));
                     }
-                    newObjectsList.push(inputObject);
                 }
-            });
+                newObjectsList.push(inputObject);
+            }
         });
-        tl.debug('New K8s objects after addin imagePullSecrets are :' + JSON.stringify(newObjectsList));
-        const newFilePaths = fileHelper.writeObjectsToFile(newObjectsList);
-        return newFilePaths;
-    }
-    return filePaths;
+    });
+    tl.debug('New K8s objects after addin imagePullSecrets are :' + JSON.stringify(newObjectsList));
+    const newFilePaths = fileHelper.writeObjectsToFile(newObjectsList);
+    return newFilePaths;
 }
 
 function captureAndPushDeploymentMetadata(filePaths: string[], allPods: any, deploymentStrategy: string, clusterInfo: any, manifestFilePaths: string[]) {

diff --git a/Tasks/KubernetesManifestV0/src/utils/KubernetesObjectUtility.ts b/Tasks/KubernetesManifestV0/src/utils/KubernetesObjectUtility.ts
@@ -287,6 +287,57 @@ function setImagePullSecrets(inputObject: any, newImagePullSecrets: any) {
     return;
 }
 
+export function updateImageDetails(inputObject: any, containers: string[]) {
+    if (!inputObject || !inputObject.spec || !containers) {
+        return;
+    }
+
+    if (inputObject.spec.template && !!inputObject.spec.template.spec) {
+        if (inputObject.spec.template.spec.containers) {
+            updateContainers(inputObject.spec.template.spec.containers, containers);
+        }
+        if (inputObject.spec.template.spec.initContainers) {
+            updateContainers(inputObject.spec.template.spec.initContainers, containers);
+        }
+        return;
+    }
+
+    if (inputObject.spec.containers) {
+        updateContainers(inputObject.spec.containers, containers);
+    }
+
+    if (inputObject.spec.initContainers) {
+        updateContainers(inputObject.spec.initContainers, containers);
+    }
+}
+
+function extractImageName(imageName) {
+    let img = '';
+    if (imageName.indexOf('/') > 0) {
+        const imgParts = imageName.split('/');
+        const registry = imgParts[0];
+        const imgName = imgParts[1].split(':')[0];
+        img = `${registry}/${imgName}`;
+    } else {
+        img = imageName.split(':')[0];
+    }
+    return img;
+}
+
+function updateContainers(containers: any[], images: string[]) {
+    if (!containers || containers.length === 0) {
+        return containers;
+    }
+    containers.forEach((container) => {
+        const imageName: string = extractImageName(container.image.trim());
+        images.forEach(image => {
+            if (extractImageName(image) === imageName) {
+                container.image = image;
+            }
+        });
+    });
+}
+
 function setSpecLabels(inputObject: any, newLabels: any) {
     let specLabels = getSpecLabels(inputObject);
     if (!!newLabels) {

diff --git a/Tasks/KubernetesManifestV0/task.json b/Tasks/KubernetesManifestV0/task.json
@@ -13,8 +13,8 @@
     "author": "Microsoft Corporation",
     "version": {
         "Major": 0,
-        "Minor": 162,
-        "Patch": 1
+        "Minor": 163,
+        "Patch": 0
     },
     "demands": [],
     "groups": [],

diff --git a/Tasks/KubernetesManifestV0/task.loc.json b/Tasks/KubernetesManifestV0/task.loc.json
@@ -13,8 +13,8 @@
   "author": "Microsoft Corporation",
   "version": {
     "Major": 0,
-    "Minor": 162,
-    "Patch": 1
+    "Minor": 163,
+    "Patch": 0
   },
   "demands": [],
   "groups": [],