From 2820ebe8c97b7d7b8a447375b74c9cb3741a4ffa Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Sun, 23 Nov 2025 21:58:23 +0530 Subject: [PATCH 1/8] fixed: --dry-run=server now respect generateName Signed-off-by: Mujib Ahasan --- pkg/kube/resource.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/kube/resource.go b/pkg/kube/resource.go index d88b171f0..98d2cb7f9 100644 --- a/pkg/kube/resource.go +++ b/pkg/kube/resource.go @@ -29,6 +29,9 @@ func (r *ResourceList) Append(val *resource.Info) { // Visit implements resource.Visitor. The visitor stops if fn returns an error. func (r ResourceList) Visit(fn resource.VisitorFunc) error { for _, i := range r { + if i.Name == "" { + continue + } if err := fn(i, nil); err != nil { return err } From b357bcae8640508f110b7e63a8dfacd865c27b6e Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Mon, 24 Nov 2025 01:16:13 +0530 Subject: [PATCH 2/8] update: business logic respected for skipping object missing name Signed-off-by: Mujib Ahasan --- pkg/action/validate.go | 8 ++++++++ pkg/kube/resource.go | 3 --- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 761ccba47..d89ae5038 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -46,6 +46,10 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) { return err } + if info.Name == "" { + return nil + } + helper := resource.NewHelper(info.Client, info.Mapping) _, err = helper.Get(info.Namespace, info.Name) if err != nil { @@ -70,6 +74,10 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN return err } + if info.Name == "" { + return nil + } + helper := resource.NewHelper(info.Client, info.Mapping) existing, err := helper.Get(info.Namespace, info.Name) if err != nil { diff --git a/pkg/kube/resource.go b/pkg/kube/resource.go index 98d2cb7f9..d88b171f0 100644 --- a/pkg/kube/resource.go +++ b/pkg/kube/resource.go @@ -29,9 +29,6 @@ func (r *ResourceList) Append(val *resource.Info) { // Visit implements resource.Visitor. The visitor stops if fn returns an error. func (r ResourceList) Visit(fn resource.VisitorFunc) error { for _, i := range r { - if i.Name == "" { - continue - } if err := fn(i, nil); err != nil { return err } From 6769fb6fb6704e29fe1215c802ecf0ea62b39715 Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Thu, 27 Nov 2025 00:11:20 +0530 Subject: [PATCH 3/8] generateName is also considered in logic Signed-off-by: Mujib Ahasan --- pkg/action/validate.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index d89ae5038..555858fcf 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -45,9 +45,11 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) { if err != nil { return err } - - if info.Name == "" { + accessor, _ := meta.Accessor(info.Object) + if info.Name == "" && accessor.GetGenerateName() != "" { return nil + } else if info.Name != "" && accessor.GetGenerateName() != "" { + return fmt.Errorf("metadata.name and metadata.generateName cannot both be set") } helper := resource.NewHelper(info.Client, info.Mapping) @@ -74,8 +76,11 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN return err } - if info.Name == "" { + accessor, _ := meta.Accessor(info.Object) + if info.Name == "" && accessor.GetGenerateName() != "" { return nil + } else if info.Name != "" && accessor.GetGenerateName() != "" { + return fmt.Errorf("metadata.name and metadata.generateName cannot both be set") } helper := resource.NewHelper(info.Client, info.Mapping) From 522d2fe61508639cfe8f06a43235e7c3eaea3b9a Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Sun, 30 Nov 2025 22:46:57 +0530 Subject: [PATCH 4/8] update: helper function added for the business logic Signed-off-by: Mujib Ahasan --- pkg/action/validate.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 555858fcf..045a70955 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -45,11 +45,10 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) { if err != nil { return err } - accessor, _ := meta.Accessor(info.Object) - if info.Name == "" && accessor.GetGenerateName() != "" { - return nil - } else if info.Name != "" && accessor.GetGenerateName() != "" { - return fmt.Errorf("metadata.name and metadata.generateName cannot both be set") + + skip, err := validateNameAndgenerateName(info) + if skip { + return err } helper := resource.NewHelper(info.Client, info.Mapping) @@ -76,11 +75,9 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN return err } - accessor, _ := meta.Accessor(info.Object) - if info.Name == "" && accessor.GetGenerateName() != "" { - return nil - } else if info.Name != "" && accessor.GetGenerateName() != "" { - return fmt.Errorf("metadata.name and metadata.generateName cannot both be set") + skip, err := validateNameAndgenerateName(info) + if skip { + return err } helper := resource.NewHelper(info.Client, info.Mapping) @@ -212,3 +209,14 @@ func mergeStrStrMaps(current, desired map[string]string) map[string]string { maps.Copy(result, desired) return result } + +func validateNameAndgenerateName(info *resource.Info) (bool, error) { + accessor, _ := meta.Accessor(info.Object) + if info.Name == "" && accessor.GetGenerateName() != "" { + return true, nil + } else if info.Name != "" && accessor.GetGenerateName() != "" { + return true, fmt.Errorf("metadata.name and metadata.generateName cannot both be set") + } + + return false, nil +} From 115409976b5c3fd94c893eabde114e655c01c573 Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Sun, 30 Nov 2025 22:49:20 +0530 Subject: [PATCH 5/8] update: test coverage added for helper function validateNameAndGenerateName Signed-off-by: Mujib Ahasan --- pkg/action/validate_test.go | 61 ++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/pkg/action/validate_test.go b/pkg/action/validate_test.go index 3efecd6ff..32ee8d802 100644 --- a/pkg/action/validate_test.go +++ b/pkg/action/validate_test.go @@ -36,7 +36,7 @@ import ( "k8s.io/client-go/rest/fake" ) -func newDeploymentResource(name, namespace string) *resource.Info { +func newDeploymentResource(name, namespace, generateName string) *resource.Info { return &resource.Info{ Name: name, Mapping: &meta.RESTMapping{ @@ -45,8 +45,9 @@ func newDeploymentResource(name, namespace string) *resource.Info { }, Object: &appsv1.Deployment{ ObjectMeta: v1.ObjectMeta{ - Name: name, - Namespace: namespace, + Name: name, + Namespace: namespace, + GenerateName: generateName, }, }, } @@ -164,7 +165,7 @@ func TestExistingResourceConflict(t *testing.T) { } func TestCheckOwnership(t *testing.T) { - deployFoo := newDeploymentResource("foo", "ns-a") + deployFoo := newDeploymentResource("foo", "ns-a", "") // Verify that a resource that lacks labels/annotations is not owned err := checkOwnership(deployFoo.Object, "rel-a", "ns-a") @@ -211,8 +212,8 @@ func TestCheckOwnership(t *testing.T) { func TestSetMetadataVisitor(t *testing.T) { var ( err error - deployFoo = newDeploymentResource("foo", "ns-a") - deployBar = newDeploymentResource("bar", "ns-a-system") + deployFoo = newDeploymentResource("foo", "ns-a", "") + deployBar = newDeploymentResource("bar", "ns-a-system", "") resources = kube.ResourceList{deployFoo, deployBar} ) @@ -233,8 +234,54 @@ func TestSetMetadataVisitor(t *testing.T) { assert.NoError(t, err) // Add a new resource that is missing ownership metadata and verify error - resources.Append(newDeploymentResource("baz", "default")) + 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`) } + +func TestValidateNameAndGenerateName(t *testing.T) { + tests := []struct { + name string + info *resource.Info + wantSkip bool + wantErr bool + errContains string + }{ + { + name: "both name and generateName present", + info: newDeploymentResource("job-a", "foo", "job-a-"), + wantSkip: true, + wantErr: true, + errContains: "metadata.name and metadata.generateName cannot both be set", + }, + { + name: "only generateName present", + info: newDeploymentResource("", "foo", "job-a-"), + wantSkip: true, + wantErr: false, + }, + { + name: "only name present", + info: newDeploymentResource("job-a", "foo", ""), + wantSkip: false, + wantErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + + skip, err := validateNameAndgenerateName(tc.info) + + if tc.wantErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tc.wantSkip, skip) + }) + } +} From 12e8b715aa0732b613c3a9896fa6af29b3201536 Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Thu, 18 Dec 2025 00:14:08 +0530 Subject: [PATCH 6/8] fix: doc string added Signed-off-by: Mujib Ahasan --- pkg/action/validate.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 045a70955..8e58af7f0 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -46,8 +46,8 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) { return err } - skip, err := validateNameAndgenerateName(info) - if skip { + isGenerateName, err := validateNameAndgenerateName(info) + if isGenerateName { return err } @@ -75,8 +75,8 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN return err } - skip, err := validateNameAndgenerateName(info) - if skip { + isGenerateName, err := validateNameAndgenerateName(info) + if isGenerateName { return err } @@ -210,11 +210,20 @@ func mergeStrStrMaps(current, desired map[string]string) map[string]string { return result } +// validateNameAndgenerateName validates that an object only has either `Name` or `GenerateName` set (and not both) +// If `GenerateName` is set, true is returned +// If an invalid combination of `Name` and `GenerateName` are set, an error is returned func validateNameAndgenerateName(info *resource.Info) (bool, error) { - accessor, _ := meta.Accessor(info.Object) + accessor, err := meta.Accessor(info.Object) + if err != nil { + return true, err + } + if info.Name == "" && accessor.GetGenerateName() != "" { return true, nil - } else if info.Name != "" && accessor.GetGenerateName() != "" { + } + + if info.Name != "" && accessor.GetGenerateName() != "" { return true, fmt.Errorf("metadata.name and metadata.generateName cannot both be set") } From 94860626ce9c83a9227b5bce02a5c03a050816ac Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Sat, 20 Dec 2025 21:49:25 +0530 Subject: [PATCH 7/8] fix: error handled correctly Signed-off-by: Mujib Ahasan --- pkg/action/validate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 8e58af7f0..df06d0105 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -47,7 +47,7 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) { } isGenerateName, err := validateNameAndgenerateName(info) - if isGenerateName { + if isGenerateName || err != nil { return err } @@ -76,7 +76,7 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN } isGenerateName, err := validateNameAndgenerateName(info) - if isGenerateName { + if isGenerateName || err != nil { return err } @@ -216,7 +216,7 @@ func mergeStrStrMaps(current, desired map[string]string) map[string]string { func validateNameAndgenerateName(info *resource.Info) (bool, error) { accessor, err := meta.Accessor(info.Object) if err != nil { - return true, err + return false, err } if info.Name == "" && accessor.GetGenerateName() != "" { From 170911459bc4f2b5efea7e549e09bd45c7578cc4 Mon Sep 17 00:00:00 2001 From: Mujib Ahasan Date: Tue, 23 Dec 2025 03:19:41 +0530 Subject: [PATCH 8/8] fix: casing issue fixed Signed-off-by: Mujib Ahasan --- pkg/action/validate.go | 8 ++++---- pkg/action/validate_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/action/validate.go b/pkg/action/validate.go index df06d0105..5894c229a 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -46,7 +46,7 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) { return err } - isGenerateName, err := validateNameAndgenerateName(info) + isGenerateName, err := validateNameAndGenerateName(info) if isGenerateName || err != nil { return err } @@ -75,7 +75,7 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN return err } - isGenerateName, err := validateNameAndgenerateName(info) + isGenerateName, err := validateNameAndGenerateName(info) if isGenerateName || err != nil { return err } @@ -210,10 +210,10 @@ func mergeStrStrMaps(current, desired map[string]string) map[string]string { return result } -// validateNameAndgenerateName validates that an object only has either `Name` or `GenerateName` set (and not both) +// validateNameAndGenerateName validates that an object only has either `Name` or `GenerateName` set (and not both) // If `GenerateName` is set, true is returned // If an invalid combination of `Name` and `GenerateName` are set, an error is returned -func validateNameAndgenerateName(info *resource.Info) (bool, error) { +func validateNameAndGenerateName(info *resource.Info) (bool, error) { accessor, err := meta.Accessor(info.Object) if err != nil { return false, err diff --git a/pkg/action/validate_test.go b/pkg/action/validate_test.go index 32ee8d802..6bf4ec533 100644 --- a/pkg/action/validate_test.go +++ b/pkg/action/validate_test.go @@ -272,7 +272,7 @@ func TestValidateNameAndGenerateName(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - skip, err := validateNameAndgenerateName(tc.info) + skip, err := validateNameAndGenerateName(tc.info) if tc.wantErr { assert.Error(t, err)