feat: make PodTemplateSpec more robust and less error prone

config/samples/dataplane-konnect.yaml

@@ -66,16 +66,10 @@ spec:
             initialDelaySeconds: 1
             periodSeconds: 1
           volumeMounts:
-          # We need to specify the cluster-certificate volume-mount because otherwise
-          # strategic merge patch would merge its entry with provided
-          # konnect-client-tls volume mount.
-          - name: cluster-certificate
-            mountPath: /var/cluster-certificate
           - name: konnect-client-tls
             mountPath: /etc/secrets/kong-cluster-cert/
             readOnly: true
         volumes:
-        - name: cluster-certificate
         - name: konnect-client-tls
           secret:
             secretName: konnect-client-tls

config/samples/dataplane-sidecar.yaml

@@ -26,7 +26,6 @@ spec:
           dataplane-pod-annotation: example
       spec:
         volumes:
-        - name: cluster-certificate
         - name: sidecar-vector-config-volume
           configMap:
             name: sidecar-vector-config
@@ -41,9 +40,6 @@ spec:
             mountPath: /etc/vector
           - name: proxy-logs
             mountPath: /etc/kong/log/
-          readinessProbe:
-            initialDelaySeconds: 1
-            periodSeconds: 1
         - name: proxy
           image: kong:3.6
           volumeMounts:

controller/controlplane/controller_utils_test.go

@@ -917,6 +917,87 @@ func TestControlPlaneSpecDeepEqual(t *testing.T) {
 			},
 			equal: true,
 		},
