fix: skip generateName resources in dry-run server validation

When running 'helm install --dry-run=server' or 'helm template --dry-run=server'
with resources that use metadata.generateName instead of metadata.name,
Helm would fail with:

  resource name may not be empty

This happened because requireAdoption() and existingResourceConflict()
attempted to Get resources by name, which is empty for generateName
resources — the server assigns the actual name at creation time.

Fix: skip the server-side existence check for resources with empty names,
since they use generateName and cannot possibly already exist.

Fixes #31509

Signed-off-by: Harshil Garg <harshil562@users.noreply.github.com>
pull/31855/head
Harshil Garg 4 weeks ago
parent ee018608f6
commit 37b0c69758

@ -46,6 +46,17 @@ func requireAdoption(resources kube.ResourceList) (kube.ResourceList, error) {
return err
}
// Resources that use generateName instead of name don't have a
// fixed identity yet — the server assigns one at creation time.
// They cannot already exist, so skip the lookup.
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)
_, err = helper.Get(info.Namespace, info.Name)
if err != nil {
@ -71,6 +82,17 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN
return err
}
// Resources that use generateName instead of name don't have a
// fixed identity yet — the server assigns one at creation time.
// They cannot already exist, so skip the conflict check.
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)
existing, err := helper.Get(info.Namespace, info.Name)
if err != nil {

@ -120,6 +120,41 @@ func fakeClientWith(code int, gv schema.GroupVersion, body string) *fake.RESTCli
}
}
func newGenerateNameDeployment(generateName, namespace string) *resource.Info {
return &resource.Info{
Name: "", // Name is empty when generateName is used
Namespace: namespace,
Mapping: &meta.RESTMapping{
Resource: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployment"},
GroupVersionKind: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
Scope: meta.RESTScopeNamespace,
},
Object: &appsv1.Deployment{
ObjectMeta: v1.ObjectMeta{
GenerateName: generateName,
Namespace: namespace,
},
},
}
}
func newUnnamedDeployment(namespace string) *resource.Info {
return &resource.Info{
Name: "", // Neither name nor generateName
Namespace: namespace,
Mapping: &meta.RESTMapping{
Resource: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployment"},
GroupVersionKind: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
Scope: meta.RESTScopeNamespace,
},
Object: &appsv1.Deployment{
ObjectMeta: v1.ObjectMeta{
Namespace: namespace,
},
},
}
}
func TestRequireAdoption(t *testing.T) {
var (
missing = newMissingDeployment("missing", "ns-a")
@ -135,6 +170,35 @@ func TestRequireAdoption(t *testing.T) {
assert.NotSame(t, found[0], existing)
}
func TestRequireAdoptionSkipsGenerateName(t *testing.T) {
var (
missing = newMissingDeployment("missing", "ns-a")
existing = newDeploymentWithOwner("existing", "ns-a", nil, nil)
generateName = newGenerateNameDeployment("hello-world-", "ns-a")
resources = kube.ResourceList{missing, existing, generateName}
)
// Resources with generateName (empty name) should be skipped,
// not cause an error about empty resource name.
found, err := requireAdoption(resources)
assert.NoError(t, err)
assert.Len(t, found, 1)
assert.Equal(t, found[0], existing)
}
func TestRequireAdoptionRejectsUnnamedResource(t *testing.T) {
var (
missing = newMissingDeployment("missing", "ns-a")
unnamed = newUnnamedDeployment("ns-a")
resources = kube.ResourceList{missing, unnamed}
)
// A resource with neither name nor generateName should produce an error.
_, err := requireAdoption(resources)
assert.Error(t, err)
assert.Contains(t, err.Error(), "missing both metadata.name and metadata.generateName")
}
func TestExistingResourceConflict(t *testing.T) {
var (
releaseName = "rel-name"
@ -165,6 +229,44 @@ func TestExistingResourceConflict(t *testing.T) {
assert.Error(t, err)
}
func TestExistingResourceConflictSkipsGenerateName(t *testing.T) {
var (
releaseName = "rel-name"
releaseNamespace = "rel-namespace"
labels = map[string]string{
appManagedByLabel: appManagedByHelm,
}
annotations = map[string]string{
helmReleaseNameAnnotation: releaseName,
helmReleaseNamespaceAnnotation: releaseNamespace,
}
missing = newMissingDeployment("missing", "ns-a")
existing = newDeploymentWithOwner("existing", "ns-a", labels, annotations)
generateName = newGenerateNameDeployment("hello-world-", "ns-a")
resources = kube.ResourceList{missing, existing, generateName}
)
// Resources with generateName (empty name) should be skipped,
// not cause an error about empty resource name.
found, err := existingResourceConflict(resources, releaseName, releaseNamespace)
assert.NoError(t, err)
assert.Len(t, found, 1)
assert.Equal(t, found[0], existing)
}
func TestExistingResourceConflictRejectsUnnamedResource(t *testing.T) {
var (
missing = newMissingDeployment("missing", "ns-a")
unnamed = newUnnamedDeployment("ns-a")
resources = kube.ResourceList{missing, unnamed}
)
// A resource with neither name nor generateName should produce an error.
_, err := existingResourceConflict(resources, "rel-name", "rel-namespace")
assert.Error(t, err)
assert.Contains(t, err.Error(), "missing both metadata.name and metadata.generateName")
}
func TestCheckOwnership(t *testing.T) {
deployFoo := newDeploymentResource("foo", "ns-a")

Loading…
Cancel
Save