From ccce0b3b2224234a8a181decf86e2d0937aa9136 Mon Sep 17 00:00:00 2001 From: cg49996w11 Date: Sun, 12 Jul 2026 13:54:37 +0530 Subject: [PATCH] incorporate copilot comments Signed-off-by: cg49996w11 --- pkg/action/validate.go | 14 ++++++-------- pkg/kube/client.go | 3 +++ pkg/kube/client_test.go | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 948005521..dfb9fe6b3 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -32,10 +32,8 @@ 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" + appManagedByLabel = "app.kubernetes.io/managed-by" + appManagedByHelm = "Helm" ) // requireAdoption returns the subset of resources that already exist in the cluster. @@ -181,10 +179,10 @@ func checkOwnership(obj runtime.Object, releaseName, releaseNamespace string) er if err := requireValue(lbls, appManagedByLabel, appManagedByHelm); err != nil { errs = append(errs, fmt.Errorf("label validation error: %w", err)) } - if err := requireValue(annos, helmReleaseNameAnnotation, releaseName); err != nil { + if err := requireValue(annos, kube.ReleaseNameAnnotation, releaseName); err != nil { errs = append(errs, fmt.Errorf("annotation validation error: %w", err)) } - if err := requireValue(annos, helmReleaseNamespaceAnnotation, releaseNamespace); err != nil { + if err := requireValue(annos, kube.ReleaseNamespaceAnnotation, releaseNamespace); err != nil { errs = append(errs, fmt.Errorf("annotation validation error: %w", err)) } @@ -231,8 +229,8 @@ func setMetadataVisitor(releaseName, releaseNamespace string, forceOwnership boo } if err := mergeAnnotations(info.Object, map[string]string{ - helmReleaseNameAnnotation: releaseName, - helmReleaseNamespaceAnnotation: releaseNamespace, + kube.ReleaseNameAnnotation: releaseName, + kube.ReleaseNamespaceAnnotation: releaseNamespace, }); err != nil { return fmt.Errorf( "%s annotations could not be updated: %w", diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 65a9cb492..66ac90db1 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -812,6 +812,9 @@ func ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManag // release. Resources annotated as belonging to a different release will be skipped. func ClientUpdateOptionOwnership(releaseName, releaseNamespace string) ClientUpdateOption { return func(o *clientUpdateOptions) error { + if releaseName == "" { + return errors.New("releaseName must not be empty for ownership check") + } o.releaseName = releaseName o.releaseNamespace = releaseNamespace diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 655651c86..5dcf5eaf9 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -663,6 +663,22 @@ func TestUpdateOwnershipCheck(t *testing.T) { } } +func TestClientUpdateOptionOwnershipValidation(t *testing.T) { + // An empty releaseName must be rejected so callers cannot silently disable + // the ownership check by passing a zero-value string. + opt := ClientUpdateOptionOwnership("", "some-ns") + var o clientUpdateOptions + err := opt(&o) + assert.ErrorContains(t, err, "releaseName must not be empty") + + // Non-empty releaseName with empty namespace is allowed (cluster-scoped resources). + opt2 := ClientUpdateOptionOwnership("my-release", "") + err = opt2(&o) + assert.NoError(t, err) + assert.Equal(t, "my-release", o.releaseName) + assert.Equal(t, "", o.releaseNamespace) +} + func TestBuild(t *testing.T) { tests := []struct { name string