add helm ownership for hooks

pkg/module_manager/conversion_webhook_ownership_test.go

@@ -0,0 +1,98 @@
+package module_manager
+
+import (
+	"testing"
+
+	"github.com/flant/kube-client/manifest"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSetHelmOwnershipMetadata(t *testing.T) {
+	const (
+		releaseName = "log-shipper"
+		releaseNs   = "d8-system"
+	)
+
+	assertOwnership := func(t *testing.T, m manifest.Manifest) {
+		t.Helper()
+
+		meta := m["metadata"].(map[string]interface{})
+
+		labels := meta["labels"].(map[string]interface{})
+		assert.Equal(t, "Helm", labels[helmManagedByLabel])
+
+		annotations := meta["annotations"].(map[string]interface{})
+		assert.Equal(t, releaseName, annotations[helmReleaseNameAnnotation])
+		assert.Equal(t, releaseNs, annotations[helmReleaseNamespaceAnnotation])
+	}
+
+	t.Run("adds metadata when labels and annotations are absent", func(t *testing.T) {
+		m := manifest.Manifest{
+			"apiVersion": "deckhouse.io/v1alpha1",
+			"kind":       "ConversionWebhook",
+			"metadata": map[string]interface{}{
+				"name": "clusterlogdestinations.deckhouse.io",
+			},
+		}
+
+		setHelmOwnershipMetadata(m, releaseName, releaseNs)
+
+		assertOwnership(t, m)
+	})
+
+	t.Run("preserves existing labels and annotations", func(t *testing.T) {
+		m := manifest.Manifest{
+			"kind": "ConversionWebhook",
+			"metadata": map[string]interface{}{
+				"name": "clusterlogdestinations.deckhouse.io",
+				"labels": map[string]interface{}{
+					"heritage": "deckhouse",
+					"module":   "log-shipper",
+				},
+				"annotations": map[string]interface{}{
+					"some.io/keep": "value",
+				},
+			},
+		}
+
+		setHelmOwnershipMetadata(m, releaseName, releaseNs)
+
+		assertOwnership(t, m)
+		meta := m["metadata"].(map[string]interface{})
+		labels := meta["labels"].(map[string]interface{})
+		assert.Equal(t, "deckhouse", labels["heritage"])
+		assert.Equal(t, "log-shipper", labels["module"])
+		annotations := meta["annotations"].(map[string]interface{})
+		assert.Equal(t, "value", annotations["some.io/keep"])
+	})
+
+	t.Run("is idempotent on an already-owned object", func(t *testing.T) {
+		m := manifest.Manifest{
+			"kind": "ConversionWebhook",
+			"metadata": map[string]interface{}{
+				"name": "clusterlogdestinations.deckhouse.io",
+				"labels": map[string]interface{}{
+					helmManagedByLabel: "Helm",
+				},
+				"annotations": map[string]interface{}{
+					helmReleaseNameAnnotation:      releaseName,
+					helmReleaseNamespaceAnnotation: releaseNs,
+				},
+			},
+		}
+
+		setHelmOwnershipMetadata(m, releaseName, releaseNs)
+
+		assertOwnership(t, m)
+	})
+
+	t.Run("creates metadata when it is missing entirely", func(t *testing.T) {
+		m := manifest.Manifest{
+			"kind": "ConversionWebhook",
+		}
+
+		setHelmOwnershipMetadata(m, releaseName, releaseNs)
+
+		assertOwnership(t, m)
+	})
+}

pkg/module_manager/module_manager.go

@@ -40,6 +40,7 @@
 	static_extender "github.com/flant/addon-operator/pkg/module_manager/scheduler/extenders/static"
 	"github.com/flant/addon-operator/pkg/task"
 	"github.com/flant/addon-operator/pkg/utils"
+	"github.com/flant/kube-client/manifest"
 	. "github.com/flant/shell-operator/pkg/hook/binding_context"
 	"github.com/flant/shell-operator/pkg/hook/controller"
 	. "github.com/flant/shell-operator/pkg/hook/types"
@@ -918,6 +919,13 @@
 
 	ops := make([]sdkpkg.PatchCollectorOperation, 0, len(webhooks))
 	for _, m := range webhooks {
+		// ConversionWebhook resources also live in the module's Helm chart
+		// (templates/), so they are rendered again and reconciled by the
+		// subsequent helm release. Stamp Helm's ownership metadata here so that
+		// helm can adopt the object we create instead of failing with
+		// "invalid ownership metadata" (missing app.kubernetes.io/managed-by and
+		// meta.helm.sh/release-* markers).
+		setHelmOwnershipMetadata(m, moduleName, mm.defaultNamespace)
 		ops = append(ops, objectpatch.NewCreateOrUpdateOperation(map[string]any(m)))
 	}
 
@@ -932,6 +940,46 @@
 	return nil
 }
 
+// Helm release ownership markers. Helm refuses to adopt a pre-existing object
+// unless it carries these exact label/annotations, see
+// https://helm.sh/docs/faq/#improved-upgrade-strategy-3-way-strategic-merge-patches.
+const (
+	helmManagedByLabel             = "app.kubernetes.io/managed-by"
+	helmManagedByValue             = "Helm"
+	helmReleaseNameAnnotation      = "meta.helm.sh/release-name"
+	helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace"
+)
+
+// setHelmOwnershipMetadata stamps the Helm release ownership label and
+// annotations onto a rendered manifest. ConversionWebhook resources are applied
+// by EnsureConversionWebhooks (outside Helm) before the module's Helm release
+// runs, yet they are also part of the chart. Without these markers the following
+// helm upgrade refuses to adopt the object we just created and fails with
+// "invalid ownership metadata". For a module, the Helm release name equals the
+// module name and the release namespace is the module's default namespace.
+func setHelmOwnershipMetadata(m manifest.Manifest, releaseName, releaseNamespace string) {
+	meta, ok := m["metadata"].(map[string]interface{})
+	if !ok {
+		meta = map[string]interface{}{}
+		m["metadata"] = meta
+	}
+
+	labels, ok := meta["labels"].(map[string]interface{})
+	if !ok {
+		labels = map[string]interface{}{}
+		meta["labels"] = labels
+	}
+	labels[helmManagedByLabel] = helmManagedByValue
+
+	annotations, ok := meta["annotations"].(map[string]interface{})
+	if !ok {
+		annotations = map[string]interface{}{}
+		meta["annotations"] = annotations
+	}
+	annotations[helmReleaseNameAnnotation] = releaseName
+	annotations[helmReleaseNamespaceAnnotation] = releaseNamespace
+}
+
 // ModuleHasConversionWebhooks reports whether the module's templates contain at
 // least one ConversionWebhook resource. It is a cheap static check used to skip
 // rendering for modules that have no conversion webhooks.