From fb292df20b8797619698ae578f31e39eafa6694f Mon Sep 17 00:00:00 2001
From: Chitrang Patel <chitrang@google.com>
Date: Fri, 19 May 2023 14:41:02 -0400
Subject: [PATCH] Issue-6823 Conversion Webhook panic fix

Prior to this, the conversion of task spec from v1beta1
was panicing when the step template was nil since it could not
access the underlying deprecated fields. This PR fixes that bug.
Related issue: https://github.com/tektoncd/pipeline/issues/6823
---
 pkg/apis/pipeline/v1beta1/task_conversion.go  | 31 +++++++++----------
 .../pipeline/v1beta1/task_conversion_test.go  | 27 ++++++++++++++++
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/pkg/apis/pipeline/v1beta1/task_conversion.go b/pkg/apis/pipeline/v1beta1/task_conversion.go
index 162358f2acd..694e72dda26 100644
--- a/pkg/apis/pipeline/v1beta1/task_conversion.go
+++ b/pkg/apis/pipeline/v1beta1/task_conversion.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"reflect"
 
 	"github.com/tektoncd/pipeline/pkg/apis/version"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -293,21 +292,21 @@ func retrieveTaskDeprecation(spec *TaskSpec) *taskDeprecation {
 			DeprecatedTTY:                      s.DeprecatedTTY,
 		})
 	}
-	dst := &StepTemplate{
-		DeprecatedName:                     spec.StepTemplate.DeprecatedName,
-		DeprecatedPorts:                    spec.StepTemplate.DeprecatedPorts,
-		DeprecatedLivenessProbe:            spec.StepTemplate.DeprecatedLivenessProbe,
-		DeprecatedReadinessProbe:           spec.StepTemplate.DeprecatedReadinessProbe,
-		DeprecatedStartupProbe:             spec.StepTemplate.DeprecatedStartupProbe,
-		DeprecatedLifecycle:                spec.StepTemplate.DeprecatedLifecycle,
-		DeprecatedTerminationMessagePath:   spec.StepTemplate.DeprecatedTerminationMessagePath,
-		DeprecatedTerminationMessagePolicy: spec.StepTemplate.DeprecatedTerminationMessagePolicy,
-		DeprecatedStdin:                    spec.StepTemplate.DeprecatedStdin,
-		DeprecatedStdinOnce:                spec.StepTemplate.DeprecatedStdinOnce,
-		DeprecatedTTY:                      spec.StepTemplate.DeprecatedTTY,
-	}
-	if reflect.DeepEqual(dst, &StepTemplate{}) {
-		dst = nil
+	var dst *StepTemplate
+	if spec.StepTemplate != nil {
+		dst = &StepTemplate{
+			DeprecatedName:                     spec.StepTemplate.DeprecatedName,
+			DeprecatedPorts:                    spec.StepTemplate.DeprecatedPorts,
+			DeprecatedLivenessProbe:            spec.StepTemplate.DeprecatedLivenessProbe,
+			DeprecatedReadinessProbe:           spec.StepTemplate.DeprecatedReadinessProbe,
+			DeprecatedStartupProbe:             spec.StepTemplate.DeprecatedStartupProbe,
+			DeprecatedLifecycle:                spec.StepTemplate.DeprecatedLifecycle,
+			DeprecatedTerminationMessagePath:   spec.StepTemplate.DeprecatedTerminationMessagePath,
+			DeprecatedTerminationMessagePolicy: spec.StepTemplate.DeprecatedTerminationMessagePolicy,
+			DeprecatedStdin:                    spec.StepTemplate.DeprecatedStdin,
+			DeprecatedStdinOnce:                spec.StepTemplate.DeprecatedStdinOnce,
+			DeprecatedTTY:                      spec.StepTemplate.DeprecatedTTY,
+		}
 	}
 	return &taskDeprecation{
 		DeprecatedSteps:        ds,
diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go
index 65345578396..6195d1ab0a6 100644
--- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go
+++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go
@@ -235,6 +235,24 @@ spec:
   - name: step-1
   stepTemplate:
     image: foo
+`
+	taskWithoutStepTemplateYAML := `
+metadata:
+  name: foo
+  namespace: bar
+  generation: 1
+spec:
+  steps:
+  - image: alpine
+    name: echo
+    readinessProbe:
+      exec:
+        command:
+        - cat
+        - /tmp/healthy
+    resources: {}
+    script: |
+      echo "Good Morning!"
 `
 	simpleTaskV1beta1 := parse.MustParseV1beta1Task(t, simpleTaskYAML)
 	simpleTaskV1 := parse.MustParseV1Task(t, simpleTaskYAML)
@@ -252,6 +270,11 @@ spec:
 			`[{"name":"","ports":[{"name":"port","containerPort":0}],"resources":{},"livenessProbe":{"initialDelaySeconds":1},"readinessProbe":{"initialDelaySeconds":2},"startupProbe":{"initialDelaySeconds":3},"lifecycle":{"postStart":{"exec":{"command":["lifecycle command"]}}},"terminationMessagePath":"path","terminationMessagePolicy":"policy","stdin":true,"stdinOnce":true,"tty":true}],` +
 			`"deprecatedStepTemplate":{"name":"","ports":[{"name":"port","containerPort":0}],"resources":{},"livenessProbe":{"initialDelaySeconds":1},"readinessProbe":{"initialDelaySeconds":2},"startupProbe":{"initialDelaySeconds":3},"lifecycle":{"postStart":{"exec":{"command":["lifecycle command"]}}},"terminationMessagePath":"path","terminationMessagePolicy":"policy","stdin":true,"stdinOnce":true,"tty":true}}}`,
 	}
+	taskWithoutStepTemplateYAMLV1beta1 := parse.MustParseV1beta1Task(t, taskWithoutStepTemplateYAML)
+	taskWithoutStepTemplateYAMLV1 := parse.MustParseV1Task(t, taskWithoutStepTemplateYAML)
+	taskWithoutStepTemplateYAMLV1.ObjectMeta.Annotations = map[string]string{
+		v1beta1.TaskDeprecationsAnnotationKey: `{"foo":{"deprecatedSteps":[{"name":"","resources":{},"readinessProbe":{"exec":{"command":["cat","/tmp/healthy"]}}}]}}`,
+	}
 
 	tests := []struct {
 		name        string
@@ -273,6 +296,10 @@ spec:
 		name:        "task conversion deprecated fields",
 		v1beta1Task: taskWithDeprecatedFieldsV1beta1,
 		v1Task:      taskWithDeprecatedFieldsV1,
+	}, {
+		name:        "task conversion deprecated step template fields panic check",
+		v1beta1Task: taskWithoutStepTemplateYAMLV1beta1,
+		v1Task:      taskWithoutStepTemplateYAMLV1,
 	},
 	}
 	var ignoreTypeMeta = cmpopts.IgnoreFields(metav1.TypeMeta{}, "Kind", "APIVersion")