diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 1bef5a742..8aab7db69 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -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 { diff --git a/pkg/action/validate_test.go b/pkg/action/validate_test.go index 879a5fa4f..0a41354c5 100644 --- a/pkg/action/validate_test.go +++ b/pkg/action/validate_test.go @@ -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")