From a29365b3c663f1c182ea78461825ae28b8573915 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 20 Feb 2020 19:42:47 -0500 Subject: [PATCH 1/5] Adopt resources into release with correct instance and managed-by labels Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 8 ++++++-- pkg/action/upgrade.go | 13 ++++++++++-- pkg/action/validate.go | 45 ++++++++++++++++++++++++++++++++++++++---- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 79abfae33..ac57b921c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -38,6 +38,7 @@ import ( "helm.sh/helm/v3/pkg/downloader" "helm.sh/helm/v3/pkg/engine" "helm.sh/helm/v3/pkg/getter" + "helm.sh/helm/v3/pkg/kube" kubefake "helm.sh/helm/v3/pkg/kube/fake" "helm.sh/helm/v3/pkg/postrender" "helm.sh/helm/v3/pkg/release" @@ -242,6 +243,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // Mark this release as in-progress rel.SetStatus(release.StatusPendingInstall, "Initial install underway") + var adoptedResources kube.ResourceList resources, err := i.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), !i.DisableOpenAPIValidation) if err != nil { return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") @@ -254,9 +256,11 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // deleting the release because the manifest will be pointing at that // resource if !i.ClientOnly && !isUpgrade { - if err := existingResourceConflict(resources); err != nil { + toBeUpdated, err := existingResourceConflict(resources, rel.Name) + if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") } + adoptedResources = toBeUpdated } // Bail out here if it is a dry run @@ -291,7 +295,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // At this point, we can do the install. Note that before we were detecting whether to // do an update, but it's not clear whether we WANT to do an update if the re-use is set // to true, since that is basically an upgrade operation. - if _, err := i.cfg.KubeClient.Create(resources); err != nil { + if _, err := i.cfg.KubeClient.Update(adoptedResources, resources, false); err != nil { return i.failRelease(rel, err) } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 08b638171..be64fa618 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -216,10 +216,19 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea } } - if err := existingResourceConflict(toBeCreated); err != nil { - return nil, errors.Wrap(err, "rendered manifests contain a new resource that already exists. Unable to continue with update") + toBeUpdated, err := existingResourceConflict(toBeCreated, upgradedRelease.Name) + if err != nil { + return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with update") } + toBeUpdated.Visit(func(r *resource.Info, err error) error { + if err != nil { + return err + } + current.Append(r) + return nil + }) + if u.DryRun { u.cfg.Log("dry run for %s", upgradedRelease.Name) if len(u.Description) > 0 { diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 6bbfc5e8d..feecc4ff3 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -21,13 +21,37 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/v3/pkg/kube" ) -func existingResourceConflict(resources kube.ResourceList) error { - err := resources.Visit(func(info *resource.Info, err error) error { +var ( + managedByReq, _ = labels.NewRequirement("app.kubernetes.io/managed-by", selection.Equals, []string{"Helm"}) + accessor = meta.NewAccessor() +) + +func newReleaseSelector(release string) (labels.Selector, error) { + releaseReq, err := labels.NewRequirement("app.kubernetes.io/instance", selection.Equals, []string{release}) + if err != nil { + return nil, err + } + + return labels.Parse(fmt.Sprintf("%s,%s", releaseReq, managedByReq)) +} + +func existingResourceConflict(resources kube.ResourceList, release string) (kube.ResourceList, error) { + sel, err := newReleaseSelector(release) + if err != nil { + return nil, err + } + + requireUpdate := kube.ResourceList{} + + err = resources.Visit(func(info *resource.Info, err error) error { if err != nil { return err } @@ -42,7 +66,20 @@ func existingResourceConflict(resources kube.ResourceList) error { return errors.Wrap(err, "could not get information about the resource") } - return fmt.Errorf("existing resource conflict: namespace: %s, name: %s, existing_kind: %s, new_kind: %s", info.Namespace, info.Name, existing.GetObjectKind().GroupVersionKind(), info.Mapping.GroupVersionKind) + // Allow adoption of the resource if it already has app.kubernetes.io/instance and app.kubernetes.io/managed-by labels. + lbls, err := accessor.Labels(existing) + if err == nil { + set := labels.Set(lbls) + if sel.Matches(set) { + requireUpdate = append(requireUpdate, info) + return nil + } + } + + return fmt.Errorf( + "existing resource conflict: namespace: %s, name: %s, existing_kind: %s, new_kind: %s", + info.Namespace, info.Name, existing.GetObjectKind().GroupVersionKind(), info.Mapping.GroupVersionKind) }) - return err + + return requireUpdate, err } From 271cb2268bfd94012639f4866e8d52189e12a97b Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 20 Feb 2020 20:53:27 -0500 Subject: [PATCH 2/5] Alternative: annotation-only solution Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 7 +++++- pkg/action/upgrade.go | 7 +++++- pkg/action/validate.go | 57 +++++++++++++++++++++--------------------- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index ac57b921c..cab55309c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -249,6 +249,11 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") } + err = resources.Visit(setMetadataVisitor(rel.Name, rel.Namespace)) + if err != nil { + return nil, err + } + // Install requires an extra validation step of checking that resources // don't already exist before we actually create resources. If we continue // forward and create the release object with resources that already exist, @@ -256,7 +261,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // deleting the release because the manifest will be pointing at that // resource if !i.ClientOnly && !isUpgrade { - toBeUpdated, err := existingResourceConflict(resources, rel.Name) + toBeUpdated, err := existingResourceConflict(resources, rel.Name, rel.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index be64fa618..b42192cda 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -203,6 +203,11 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } + err = target.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace)) + if err != nil { + return upgradedRelease, err + } + // Do a basic diff using gvk + name to figure out what new resources are being created so we can validate they don't already exist existingResources := make(map[string]bool) for _, r := range current { @@ -216,7 +221,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea } } - toBeUpdated, err := existingResourceConflict(toBeCreated, upgradedRelease.Name) + toBeUpdated, err := existingResourceConflict(toBeCreated, upgradedRelease.Name, upgradedRelease.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with update") } diff --git a/pkg/action/validate.go b/pkg/action/validate.go index feecc4ff3..17a61863e 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -22,36 +22,22 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/v3/pkg/kube" ) -var ( - managedByReq, _ = labels.NewRequirement("app.kubernetes.io/managed-by", selection.Equals, []string{"Helm"}) - accessor = meta.NewAccessor() -) - -func newReleaseSelector(release string) (labels.Selector, error) { - releaseReq, err := labels.NewRequirement("app.kubernetes.io/instance", selection.Equals, []string{release}) - if err != nil { - return nil, err - } - - return labels.Parse(fmt.Sprintf("%s,%s", releaseReq, managedByReq)) -} +var accessor = meta.NewAccessor() -func existingResourceConflict(resources kube.ResourceList, release string) (kube.ResourceList, error) { - sel, err := newReleaseSelector(release) - if err != nil { - return nil, err - } +const ( + helmReleaseNameAnnotation = "meta.helm.sh/release-name" + helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" +) +func existingResourceConflict(resources kube.ResourceList, releaseName, releaseNamespace string) (kube.ResourceList, error) { requireUpdate := kube.ResourceList{} - err = resources.Visit(func(info *resource.Info, err error) error { + err := resources.Visit(func(info *resource.Info, err error) error { if err != nil { return err } @@ -67,13 +53,10 @@ func existingResourceConflict(resources kube.ResourceList, release string) (kube } // Allow adoption of the resource if it already has app.kubernetes.io/instance and app.kubernetes.io/managed-by labels. - lbls, err := accessor.Labels(existing) - if err == nil { - set := labels.Set(lbls) - if sel.Matches(set) { - requireUpdate = append(requireUpdate, info) - return nil - } + anno, err := accessor.Annotations(existing) + if err == nil && anno != nil && anno[helmReleaseNameAnnotation] == releaseName && anno[helmReleaseNamespaceAnnotation] == releaseNamespace { + requireUpdate = append(requireUpdate, info) + return nil } return fmt.Errorf( @@ -83,3 +66,21 @@ func existingResourceConflict(resources kube.ResourceList, release string) (kube return requireUpdate, err } + +func setMetadataVisitor(releaseName, releaseNamespace string) resource.VisitorFunc { + return func(info *resource.Info, err error) error { + if err != nil { + return err + } + anno, err := accessor.Annotations(info.Object) + if err != nil { + return err + } + if anno == nil { + anno = make(map[string]string) + } + anno[helmReleaseNameAnnotation] = releaseName + anno[helmReleaseNamespaceAnnotation] = releaseNamespace + return accessor.SetAnnotations(info.Object, anno) + } +} From d829343c1514db17bee7a90624d06cdfbffde963 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 5 Mar 2020 21:41:30 -0500 Subject: [PATCH 3/5] Use Create method if no resources need to be adopted Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index cab55309c..d6fd829e5 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -243,7 +243,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // Mark this release as in-progress rel.SetStatus(release.StatusPendingInstall, "Initial install underway") - var adoptedResources kube.ResourceList + var toBeAdopted kube.ResourceList resources, err := i.cfg.KubeClient.Build(bytes.NewBufferString(rel.Manifest), !i.DisableOpenAPIValidation) if err != nil { return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") @@ -261,11 +261,10 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // deleting the release because the manifest will be pointing at that // resource if !i.ClientOnly && !isUpgrade { - toBeUpdated, err := existingResourceConflict(resources, rel.Name, rel.Namespace) + toBeAdopted, err = existingResourceConflict(resources, rel.Name, rel.Namespace) if err != nil { return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") } - adoptedResources = toBeUpdated } // Bail out here if it is a dry run @@ -300,8 +299,14 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. // At this point, we can do the install. Note that before we were detecting whether to // do an update, but it's not clear whether we WANT to do an update if the re-use is set // to true, since that is basically an upgrade operation. - if _, err := i.cfg.KubeClient.Update(adoptedResources, resources, false); err != nil { - return i.failRelease(rel, err) + if len(toBeAdopted) == 0 { + if _, err := i.cfg.KubeClient.Create(resources); err != nil { + return i.failRelease(rel, err) + } + } else { + if _, err := i.cfg.KubeClient.Update(toBeAdopted, resources, false); err != nil { + return i.failRelease(rel, err) + } } if i.Wait { From 8d1de39fe3234c8925dac6d3efca6aba687ebf87 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Tue, 10 Mar 2020 22:22:15 -0400 Subject: [PATCH 4/5] Add more detail to error messages and support a non-force mode in metadata visitor Signed-off-by: Jacob LeGrone --- pkg/action/install.go | 3 +- pkg/action/upgrade.go | 3 +- pkg/action/validate.go | 136 +++++++++++++++++++++++++++++++++++------ 3 files changed, 121 insertions(+), 21 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index d6fd829e5..f7e8f77d1 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -249,7 +249,8 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. return nil, errors.Wrap(err, "unable to build kubernetes objects from release manifest") } - err = resources.Visit(setMetadataVisitor(rel.Name, rel.Namespace)) + // It is safe to use "force" here because these are resources currently rendered by the chart. + err = resources.Visit(setMetadataVisitor(rel.Name, rel.Namespace, true)) if err != nil { return nil, err } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index b42192cda..0741ef19f 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -203,7 +203,8 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } - err = target.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace)) + // It is safe to use force only on target because these are resources currently rendered by the chart. + err = target.Visit(setMetadataVisitor(upgradedRelease.Name, upgradedRelease.Namespace, true)) if err != nil { return upgradedRelease, err } diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 17a61863e..0c40a9c3c 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -22,6 +22,7 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/v3/pkg/kube" @@ -30,12 +31,14 @@ import ( var accessor = meta.NewAccessor() const ( + appManagedByLabel = "app.kubernetes.io/managed-by" + appManagedByHelm = "Helm" helmReleaseNameAnnotation = "meta.helm.sh/release-name" helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace" ) func existingResourceConflict(resources kube.ResourceList, releaseName, releaseNamespace string) (kube.ResourceList, error) { - requireUpdate := kube.ResourceList{} + var requireUpdate kube.ResourceList err := resources.Visit(func(info *resource.Info, err error) error { if err != nil { @@ -48,39 +51,134 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN if apierrors.IsNotFound(err) { return nil } - return errors.Wrap(err, "could not get information about the resource") } - // Allow adoption of the resource if it already has app.kubernetes.io/instance and app.kubernetes.io/managed-by labels. - anno, err := accessor.Annotations(existing) - if err == nil && anno != nil && anno[helmReleaseNameAnnotation] == releaseName && anno[helmReleaseNamespaceAnnotation] == releaseNamespace { - requireUpdate = append(requireUpdate, info) - return nil + // Allow adoption of the resource if it is managed by Helm and is annotated with correct release name and namespace. + if err := checkOwnership(existing, releaseName, releaseNamespace); err != nil { + return fmt.Errorf("%s exists and cannot be imported into the current release: %s", resourceString(info), err) } - return fmt.Errorf( - "existing resource conflict: namespace: %s, name: %s, existing_kind: %s, new_kind: %s", - info.Namespace, info.Name, existing.GetObjectKind().GroupVersionKind(), info.Mapping.GroupVersionKind) + requireUpdate.Append(info) + return nil }) return requireUpdate, err } -func setMetadataVisitor(releaseName, releaseNamespace string) resource.VisitorFunc { +func checkOwnership(obj runtime.Object, releaseName, releaseNamespace string) error { + lbls, err := accessor.Labels(obj) + if err != nil { + return err + } + annos, err := accessor.Annotations(obj) + if err != nil { + return err + } + + var errs []error + if err := requireValue(lbls, appManagedByLabel, appManagedByHelm); err != nil { + errs = append(errs, fmt.Errorf("label validation error: %s", err)) + } + if err := requireValue(annos, helmReleaseNameAnnotation, releaseName); err != nil { + errs = append(errs, fmt.Errorf("annotation validation error: %s", err)) + } + if err := requireValue(annos, helmReleaseNamespaceAnnotation, releaseNamespace); err != nil { + errs = append(errs, fmt.Errorf("annotation validation error: %s", err)) + } + + if len(errs) > 0 { + err := errors.New("invalid ownership metadata") + for _, e := range errs { + err = fmt.Errorf("%w; %s", err, e) + } + return err + } + + return nil +} + +func requireValue(meta map[string]string, k, v string) error { + actual, ok := meta[k] + if !ok { + return fmt.Errorf("missing key %q: must be set to %q", k, v) + } + if actual != v { + return fmt.Errorf("key %q must equal %q: current value is %q", k, v, actual) + } + return nil +} + +// setMetadataVisitor adds release tracking metadata to all resources. If force is enabled, existing +// ownership metadata will be overwritten. Otherwise an error will be returned if any resource has an +// existing and conflicting value for the managed by label or Helm release/namespace annotations. +func setMetadataVisitor(releaseName, releaseNamespace string, force bool) resource.VisitorFunc { return func(info *resource.Info, err error) error { if err != nil { return err } - anno, err := accessor.Annotations(info.Object) - if err != nil { - return err + + if !force { + if err := checkOwnership(info.Object, releaseName, releaseNamespace); err != nil { + return fmt.Errorf("%s cannot be owned: %s", resourceString(info), err) + } } - if anno == nil { - anno = make(map[string]string) + + if err := mergeLabels(info.Object, map[string]string{ + appManagedByLabel: appManagedByHelm, + }); err != nil { + return fmt.Errorf( + "%s labels could not be updated: %s", + resourceString(info), err, + ) + } + + if err := mergeAnnotations(info.Object, map[string]string{ + helmReleaseNameAnnotation: releaseName, + helmReleaseNamespaceAnnotation: releaseNamespace, + }); err != nil { + return fmt.Errorf( + "%s annotations could not be updated: %s", + resourceString(info), err, + ) } - anno[helmReleaseNameAnnotation] = releaseName - anno[helmReleaseNamespaceAnnotation] = releaseNamespace - return accessor.SetAnnotations(info.Object, anno) + + return nil + } +} + +func resourceString(info *resource.Info) string { + _, k := info.Mapping.GroupVersionKind.ToAPIVersionAndKind() + return fmt.Sprintf( + "%s %q in namespace %q", + k, info.Name, info.Namespace, + ) +} + +func mergeLabels(obj runtime.Object, labels map[string]string) error { + current, err := accessor.Labels(obj) + if err != nil { + return err + } + return accessor.SetLabels(obj, mergeStrStrMaps(current, labels)) +} + +func mergeAnnotations(obj runtime.Object, annotations map[string]string) error { + current, err := accessor.Annotations(obj) + if err != nil { + return err + } + return accessor.SetAnnotations(obj, mergeStrStrMaps(current, annotations)) +} + +// merge two maps, always taking the value on the right +func mergeStrStrMaps(current, desired map[string]string) map[string]string { + result := make(map[string]string) + for k, v := range current { + result[k] = v + } + for k, desiredVal := range desired { + result[k] = desiredVal } + return result } From 9496a7474bf3bcf8bb0b7efc953b85174ae8d8da Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Tue, 10 Mar 2020 22:22:33 -0400 Subject: [PATCH 5/5] Add tests Signed-off-by: Jacob LeGrone --- pkg/action/validate_test.go | 123 ++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 pkg/action/validate_test.go diff --git a/pkg/action/validate_test.go b/pkg/action/validate_test.go new file mode 100644 index 000000000..a9c1cb49c --- /dev/null +++ b/pkg/action/validate_test.go @@ -0,0 +1,123 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "helm.sh/helm/v3/pkg/kube" + + appsv1 "k8s.io/api/apps/v1" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/meta" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/cli-runtime/pkg/resource" +) + +func newDeploymentResource(name, namespace string) *resource.Info { + return &resource.Info{ + Name: name, + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployment"}, + GroupVersionKind: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + }, + Object: &appsv1.Deployment{ + ObjectMeta: v1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + }, + } +} + +func TestCheckOwnership(t *testing.T) { + deployFoo := newDeploymentResource("foo", "ns-a") + + // Verify that a resource that lacks labels/annotations is not owned + err := checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; label validation error: missing key "app.kubernetes.io/managed-by": must be set to "Helm"; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "rel-a"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "ns-a"`) + + // Set managed by label and verify annotation error message + _ = accessor.SetLabels(deployFoo.Object, map[string]string{ + appManagedByLabel: appManagedByHelm, + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: missing key "meta.helm.sh/release-name": must be set to "rel-a"; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "ns-a"`) + + // Set only the release name annotation and verify missing release namespace error message + _ = accessor.SetAnnotations(deployFoo.Object, map[string]string{ + helmReleaseNameAnnotation: "rel-a", + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: missing key "meta.helm.sh/release-namespace": must be set to "ns-a"`) + + // Set both release name and namespace annotations and verify no ownership errors + _ = accessor.SetAnnotations(deployFoo.Object, map[string]string{ + helmReleaseNameAnnotation: "rel-a", + helmReleaseNamespaceAnnotation: "ns-a", + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.NoError(t, err) + + // Verify ownership error for wrong release name + err = checkOwnership(deployFoo.Object, "rel-b", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-name" must equal "rel-b": current value is "rel-a"`) + + // Verify ownership error for wrong release namespace + err = checkOwnership(deployFoo.Object, "rel-a", "ns-b") + assert.EqualError(t, err, `invalid ownership metadata; annotation validation error: key "meta.helm.sh/release-namespace" must equal "ns-b": current value is "ns-a"`) + + // Verify ownership error for wrong manager label + _ = accessor.SetLabels(deployFoo.Object, map[string]string{ + appManagedByLabel: "helm", + }) + err = checkOwnership(deployFoo.Object, "rel-a", "ns-a") + assert.EqualError(t, err, `invalid ownership metadata; label validation error: key "app.kubernetes.io/managed-by" must equal "Helm": current value is "helm"`) +} + +func TestSetMetadataVisitor(t *testing.T) { + var ( + err error + deployFoo = newDeploymentResource("foo", "ns-a") + deployBar = newDeploymentResource("bar", "ns-a-system") + resources = kube.ResourceList{deployFoo, deployBar} + ) + + // Set release tracking metadata and verify no error + err = resources.Visit(setMetadataVisitor("rel-a", "ns-a", true)) + assert.NoError(t, err) + + // Verify that release "b" cannot take ownership of "a" + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false)) + assert.Error(t, err) + + // Force release "b" to take ownership + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", true)) + assert.NoError(t, err) + + // Check that there is now no ownership error when setting metadata without force + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false)) + assert.NoError(t, err) + + // Add a new resource that is missing ownership metadata and verify error + resources.Append(newDeploymentResource("baz", "default")) + err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false)) + assert.Error(t, err) + assert.Contains(t, err.Error(), `Deployment "baz" in namespace "" cannot be owned`) +}