fix: resolve merge conflict with upstream validateNameAndGenerateName

Adopt upstream's shared validateNameAndGenerateName helper which
provides the same generateName skip logic in a cleaner, reusable way.
pull/31855/head
harshil562 8 months ago
commit c18719e72a

@ -46,15 +46,9 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) {
return err return err
} }
// Resources that use generateName instead of name don't have a isGenerateName, err := validateNameAndGenerateName(info)
// fixed identity yet — the server assigns one at creation time. if isGenerateName || err != nil {
// They cannot already exist, so skip the lookup. return err
if info.Name == "" {
generateName, _ := accessor.GenerateName(info.Object)
if generateName != "" {
return nil
}
return fmt.Errorf("resource %s is missing both metadata.name and metadata.generateName", resourceString(info))
} }
helper := resource.NewHelper(info.Client, info.Mapping) helper := resource.NewHelper(info.Client, info.Mapping)
@ -82,15 +76,9 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN
return err return err
} }
// Resources that use generateName instead of name don't have a isGenerateName, err := validateNameAndGenerateName(info)
// fixed identity yet — the server assigns one at creation time. if isGenerateName || err != nil {
// They cannot already exist, so skip the conflict check. return err
if info.Name == "" {
generateName, _ := accessor.GenerateName(info.Object)
if generateName != "" {
return nil
}
return fmt.Errorf("resource %s is missing both metadata.name and metadata.generateName", resourceString(info))
} }
helper := resource.NewHelper(info.Client, info.Mapping) helper := resource.NewHelper(info.Client, info.Mapping)
@ -223,3 +211,23 @@ func mergeStrStrMaps(current, desired map[string]string) map[string]string {
maps.Copy(result, desired) maps.Copy(result, desired)
return result 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, err := meta.Accessor(info.Object)
if err != nil {
return false, err
}
if info.Name == "" && accessor.GetGenerateName() != "" {
return true, nil
}
if info.Name != "" && accessor.GetGenerateName() != "" {
return true, fmt.Errorf("metadata.name and metadata.generateName cannot both be set")
}
return false, nil
}

@ -36,7 +36,7 @@ import (
"k8s.io/client-go/rest/fake" "k8s.io/client-go/rest/fake"
) )
func newDeploymentResource(name, namespace string) *resource.Info { func newDeploymentResource(name, namespace, generateName string) *resource.Info {
return &resource.Info{ return &resource.Info{
Name: name, Name: name,
Mapping: &meta.RESTMapping{ Mapping: &meta.RESTMapping{
@ -45,8 +45,9 @@ func newDeploymentResource(name, namespace string) *resource.Info {
}, },
Object: &appsv1.Deployment{ Object: &appsv1.Deployment{
ObjectMeta: v1.ObjectMeta{ ObjectMeta: v1.ObjectMeta{
Name: name, Name: name,
Namespace: namespace, Namespace: namespace,
GenerateName: generateName,
}, },
}, },
} }
@ -268,7 +269,7 @@ func TestExistingResourceConflictRejectsUnnamedResource(t *testing.T) {
} }
func TestCheckOwnership(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 // Verify that a resource that lacks labels/annotations is not owned
err := checkOwnership(deployFoo.Object, "rel-a", "ns-a") err := checkOwnership(deployFoo.Object, "rel-a", "ns-a")
@ -315,8 +316,8 @@ func TestCheckOwnership(t *testing.T) {
func TestSetMetadataVisitor(t *testing.T) { func TestSetMetadataVisitor(t *testing.T) {
var ( var (
err error err error
deployFoo = newDeploymentResource("foo", "ns-a") deployFoo = newDeploymentResource("foo", "ns-a", "")
deployBar = newDeploymentResource("bar", "ns-a-system") deployBar = newDeploymentResource("bar", "ns-a-system", "")
resources = kube.ResourceList{deployFoo, deployBar} resources = kube.ResourceList{deployFoo, deployBar}
) )
@ -337,8 +338,54 @@ func TestSetMetadataVisitor(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
// Add a new resource that is missing ownership metadata and verify error // 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)) err = resources.Visit(setMetadataVisitor("rel-b", "ns-a", false))
assert.Error(t, err) assert.Error(t, err)
assert.Contains(t, err.Error(), `Deployment "baz" in namespace "" cannot be owned`) 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)
})
}
}

Loading…
Cancel
Save