Fix: CA bundle volume conflict during operator upgrade

derekhiggins · derekhiggins · commit 2776f8799ed7 · 2026-02-05T15:48:41.000Z
When upgrading a version with legacy CA bundle volumes, deployments fail with volume
validation errors if the odh-trusted-ca-bundle ConfigMap exists. The old
operator used an emptyDir volume ("ca-bundle") with an init container to
copy certificates, while the new operator uses a managed ConfigMap volume.

Server-Side Apply creates field conflicts when trying to change
the volume type from emptyDir to ConfigMap, resulting in a volume with
both properties defined, which violates Kubernetes validation.

This fix detects deployments with legacy CA bundle volumes and uses a
full Update (replacement) instead of SSA Patch. This ensures clean
migration by removing all legacy artifacts (emptyDir volume, source
ConfigMap volume, and init container) in a single operation.

Assisted-by: Claude Sonnet 4.5 &lt;noreply@anthropic.com&gt;
Signed-off-by: Derek Higgins &lt;derekh@redhat.com&gt;
diff --git a/pkg/deploy/kustomizer.go b/pkg/deploy/kustomizer.go
@@ -177,15 +177,25 @@ func patchResource(ctx context.Context, cli client.Client, desired, existing *un
 		return nil
 	}
 
-	if existing.GetKind() == "PersistentVolumeClaim" {
+	switch existing.GetKind() {
+	case "PersistentVolumeClaim":
 		logger.V(1).Info("Skipping PVC patch - PVCs are immutable after creation",
 			"name", existing.GetName(),
 			"namespace", existing.GetNamespace())
 		return nil
-	} else if existing.GetKind() == "Service" {
+	case "Service":
 		if err := compare.CheckAndLogServiceChanges(ctx, cli, desired); err != nil {
 			return fmt.Errorf("failed to validate resource mutations while patching: %w", err)
 		}
+	case deploymentKind:
+		// Check for legacy CA bundle volumes and use full replacement to avoid SSA conflicts
+		if hasLegacyCABundleVolumes(ctx, existing) {
+			logger.Info("Detected legacy CA bundle volumes, using full replacement instead of SSA",
+				"deployment", existing.GetName(),
+				"namespace", existing.GetNamespace())
+			desired.SetResourceVersion(existing.GetResourceVersion())
+			return cli.Update(ctx, desired)
+		}
 	}
 
 	data, err := json.Marshal(desired)
@@ -661,6 +671,46 @@ func FilterExcludeKinds(resMap *resmap.ResMap, kindsToExclude []string) (*resmap
 	return &filteredResMap, nil
 }
 
+// hasLegacyCABundleVolumes detects if a deployment has legacy CA bundle volumes
+// from the old operator that used emptyDir + ConfigMap pattern.
+func hasLegacyCABundleVolumes(ctx context.Context, deployment *unstructured.Unstructured) bool {
+	logger := log.FromContext(ctx)
+
+	volumes, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "volumes")
+	if err != nil || !found {
+		return false
+	}
+
+	for _, vol := range volumes {
+		volumeMap, ok := vol.(map[string]interface{})
+		if !ok {
+			continue
+		}
+
+		volumeName, _, _ := unstructured.NestedString(volumeMap, "name")
+
+		// Legacy pattern: volume named "ca-bundle" with emptyDir
+		if volumeName == "ca-bundle" {
+			if _, hasEmptyDir := volumeMap["emptyDir"]; hasEmptyDir {
+				logger.V(1).Info("Found legacy ca-bundle emptyDir volume",
+					"deployment", deployment.GetName(),
+					"namespace", deployment.GetNamespace())
+				return true
+			}
+		}
+
+		// Legacy pattern: volume named "ca-bundle-source" (ConfigMap source)
+		if volumeName == "ca-bundle-source" {
+			logger.V(1).Info("Found legacy ca-bundle-source volume",
+				"deployment", deployment.GetName(),
+				"namespace", deployment.GetNamespace())
+			return true
+		}
+	}
+
+	return false
+}
+
 // CheckClusterRoleExists checks if a RoleBinding should be skipped due to missing SCC ClusterRole.
 func CheckClusterRoleExists(ctx context.Context, cli client.Client, crb *unstructured.Unstructured) (bool, error) {
 	roleRef, found, _ := unstructured.NestedMap(crb.Object, "roleRef")
diff --git a/pkg/deploy/kustomizer_test.go b/pkg/deploy/kustomizer_test.go
@@ -18,9 +18,11 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/kustomize/api/resmap"
+	kresource "sigs.k8s.io/kustomize/api/resource"
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
@@ -628,3 +630,283 @@ func TestRemoveDeploymentReplicas(t *testing.T) {
 
 	require.False(t, hasReplicas, "replicas should be removed from all deployments when autoscaling is enabled")
 }
+
+// TestHasLegacyCABundleVolumes tests the detection of legacy CA bundle volumes.
+func TestHasLegacyCABundleVolumes(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("detects legacy emptyDir ca-bundle volume", func(t *testing.T) {
+		deployment := newTestResource(t, "apps/v1", "Deployment", "test-deploy", "test-ns", map[string]any{
+			"template": map[string]any{
+				"spec": map[string]any{
+					"volumes": []any{
+						map[string]any{
+							"name":     "ca-bundle",
+							"emptyDir": map[string]any{},
+						},
+					},
+				},
+			},
+		})
+
+		u, err := resourceToUnstructured(t, deployment)
+		require.NoError(t, err)
+
+		result := hasLegacyCABundleVolumes(ctx, u)
+		require.True(t, result, "should detect legacy ca-bundle emptyDir volume")
+	})
+
+	t.Run("detects legacy ca-bundle-source volume", func(t *testing.T) {
+		deployment := newTestResource(t, "apps/v1", "Deployment", "test-deploy", "test-ns", map[string]any{
+			"template": map[string]any{
+				"spec": map[string]any{
+					"volumes": []any{
+						map[string]any{
+							"name": "ca-bundle-source",
+							"configMap": map[string]any{
+								"name": "odh-trusted-ca-bundle",
+							},
+						},
+					},
+				},
+			},
+		})
+
+		u, err := resourceToUnstructured(t, deployment)
+		require.NoError(t, err)
+
+		result := hasLegacyCABundleVolumes(ctx, u)
+		require.True(t, result, "should detect legacy ca-bundle-source volume")
+	})
+
+	t.Run("does not detect new-style ca-bundle configMap", func(t *testing.T) {
+		deployment := newTestResource(t, "apps/v1", "Deployment", "test-deploy", "test-ns", map[string]any{
+			"template": map[string]any{
+				"spec": map[string]any{
+					"volumes": []any{
+						map[string]any{
+							"name": "ca-bundle",
+							"configMap": map[string]any{
+								"name": "managed-ca-bundle",
+							},
+						},
+					},
+				},
+			},
+		})
+
+		u, err := resourceToUnstructured(t, deployment)
+		require.NoError(t, err)
+
+		result := hasLegacyCABundleVolumes(ctx, u)
+		require.False(t, result, "should not detect new-style ca-bundle ConfigMap as legacy")
+	})
+
+	t.Run("does not detect unrelated volumes", func(t *testing.T) {
+		deployment := newTestResource(t, "apps/v1", "Deployment", "test-deploy", "test-ns", map[string]any{
+			"template": map[string]any{
+				"spec": map[string]any{
+					"volumes": []any{
+						map[string]any{
+							"name":     "data",
+							"emptyDir": map[string]any{},
+						},
+						map[string]any{
+							"name": "config",
+							"configMap": map[string]any{
+								"name": "app-config",
+							},
+						},
+					},
+				},
+			},
+		})
+
+		u, err := resourceToUnstructured(t, deployment)
+		require.NoError(t, err)
+
+		result := hasLegacyCABundleVolumes(ctx, u)
+		require.False(t, result, "should not detect unrelated volumes as legacy")
+	})
+
+	t.Run("returns false when no volumes present", func(t *testing.T) {
+		deployment := newTestResource(t, "apps/v1", "Deployment", "test-deploy", "test-ns", map[string]any{})
+
+		u, err := resourceToUnstructured(t, deployment)
+		require.NoError(t, err)
+
+		result := hasLegacyCABundleVolumes(ctx, u)
+		require.False(t, result, "should return false when no volumes present")
+	})
+}
+
+// TestLegacyCABundleUpgrade tests that deployments with legacy CA bundle volumes
+// are replaced instead of patched to avoid SSA conflicts.
+func TestLegacyCABundleUpgrade(t *testing.T) {
+	ctx, testNs, owner := setupApplyResourcesTest(t, "legacy-ca-upgrade")
+
+	// Create an existing deployment with legacy CA bundle volumes (old operator pattern)
+	existingDeployment := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-deployment",
+			Namespace: testNs,
+			Labels:    map[string]string{"version": "old"},
+			OwnerReferences: []metav1.OwnerReference{
+				*metav1.NewControllerRef(owner, owner.GroupVersionKind()),
+			},
+		},
+		Spec: appsv1.DeploymentSpec{
+			Replicas: ptr(int32(1)),
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"app": "test"},
+			},
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{"app": "test"},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name:  "main",
+							Image: "test:v1",
+						},
+					},
+					// Legacy CA bundle volumes (old operator pattern)
+					Volumes: []corev1.Volume{
+						{
+							Name: "data",
+							VolumeSource: corev1.VolumeSource{
+								EmptyDir: &corev1.EmptyDirVolumeSource{},
+							},
+						},
+						{
+							Name: "ca-bundle",
+							VolumeSource: corev1.VolumeSource{
+								EmptyDir: &corev1.EmptyDirVolumeSource{},
+							},
+						},
+						{
+							Name: "ca-bundle-source",
+							VolumeSource: corev1.VolumeSource{
+								ConfigMap: &corev1.ConfigMapVolumeSource{
+									LocalObjectReference: corev1.LocalObjectReference{
+										Name: "odh-trusted-ca-bundle",
+									},
+								},
+							},
+						},
+					},
+					InitContainers: []corev1.Container{
+						{
+							Name:  "ca-bundle-init",
+							Image: "busybox",
+						},
+					},
+				},
+			},
+		},
+	}
+	require.NoError(t, k8sClient.Create(ctx, existingDeployment))
+
+	// Create desired deployment with new CA bundle pattern (new operator pattern)
+	// Must use same selector as existing deployment (selector is immutable)
+	desiredDeployment := newTestResource(t, "apps/v1", "Deployment", "test-deployment", testNs, map[string]any{
+		"replicas": int32(1),
+		"selector": map[string]any{
+			"matchLabels": map[string]any{
+				"app": "test",
+			},
+		},
+		"template": map[string]any{
+			"metadata": map[string]any{
+				"labels": map[string]any{
+					"app": "test",
+				},
+			},
+			"spec": map[string]any{
+				"containers": []any{
+					map[string]any{
+						"name":  "main",
+						"image": "test:v2",
+					},
+				},
+				"volumes": []any{
+					map[string]any{
+						"name":     "data",
+						"emptyDir": map[string]any{},
+					},
+					// New CA bundle pattern - single ConfigMap volume
+					map[string]any{
+						"name": "ca-bundle",
+						"configMap": map[string]any{
+							"name": "managed-ca-bundle",
+						},
+					},
+				},
+			},
+		},
+	})
+	desiredDeployment.SetLabels(map[string]string{"version": "new"})
+
+	resMap := resmap.New()
+	require.NoError(t, resMap.Append(desiredDeployment))
+
+	// Apply the resources (should trigger replacement instead of patch)
+	require.NoError(t, ApplyResources(ctx, k8sClient, scheme.Scheme, owner, &resMap))
+
+	// Verify the deployment was updated
+	updatedDeployment := &appsv1.Deployment{}
+	deploymentKey := types.NamespacedName{Name: "test-deployment", Namespace: testNs}
+	require.NoError(t, k8sClient.Get(ctx, deploymentKey, updatedDeployment))
+
+	// Verify labels were updated (proves Update was used, not just patch)
+	require.Equal(t, "new", updatedDeployment.Labels["version"], "deployment labels should be updated")
+
+	// Verify container image was updated
+	require.Equal(t, "test:v2", updatedDeployment.Spec.Template.Spec.Containers[0].Image, "container image should be updated")
+
+	// Verify legacy volumes are removed
+	volumeNames := make([]string, len(updatedDeployment.Spec.Template.Spec.Volumes))
+	for i, vol := range updatedDeployment.Spec.Template.Spec.Volumes {
+		volumeNames[i] = vol.Name
+	}
+	require.NotContains(t, volumeNames, "ca-bundle-source", "legacy ca-bundle-source volume should be removed")
+
+	// Verify new ca-bundle is a ConfigMap (not emptyDir)
+	var caBundleVolume *corev1.Volume
+	for i := range updatedDeployment.Spec.Template.Spec.Volumes {
+		if updatedDeployment.Spec.Template.Spec.Volumes[i].Name == "ca-bundle" {
+			caBundleVolume = &updatedDeployment.Spec.Template.Spec.Volumes[i]
+			break
+		}
+	}
+	require.NotNil(t, caBundleVolume, "ca-bundle volume should exist")
+	require.NotNil(t, caBundleVolume.ConfigMap, "ca-bundle should be a ConfigMap volume")
+	require.Nil(t, caBundleVolume.EmptyDir, "ca-bundle should not be an emptyDir volume")
+	require.Equal(t, "managed-ca-bundle", caBundleVolume.ConfigMap.Name, "ca-bundle ConfigMap name should be correct")
+
+	// Verify init containers are removed
+	require.Empty(t, updatedDeployment.Spec.Template.Spec.InitContainers, "legacy init containers should be removed")
+}
+
+// resourceToUnstructured converts a kustomize resource to an unstructured object.
+func resourceToUnstructured(t *testing.T, res *kresource.Resource) (*unstructured.Unstructured, error) {
+	t.Helper()
+
+	yamlBytes, err := res.AsYAML()
+	if err != nil {
+		return nil, err
+	}
+
+	u := &unstructured.Unstructured{}
+	if err := yaml.Unmarshal(yamlBytes, u); err != nil {
+		return nil, err
+	}
+
+	return u, nil
+}
+
+// ptr is a helper function to get a pointer to a value.
+func ptr[T any](v T) *T {
+	return &v
+}
