[module-sdk] fix: preserve crd vendor extensions

pkg/crd-installer/installer.go

@@ -15,6 +15,8 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apimachineryv1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	apimachineryYaml "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/client-go/dynamic"
@@ -190,32 +192,29 @@ func (cp *CRDsInstaller) processCRD(ctx context.Context, crdFilePath string) err
 }
 
 func (cp *CRDsInstaller) putCRDToCluster(ctx context.Context, crdReader io.Reader, bufferSize int) error {
-	var crd *apiextensionsv1.CustomResourceDefinition
-
-	err := apimachineryYaml.NewYAMLOrJSONDecoder(crdReader, bufferSize).Decode(&crd)
+	// Decode into unstructured so vendor schema extensions (x-kubernetes-*, x-ui-*, ...)
+	// survive verbatim; the typed struct below is used only to read metadata.
+	desired := &unstructured.Unstructured{}
+	err := apimachineryYaml.NewYAMLOrJSONDecoder(crdReader, bufferSize).Decode(&desired)
 	if err != nil {
 		return err
 	}
 
-	// it could be a comment or some other peace of yaml file, skip it
-	if crd == nil {
+	// a comment or other non-object yaml document decodes to a nil pointer / empty object, skip it
+	if desired == nil || len(desired.Object) == 0 {
 		return nil
 	}
 
-	if crd.APIVersion != apiextensionsv1.SchemeGroupVersion.String() && crd.Kind != "CustomResourceDefinition" {
-		return fmt.Errorf("invalid CRD document apiversion/kind: '%s/%s'", crd.APIVersion, crd.Kind)
-	}
-
-	if len(crd.ObjectMeta.Labels) == 0 {
-		crd.ObjectMeta.Labels = make(map[string]string, 1)
+	crd := &apiextensionsv1.CustomResourceDefinition{}
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(desired.Object, crd); err != nil {
+		return err
 	}
-
-	for crdExtraLabel := range cp.crdExtraLabels {
-		crd.ObjectMeta.Labels[crdExtraLabel] = cp.crdExtraLabels[crdExtraLabel]
+	if crd.APIVersion != apiextensionsv1.SchemeGroupVersion.String() || crd.Kind != "CustomResourceDefinition" {
+		return fmt.Errorf("invalid CRD document apiversion/kind: '%s/%s'", crd.APIVersion, crd.Kind)
 	}
 
 	cp.k8sTasks.Go(func() error {
-		err := cp.updateOrInsertCRD(ctx, crd)
+		err := cp.updateOrInsertCRD(ctx, crd, desired)
 		if err == nil {
 			var crdGroup, crdKind string
 
@@ -255,16 +254,13 @@ func (cp *CRDsInstaller) putCRDToCluster(ctx context.Context, crdReader io.Reade
 	return nil
 }
 
-func (cp *CRDsInstaller) updateOrInsertCRD(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition) error {
+func (cp *CRDsInstaller) updateOrInsertCRD(ctx context.Context, crd *apiextensionsv1.CustomResourceDefinition, desired *unstructured.Unstructured) error {
 	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		existCRD, err := cp.GetCRDFromCluster(ctx, crd.GetName())
+		existing, err := cp.k8sClient.Resource(crdGVR).Get(ctx, crd.GetName(), apimachineryv1.GetOptions{})
 		if apierrors.IsNotFound(err) {
-			ucrd, err := utils.ToUnstructured(crd)
-			if err != nil {
-				return err
-			}
+			mergeLabels(desired, cp.crdExtraLabels)
 
-			_, err = cp.k8sClient.Resource(crdGVR).Create(ctx, ucrd, apimachineryv1.CreateOptions{})
+			_, err = cp.k8sClient.Resource(crdGVR).Create(ctx, desired, apimachineryv1.CreateOptions{})
 			if err != nil {
 				return fmt.Errorf("create crd: %w", err)
 			}
@@ -276,6 +272,13 @@ func (cp *CRDsInstaller) updateOrInsertCRD(ctx context.Context, crd *apiextensio
 			return fmt.Errorf("get crd from cluster: %w", err)
 		}
 
+		// typed view of the existing object is used only to reconcile storedVersions;
+		// it is never written back as the CRD body (that would drop vendor extensions).
+		existCRD := &apiextensionsv1.CustomResourceDefinition{}
+		if err := runtime.DefaultUnstructuredConverter.FromUnstructured(existing.Object, existCRD); err != nil {
+			return fmt.Errorf("existing crd from unstructured: %w", err)
+		}
+
 		versionsFromNewSpec := make(map[string]struct{}, len(crd.Spec.Versions))
 		for _, version := range crd.Spec.Versions {
 			versionsFromNewSpec[version.Name] = struct{}{}
@@ -288,8 +291,10 @@ func (cp *CRDsInstaller) updateOrInsertCRD(ctx context.Context, crd *apiextensio
 			}
 		}
 
+		resourceVersion := existing.GetResourceVersion()
 		if !slices.Equal(newStoredVersions, existCRD.Status.StoredVersions) {
 			existCRD.Status.StoredVersions = newStoredVersions
+			// the status subresource update ignores .spec, so writing the typed (lossy) body here is safe
 			ucrd, err := utils.ToUnstructured(existCRD)
 			if err != nil {
 				return fmt.Errorf("crd to unstructured: %w", err)
@@ -301,38 +306,48 @@ func (cp *CRDsInstaller) updateOrInsertCRD(ctx context.Context, crd *apiextensio
 			}
 
 			if resp != nil {
-				existCRD.ObjectMeta.ResourceVersion = resp.GetResourceVersion()
+				resourceVersion = resp.GetResourceVersion()
 			}
 		}
 
-		if existCRD.Spec.Conversion != nil {
-			crd.Spec.Conversion = existCRD.Spec.Conversion
+		// keep the in-cluster conversion webhook config (it is not present in the CRD file)
+		if conv, found, err := unstructured.NestedFieldCopy(existing.Object, "spec", "conversion"); err == nil && found {
+			if err := unstructured.SetNestedField(desired.Object, conv, "spec", "conversion"); err != nil {
+				return fmt.Errorf("preserve conversion: %w", err)
+			}
 		}
 
-		if cmp.Equal(existCRD.Spec, crd.Spec) &&
-			cmp.Equal(existCRD.GetLabels(), crd.GetLabels()) &&
-			cmp.Equal(existCRD.GetAnnotations(), crd.GetAnnotations()) {
-			return nil
-		}
+		mergeLabels(desired, cp.crdExtraLabels)
 
-		existCRD.Spec = crd.Spec
-		existCRD.Labels = crd.Labels
-		existCRD.Annotations = crd.Annotations
+		desiredSpec, _, err := unstructured.NestedMap(desired.Object, "spec")
+		if err != nil {
+			return fmt.Errorf("read desired spec: %w", err)
+		}
 
-		if len(existCRD.ObjectMeta.Labels) == 0 {
-			existCRD.ObjectMeta.Labels = make(map[string]string, 1)
+		existingSpec, _, err := unstructured.NestedMap(existing.Object, "spec")
+		if err != nil {
+			return fmt.Errorf("read existing spec: %w", err)
 		}
 
-		for crdExtraLabel := range cp.crdExtraLabels {
-			existCRD.ObjectMeta.Labels[crdExtraLabel] = cp.crdExtraLabels[crdExtraLabel]
+		// diff on lossless unstructured specs so vendor extensions are not silently dropped
+		// ponytail: apiserver-defaulted .spec fields may differ from the manifest and cause
+		// reconcile churn; the update stays idempotent, tighten the diff here if it ever churns.
+		if cmp.Equal(existingSpec, desiredSpec) &&
+			cmp.Equal(existing.GetLabels(), desired.GetLabels()) &&
+			cmp.Equal(existing.GetAnnotations(), desired.GetAnnotations()) {
+			return nil
 		}
 
-		ucrd, err := utils.ToUnstructured(existCRD)
-		if err != nil {
-			return err
+		// write back the fetched object with only spec/labels/annotations overlaid, so
+		// server-managed metadata (finalizers, ownerReferences, uid, ...) is preserved.
+		if err := unstructured.SetNestedMap(existing.Object, desiredSpec, "spec"); err != nil {
+			return fmt.Errorf("set spec: %w", err)
 		}
+		existing.SetLabels(desired.GetLabels())
+		existing.SetAnnotations(desired.GetAnnotations())
+		existing.SetResourceVersion(resourceVersion)
 
-		_, err = cp.k8sClient.Resource(crdGVR).Update(ctx, ucrd, apimachineryv1.UpdateOptions{})
+		_, err = cp.k8sClient.Resource(crdGVR).Update(ctx, existing, apimachineryv1.UpdateOptions{})
 		if err != nil {
 			return fmt.Errorf("update crd: %w", err)
 		}
@@ -341,6 +356,19 @@ func (cp *CRDsInstaller) updateOrInsertCRD(ctx context.Context, crd *apiextensio
 	})
 }
 
+func mergeLabels(u *unstructured.Unstructured, extra map[string]string) {
+	labels := u.GetLabels()
+	if labels == nil {
+		labels = make(map[string]string, len(extra))
+	}
+
+	for k, v := range extra {
+		labels[k] = v
+	}
+
+	u.SetLabels(labels)
+}
+
 func (cp *CRDsInstaller) GetCRDFromCluster(ctx context.Context, crdName string) (*apiextensionsv1.CustomResourceDefinition, error) {
 	crd := &apiextensionsv1.CustomResourceDefinition{}
 

pkg/crd-installer/installer_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apimachineryv1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic/fake"
@@ -66,4 +67,94 @@ func TestCRDInstaller(t *testing.T) {
 		f4 := crd.Spec.Versions[0].Schema.OpenAPIV3Schema.Properties["spec"].Properties["field4"].Type
 		assert.Equal(t, "boolean", f4)
 	})
+
+	// Regression: vendor schema extensions (x-kubernetes-sensitive-data and friends) must
+	// survive the install path. Decoding CRDs into the typed struct drops them, so the
+	// installer must apply the object as unstructured.
+	t.Run("preserves vendor schema extensions", func(t *testing.T) {
+		inst := NewCRDsInstaller(fc, []string{"testdata/3_sensitive.yaml"})
+		require.NoError(t, inst.Run(context.Background()))
+
+		un, err := fc.Resource(gvr).Get(context.Background(), "secrets.example.com", apimachineryv1.GetOptions{})
+		require.NoError(t, err)
+
+		versions, found, err := unstructured.NestedSlice(un.Object, "spec", "versions")
+		require.NoError(t, err)
+		require.True(t, found, "spec.versions must be present")
+
+		token, found, err := unstructured.NestedMap(versions[0].(map[string]any),
+			"schema", "openAPIV3Schema", "properties", "spec", "properties", "token")
+		require.NoError(t, err)
+		require.True(t, found, "token property must be present")
+		assert.Equal(t, true, token["x-kubernetes-sensitive-data"],
+			"vendor extension must survive the install path")
+	})
+
+	// Regression: updating an existing CRD must apply the manifest spec (with vendor
+	// extensions) while preserving server-managed metadata (finalizers) and the in-cluster
+	// conversion config, which is never present in the manifest.
+	t.Run("update preserves finalizers and in-cluster conversion", func(t *testing.T) {
+		seed := &unstructured.Unstructured{Object: map[string]any{
+			"apiVersion": "apiextensions.k8s.io/v1",
+			"kind":       "CustomResourceDefinition",
+			"metadata": map[string]any{
+				"name":       "things.example.com",
+				"finalizers": []any{"customresourcecleanup.apiextensions.k8s.io"},
+			},
+			"spec": map[string]any{
+				"group": "example.com",
+				"names": map[string]any{
+					"kind": "Thing", "listKind": "ThingList", "plural": "things", "singular": "thing",
+				},
+				"scope": "Namespaced",
+				"conversion": map[string]any{
+					"strategy": "Webhook",
+					"webhook": map[string]any{
+						"conversionReviewVersions": []any{"v1"},
+						"clientConfig": map[string]any{
+							"service": map[string]any{"name": "conv", "namespace": "default"},
+						},
+					},
+				},
+				"versions": []any{
+					map[string]any{"name": "v1", "served": true, "storage": true},
+				},
+			},
+		}}
+		_, err := fc.Resource(gvr).Create(context.Background(), seed, apimachineryv1.CreateOptions{})
+		require.NoError(t, err)
+
+		inst := NewCRDsInstaller(fc, []string{"testdata/4_conversion.yaml"})
+		require.NoError(t, inst.Run(context.Background()))
+
+		un, err := fc.Resource(gvr).Get(context.Background(), "things.example.com", apimachineryv1.GetOptions{})
+		require.NoError(t, err)
+
+		finalizers, found, err := unstructured.NestedStringSlice(un.Object, "metadata", "finalizers")
+		require.NoError(t, err)
+		require.True(t, found, "finalizers must be preserved")
+		assert.Contains(t, finalizers, "customresourcecleanup.apiextensions.k8s.io")
+
+		_, found, err = unstructured.NestedMap(un.Object, "spec", "conversion")
+		require.NoError(t, err)
+		assert.True(t, found, "in-cluster conversion must be preserved")
+
+		versions, _, err := unstructured.NestedSlice(un.Object, "spec", "versions")
+		require.NoError(t, err)
+		token, found, err := unstructured.NestedMap(versions[0].(map[string]any),
+			"schema", "openAPIV3Schema", "properties", "spec", "properties", "token")
+		require.NoError(t, err)
+		require.True(t, found, "manifest schema must be applied")
+		assert.Equal(t, true, token["x-kubernetes-sensitive-data"])
+	})
+
+	// Regression: a comment-only yaml document decodes to a nil object and must be skipped,
+	// not panic on the nil dereference.
+	t.Run("skips comment-only documents", func(t *testing.T) {
+		inst := NewCRDsInstaller(fc, []string{"testdata/5_comment.yaml"})
+		require.NoError(t, inst.Run(context.Background()))
+
+		_, err := fc.Resource(gvr).Get(context.Background(), "comments.example.com", apimachineryv1.GetOptions{})
+		require.NoError(t, err)
+	})
 }

pkg/crd-installer/testdata/3_sensitive.yaml

@@ -0,0 +1,26 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: secrets.example.com
+spec:
+  group: example.com
+  names:
+    kind: Secret
+    listKind: SecretList
+    plural: secrets
+    singular: secret
+  scope: Namespaced
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            spec:
+              type: object
+              properties:
+                token:
+                  type: string
+                  x-kubernetes-sensitive-data: true

pkg/crd-installer/testdata/4_conversion.yaml

@@ -0,0 +1,26 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: things.example.com
+spec:
+  group: example.com
+  names:
+    kind: Thing
+    listKind: ThingList
+    plural: things
+    singular: thing
+  scope: Namespaced
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            spec:
+              type: object
+              properties:
+                token:
+                  type: string
+                  x-kubernetes-sensitive-data: true

pkg/crd-installer/testdata/5_comment.yaml

@@ -0,0 +1,21 @@
+# leading comment document — decodes to null and must be skipped, not crash
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: comments.example.com
+spec:
+  group: example.com
+  names:
+    kind: Comment
+    listKind: CommentList
+    plural: comments
+    singular: comment
+  scope: Namespaced
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object