Fix processing of stepTemplate volumeMounts

The merge process for display of step details was inadvertently modifying the stepTemplate on each pass so display of volumeMounts in the step details tab could end up showing volumeMounts that are not actually relevant for the selected step. Ensure we make a copy of the items array before processing to avoid modifying the original. Add a new test to ensure we don't regress in future.
tektoncd · May 31, 2023 · 36570a5 · 36570a5
1 parent 0c773d4
commit 36570a5
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 27 deletions.
diff --git a/packages/utils/src/utils/index.js b/packages/utils/src/utils/index.js
@@ -65,10 +65,11 @@ function mergeContainerField({
   step,
   stepTemplate
 }) {
-  const items = stepTemplate[field];
+  let items = stepTemplate[field];
   if (!items) {
     return;
   }
+  items = [...items]; // make a copy so we don't modify stepTemplate
 
   const stepItems = step[field];
   (stepItems || []).forEach(stepItem => {

diff --git a/packages/utils/src/utils/index.test.js b/packages/utils/src/utils/index.test.js
@@ -311,34 +311,75 @@ describe('getResources', () => {
   });
 });
 
-it('applyStepTemplate', () => {
-  const stepTemplate = {
-    args: ['some_args'],
-    command: ['sh'],
-    env: [
-      { name: 'env1', value: 'value1' },
-      { name: 'env2', value: 'value2' }
-    ],
-    image: 'alpine',
-    ports: [{ containerPort: 8888 }, { containerPort: 7777, name: 'my-port' }]
-  };
+describe('applyStepTemplate', () => {
+  it('merges fields from the step with the stepTemplate according to the Kubernetes strategic merge patch rules', () => {
+    const stepTemplate = {
+      args: ['some_args'],
+      command: ['sh'],
+      env: [
+        { name: 'env1', value: 'value1' },
+        { name: 'env2', value: 'value2' }
+      ],
+      image: 'alpine',
+      ports: [{ containerPort: 8888 }, { containerPort: 7777, name: 'my-port' }]
+    };
 
-  const step = {
-    args: ['step_args'],
-    env: [
-      { name: 'env1', value: 'step_value1' },
-      { name: 'env3', value: 'step_value3' }
-    ],
-    image: 'ubuntu',
-    ports: [{ containerPort: 7777, name: 'my-step-port' }]
-  };
+    const step = {
+      args: ['step_args'],
+      env: [
+        { name: 'env1', value: 'step_value1' },
+        { name: 'env3', value: 'step_value3' }
+      ],
+      image: 'ubuntu',
+      ports: [{ containerPort: 7777, name: 'my-step-port' }]
+    };
+
+    expect(applyStepTemplate({ step, stepTemplate })).toEqual({
+      args: step.args,
+      command: stepTemplate.command,
+      env: [step.env[0], stepTemplate.env[1], step.env[1]],
+      image: step.image,
+      ports: [stepTemplate.ports[0], step.ports[0]]
+    });
+  });
+
+  it('handles volumeMounts without modifying the stepTemplate', () => {
+    const templateVolumeMounts = [
+      {
+        mountPath: '/tmp/template-mount-path',
+        name: 'template-mount-name'
+      }
+    ];
+    const stepTemplate = {
+      volumeMounts: [...templateVolumeMounts]
+    };
 
-  expect(applyStepTemplate({ step, stepTemplate })).toEqual({
-    args: step.args,
-    command: stepTemplate.command,
-    env: [step.env[0], stepTemplate.env[1], step.env[1]],
-    image: step.image,
-    ports: [stepTemplate.ports[0], step.ports[0]]
+    const step1 = {
+      image: 'ubuntu',
+      script: 'fake_script',
+      volumeMounts: [
+        {
+          mountPath: '/tmp/step-mount-path',
+          name: 'step-mount-name'
+        }
+      ]
+    };
+
+    expect(applyStepTemplate({ step: step1, stepTemplate })).toEqual({
+      image: 'ubuntu',
+      script: 'fake_script',
+      volumeMounts: [
+        {
+          mountPath: '/tmp/template-mount-path',
+          name: 'template-mount-name'
+        },
+        {
+          mountPath: '/tmp/step-mount-path',
+          name: 'step-mount-name'
+        }
+      ]
+    });
+    expect(stepTemplate.volumeMounts).toEqual(templateVolumeMounts);
   });
 });