diff --git a/pkg/module_manager/conversion_webhook_ownership_test.go b/pkg/module_manager/conversion_webhook_ownership_test.go new file mode 100644 index 00000000..9df5b267 --- /dev/null +++ b/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) + }) +} diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index 550ab9b1..a6e5adb5 100644 --- a/pkg/module_manager/module_manager.go +++ b/pkg/module_manager/module_manager.go @@ -40,6 +40,7 @@ import ( 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 @@ func (mm *ModuleManager) EnsureConversionWebhooks(moduleName string, logLabels m 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 @@ func (mm *ModuleManager) EnsureConversionWebhooks(moduleName string, logLabels m 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.