+		{
+			name: "not matching Extensions, different length",
+			spec1: &operatorv1beta1.ControlPlaneOptions{
+				Extensions: []operatorv1alpha1.ExtensionRef{
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test",
+						},
+					},
+				},
+			},
+			spec2: &operatorv1beta1.ControlPlaneOptions{
+				Extensions: []operatorv1alpha1.ExtensionRef{
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test",
+						},
+					},
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test2",
+						},
+					},
+				},
+			},
+			equal: false,
+		},
+		{
+			name: "matching Extensions, different order",
+			spec1: &operatorv1beta1.ControlPlaneOptions{
+				Extensions: []operatorv1alpha1.ExtensionRef{
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test",
+						},
+					},
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test2",
+						},
+					},
+				},
+			},
+			spec2: &operatorv1beta1.ControlPlaneOptions{
+				Extensions: []operatorv1alpha1.ExtensionRef{
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test2",
+						},
+					},
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test",
+						},
+					},
+				},
+			},
+			equal: false,
+		},
+		{
+			name: "not matching Extensions, different names",
+			spec1: &operatorv1beta1.ControlPlaneOptions{
+				Extensions: []operatorv1alpha1.ExtensionRef{
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test",
+						},
+					},
+				},
+			},
+			spec2: &operatorv1beta1.ControlPlaneOptions{
+				Extensions: []operatorv1alpha1.ExtensionRef{
+					{
+						NamespacedRef: operatorv1alpha1.NamespacedRef{
+							Name: "test2",
+						},
+					},
+				},
+			},
+			equal: false,
+		},
 	}
 
 	for _, tc := range testCases {

controller/dataplane/controller_reconciler_utils_test.go

@@ -104,11 +104,6 @@ func TestDeploymentBuilder(t *testing.T) {
 								PodTemplateSpec: &corev1.PodTemplateSpec{
 									Spec: corev1.PodSpec{
 										Volumes: []corev1.Volume{
-											{
-												// NOTE: we need to provide the existing entry in the slice
-												// to prevent merging the provided new entry with existing entries.
-												Name: consts.ClusterCertificateVolume,
-											},
 											{
 												Name: "test-volume",
 												VolumeSource: corev1.VolumeSource{
@@ -122,10 +117,6 @@ func TestDeploymentBuilder(t *testing.T) {
 											{
 												Name: consts.DataPlaneProxyContainerName,
 												VolumeMounts: []corev1.VolumeMount{
-													{
-														Name:      consts.ClusterCertificateVolume,
-														MountPath: consts.ClusterCertificateVolumeMountPath,
-													},
 													{
 														Name:      "test-volume",
 														MountPath: "/var/test/",
@@ -187,16 +178,20 @@ func TestDeploymentBuilder(t *testing.T) {
 				k8sresources.SetDefaultsVolume(&testVolume)
 				testVolume.Name = "test-volume"
 				testVolume.VolumeSource.Secret.SecretName = "test-secret"
+				require.Equal(t,
+					[]corev1.Volume{testVolume, certificateVolume},
+					deployment.Spec.Template.Spec.Volumes,
+				)
 
 				require.Equal(t, []corev1.VolumeMount{
 					{
-						Name:      consts.ClusterCertificateVolume,
-						MountPath: consts.ClusterCertificateVolumeMountPath,
+						Name:      "test-volume",
+						MountPath: "/var/test/",
 						ReadOnly:  true,
 					},
 					{
-						Name:      "test-volume",
-						MountPath: "/var/test/",
+						Name:      consts.ClusterCertificateVolume,
+						MountPath: consts.ClusterCertificateVolumeMountPath,
 						ReadOnly:  true,
 					},
 				},

pkg/utils/kubernetes/resources/deployments.go

@@ -101,7 +101,6 @@ func GenerateNewDeploymentForControlPlane(params GenerateNewDeploymentForControl
 			},
 		},
 	}
-	SetDefaultsPodTemplateSpec(&deployment.Spec.Template)
 	LabelObjectAsControlPlaneManaged(deployment)
 
 	if params.ControlPlane.Spec.Deployment.PodTemplateSpec != nil {

pkg/utils/kubernetes/resources/strategicmerge.go

@@ -6,6 +6,7 @@ import (
 	"github.com/goccy/go-json"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/util/mergepatch"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	pkgapiscorev1 "k8s.io/kubernetes/pkg/apis/core/v1"
 
@@ -19,24 +20,51 @@ func StrategicMergePatchPodTemplateSpec(base, patch *corev1.PodTemplateSpec) (*c
 		return base, nil
 	}
 
+	// NOTE: this is needed because without it the patch will wipe out the containers from the base.
+	if len(patch.Spec.Containers) == 0 {
+		patch.Spec.Containers = base.Spec.Containers
+	}
+
+	SetDefaultsPodTemplateSpec(base)
 	baseBytes, err := json.Marshal(base)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal JSON for base %s: %w", base.Name, err)
 	}
+	baseMap := map[string]interface{}{}
+	if err := json.Unmarshal(baseBytes, &baseMap); err != nil {
+		return nil, mergepatch.ErrBadJSONDoc
+	}
 
 	SetDefaultsPodTemplateSpec(patch)
 	patchBytes, err := json.Marshal(patch)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal JSON for patch %s: %w", patch.Name, err)
 	}
+	patchMap := map[string]interface{}{}
+	if err := json.Unmarshal(patchBytes, &patchMap); err != nil {
+		return nil, mergepatch.ErrBadJSONDoc
+	}
+	schema, err := strategicpatch.NewPatchMetaFromStruct(&corev1.PodTemplateSpec{})
+	if err != nil {
+		return nil, err
+	}
 
-	// Calculate the patch result.
-	jsonResultBytes, err := strategicpatch.StrategicMergePatch(baseBytes, patchBytes, &corev1.PodTemplateSpec{})
+	out, err := CreateTwoWayMergeMapPatchUsingLookupPatchMeta(baseMap, patchMap, schema)
+	if err != nil {
+		return nil, err
+	}
+
+	outMap, err := strategicpatch.StrategicMergeMapPatch(baseMap, out, &corev1.PodTemplateSpec{})
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate merge patch for %s: %w", base.Name, err)
 	}
 
-	patchResult := base.DeepCopy()
+	jsonResultBytes, err := json.Marshal(outMap)
+	if err != nil {
+		return nil, err
+	}
+
+	patchResult := &corev1.PodTemplateSpec{}
 	if err := json.Unmarshal(jsonResultBytes, patchResult); err != nil {
 		return nil, fmt.Errorf("failed to unmarshal merged %s: %w", base.Name, err)
 	}