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 · 8e80d37 · 8e80d37
1 parent 3b389d1
commit 8e80d37
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
@@ -287,34 +287,75 @@ describe('getParams', () => {
   });
 });
 
-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);
   });
 });