Merge pull request #53185 from dixudx/fix_admission_attributes_populate_name

populate object name for admission attributes when CREATE

pkg/registry/core/pod/storage/BUILD

@@ -64,6 +64,7 @@ go_library(
         "//pkg/registry/core/pod/rest:go_default_library",
         "//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",

pkg/registry/core/pod/storage/eviction.go

@@ -67,7 +67,7 @@ type EvictionREST struct {
 	podDisruptionBudgetClient policyclient.PodDisruptionBudgetsGetter
 }
 
-var _ = rest.Creater(&EvictionREST{})
+var _ = rest.NamedCreater(&EvictionREST{})
 var _ = rest.GroupVersionKindProvider(&EvictionREST{})
 
 // GroupVersionKind specifies a particular GroupVersionKind to discovery
@@ -101,8 +101,15 @@ func propagateDryRun(eviction *policy.Eviction, options *metav1.CreateOptions) (
 }
 
 // Create attempts to create a new eviction.  That is, it tries to evict a pod.
-func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
+func (r *EvictionREST) Create(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	eviction := obj.(*policy.Eviction)
+	eviction, ok := obj.(*policy.Eviction)
+	if !ok {
+		return nil, errors.NewBadRequest(fmt.Sprintf("not a Eviction object: %T", obj))
+	}
+
+	if name != eviction.Name {
+		return nil, errors.NewBadRequest("name in URL does not match name in Eviction object")
+	}
 
 	deletionOptions, err := propagateDryRun(eviction, options)
 	if err != nil {

pkg/registry/core/pod/storage/eviction_test.go

@@ -42,9 +42,10 @@ func TestEviction(t *testing.T) {
 	testcases := []struct {
 		name     string
 		pdbs     []runtime.Object
-		pod      *api.Pod
 		eviction *policy.Eviction
 
+		badNameInURL bool
+
 		expectError   bool
 		expectDeleted bool
 	}{
@@ -55,12 +56,6 @@ func TestEviction(t *testing.T) {
 				Spec:       policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
 				Status:     policyv1beta1.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 0},
 			}},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 			eviction:    &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
 			expectError: true,
 		},
@@ -71,12 +66,6 @@ func TestEviction(t *testing.T) {
 				Spec:       policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
 				Status:     policyv1beta1.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 1},
 			}},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
 			expectDeleted: true,
 		},
@@ -87,15 +76,20 @@ func TestEviction(t *testing.T) {
 				Spec:       policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"b": "true"}}},
 				Status:     policyv1beta1.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 0},
 			}},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
 			expectDeleted: true,
 		},
+		{
+			name: "matching pdbs with disruptions allowed but bad name in Url",
+			pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
+				Spec:       policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
+				Status:     policyv1beta1.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 1},
+			}},
+			badNameInURL: true,
+			eviction:     &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
+			expectError:  true,
+		},
 	}
 
 	for _, tc := range testcases {
@@ -104,43 +98,58 @@ func TestEviction(t *testing.T) {
 			storage, _, _, server := newStorage(t)
 			defer server.Terminate(t)
 			defer storage.Store.DestroyFunc()
-			if tc.pod != nil {
+
-				if _, err := storage.Create(testContext, tc.pod, nil, &metav1.CreateOptions{}); err != nil {
+			pod := validNewPod()
-					t.Error(err)
+			pod.Labels = map[string]string{"a": "true"}
-				}
+			pod.Spec.NodeName = "foo"
+
+			if _, err := storage.Create(testContext, pod, nil, &metav1.CreateOptions{}); err != nil {
+				t.Error(err)
 			}
 
 			client := fake.NewSimpleClientset(tc.pdbs...)
 			evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1())
-			_, err := evictionRest.Create(testContext, tc.eviction, nil, &metav1.CreateOptions{})
+
+			name := pod.Name
+			if tc.badNameInURL {
+				name += "bad-name"
+			}
+			_, err := evictionRest.Create(testContext, name, tc.eviction, nil, &metav1.CreateOptions{})
 			if (err != nil) != tc.expectError {
 				t.Errorf("expected error=%v, got %v", tc.expectError, err)
 				return
 			}
+			if tc.badNameInURL {
+				if err == nil {
+					t.Error("expected error here, but got nil")
+					return
+				}
+				if err.Error() != "name in URL does not match name in Eviction object" {
+					t.Errorf("got unexpected error: %v", err)
+				}
+			}
 			if tc.expectError {
 				return
 			}
 
-			if tc.pod != nil {
+			existingPod, err := storage.Get(testContext, pod.Name, &metav1.GetOptions{})
-				existingPod, err := storage.Get(testContext, tc.pod.Name, &metav1.GetOptions{})
+			if tc.expectDeleted {
-				if tc.expectDeleted {
+				if !apierrors.IsNotFound(err) {
-					if !apierrors.IsNotFound(err) {
+					t.Errorf("expected to be deleted, lookup returned %#v", existingPod)
-						t.Errorf("expected to be deleted, lookup returned %#v", existingPod)
-					}
-					return
-				} else if apierrors.IsNotFound(err) {
-					t.Errorf("expected graceful deletion, got %v", err)
-					return
 				}
+				return
+			} else if apierrors.IsNotFound(err) {
+				t.Errorf("expected graceful deletion, got %v", err)
+				return
+			}
 
-				if err != nil {
+			if err != nil {
-					t.Errorf("%#v", err)
+				t.Errorf("%#v", err)
-					return
+				return
-				}
+			}
 
-				if existingPod.(*api.Pod).DeletionTimestamp == nil {
+			if existingPod.(*api.Pod).DeletionTimestamp == nil {
-					t.Errorf("expected gracefully deleted pod with deletionTimestamp set, got %#v", existingPod)
+				t.Errorf("expected gracefully deleted pod with deletionTimestamp set, got %#v", existingPod)
-				}
 			}
 		})
 	}
@@ -186,52 +195,27 @@ func TestEvictionDryRun(t *testing.T) {
 		name            string
 		evictionOptions *metav1.DeleteOptions
 		requestOptions  *metav1.CreateOptions
-		pod             *api.Pod
 		pdbs            []runtime.Object
 	}{
 		{
 			name:            "just request-options",
 			requestOptions:  &metav1.CreateOptions{DryRun: []string{"All"}},
 			evictionOptions: &metav1.DeleteOptions{},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 		},
 		{
 			name:            "just eviction-options",
 			requestOptions:  &metav1.CreateOptions{},
 			evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 		},
 		{
 			name:            "both options",
 			evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}},
 			requestOptions:  &metav1.CreateOptions{DryRun: []string{"All"}},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 		},
 		{
 			name:            "with pdbs",
 			evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}},
 			requestOptions:  &metav1.CreateOptions{DryRun: []string{"All"}},
-			pod: func() *api.Pod {
-				pod := validNewPod()
-				pod.Labels = map[string]string{"a": "true"}
-				pod.Spec.NodeName = "foo"
-				return pod
-			}(),
 			pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{
 				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
 				Spec:       policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
@@ -257,7 +241,7 @@ func TestEvictionDryRun(t *testing.T) {
 			client := fake.NewSimpleClientset(tc.pdbs...)
 			evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1())
 			eviction := &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: tc.evictionOptions}
-			_, err := evictionRest.Create(testContext, eviction, nil, tc.requestOptions)
+			_, err := evictionRest.Create(testContext, pod.Name, eviction, nil, tc.requestOptions)
 			if err != nil {
 				t.Fatalf("Failed to run eviction: %v", err)
 			}

pkg/registry/core/pod/storage/storage.go

@@ -23,6 +23,7 @@ import (
 	"net/url"
 
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
@@ -49,6 +50,7 @@ import (
 type PodStorage struct {
 	Pod                 *REST
 	Binding             *BindingREST
+	LegacyBinding       *LegacyBindingREST
 	Eviction            *EvictionREST
 	Status              *StatusREST
 	EphemeralContainers *EphemeralContainersREST
@@ -95,9 +97,11 @@ func NewStorage(optsGetter generic.RESTOptionsGetter, k client.ConnectionInfoGet
 	ephemeralContainersStore := *store
 	ephemeralContainersStore.UpdateStrategy = pod.EphemeralContainersStrategy
 
+	bindingREST := &BindingREST{store: store}
 	return PodStorage{
 		Pod:                 &REST{store, proxyTransport},
 		Binding:             &BindingREST{store: store},
+		LegacyBinding:       &LegacyBindingREST{bindingREST},
 		Eviction:            newEvictionStorage(store, podDisruptionBudgetClient),
 		Status:              &StatusREST{store: &statusStore},
 		EphemeralContainers: &EphemeralContainersREST{store: &ephemeralContainersStore},
@@ -148,11 +152,18 @@ func (r *BindingREST) New() runtime.Object {
 	return &api.Binding{}
 }
 
-var _ = rest.Creater(&BindingREST{})
+var _ = rest.NamedCreater(&BindingREST{})
 
 // Create ensures a pod is bound to a specific host.
-func (r *BindingREST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (out runtime.Object, err error) {
+func (r *BindingREST) Create(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (out runtime.Object, err error) {
-	binding := obj.(*api.Binding)
+	binding, ok := obj.(*api.Binding)
+	if !ok {
+		return nil, errors.NewBadRequest(fmt.Sprintf("not a Binding object: %#v", obj))
+	}
+
+	if name != binding.Name {
+		return nil, errors.NewBadRequest("name in URL does not match name in Binding object")
+	}
 
 	// TODO: move me to a binding strategy
 	if errs := validation.ValidatePodBinding(binding); len(errs) != 0 {
@@ -218,6 +229,32 @@ func (r *BindingREST) assignPod(ctx context.Context, podID string, machine strin
 	return
 }
 
+var _ = rest.Creater(&LegacyBindingREST{})
+
+// LegacyBindingREST implements the REST endpoint for binding pods to nodes when etcd is in use.
+type LegacyBindingREST struct {
+	bindingRest *BindingREST
+}
+
+// NamespaceScoped fulfill rest.Scoper
+func (r *LegacyBindingREST) NamespaceScoped() bool {
+	return r.bindingRest.NamespaceScoped()
+}
+
+// New creates a new binding resource
+func (r *LegacyBindingREST) New() runtime.Object {
+	return r.bindingRest.New()
+}
+
+// Create ensures a pod is bound to a specific host.
+func (r *LegacyBindingREST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (out runtime.Object, err error) {
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return nil, errors.NewBadRequest(fmt.Sprintf("not a Binding object: %T", obj))
+	}
+	return r.bindingRest.Create(ctx, metadata.GetName(), obj, createValidation, options)
+}
+
 // StatusREST implements the REST endpoint for changing the status of a pod.
 type StatusREST struct {
 	store *genericregistry.Store

pkg/registry/core/pod/storage/storage_test.go

@@ -587,7 +587,7 @@ func TestEtcdCreate(t *testing.T) {
 	}
 
 	// Suddenly, a wild scheduler appears:
-	_, err = bindingStorage.Create(ctx, &api.Binding{
+	_, err = bindingStorage.Create(ctx, "foo", &api.Binding{
 		ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
 		Target:     api.ObjectReference{Name: "machine"},
 	}, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
@@ -613,7 +613,7 @@ func TestEtcdCreateBindingNoPod(t *testing.T) {
 	// - Create (apiserver)
 	// - Schedule (scheduler)
 	// - Delete (apiserver)
-	_, err := bindingStorage.Create(ctx, &api.Binding{
+	_, err := bindingStorage.Create(ctx, "foo", &api.Binding{
 		ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
 		Target:     api.ObjectReference{Name: "machine"},
 	}, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
@@ -657,7 +657,7 @@ func TestEtcdCreateWithContainersNotFound(t *testing.T) {
 	}
 
 	// Suddenly, a wild scheduler appears:
-	_, err = bindingStorage.Create(ctx, &api.Binding{
+	_, err = bindingStorage.Create(ctx, "foo", &api.Binding{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:   metav1.NamespaceDefault,
 			Name:        "foo",
@@ -700,12 +700,12 @@ func TestEtcdCreateWithConflict(t *testing.T) {
 		},
 		Target: api.ObjectReference{Name: "machine"},
 	}
-	_, err = bindingStorage.Create(ctx, &binding, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
+	_, err = bindingStorage.Create(ctx, binding.Name, &binding, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 
-	_, err = bindingStorage.Create(ctx, &binding, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
+	_, err = bindingStorage.Create(ctx, binding.Name, &binding, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
 	if err == nil || !errors.IsConflict(err) {
 		t.Fatalf("expected resource conflict error, not: %v", err)
 	}
@@ -722,7 +722,7 @@ func TestEtcdCreateWithExistingContainers(t *testing.T) {
 	}
 
 	// Suddenly, a wild scheduler appears:
-	_, err = bindingStorage.Create(ctx, &api.Binding{
+	_, err = bindingStorage.Create(ctx, "foo", &api.Binding{
 		ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
 		Target:     api.ObjectReference{Name: "machine"},
 	}, rest.ValidateAllObjectFunc, &metav1.CreateOptions{})
@@ -740,8 +740,9 @@ func TestEtcdCreateBinding(t *testing.T) {
 	ctx := genericapirequest.NewDefaultContext()
 
 	testCases := map[string]struct {
-		binding api.Binding
+		binding      api.Binding
-		errOK   func(error) bool
+		badNameInURL bool
+		errOK        func(error) bool
 	}{
 		"noName": {
 			binding: api.Binding{
@@ -750,6 +751,14 @@ func TestEtcdCreateBinding(t *testing.T) {
 			},
 			errOK: func(err error) bool { return err != nil },
 		},
+		"badNameInURL": {
+			binding: api.Binding{
+				ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
+				Target:     api.ObjectReference{},
+			},
+			badNameInURL: true,
+			errOK:        func(err error) bool { return err != nil },
+		},
 		"badKind": {
 			binding: api.Binding{
 				ObjectMeta: metav1.ObjectMeta{Namespace: metav1.NamespaceDefault, Name: "foo"},
@@ -778,7 +787,11 @@ func TestEtcdCreateBinding(t *testing.T) {
 		if _, err := storage.Create(ctx, validNewPod(), rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil {
 			t.Fatalf("%s: unexpected error: %v", k, err)
 		}
-		if _, err := bindingStorage.Create(ctx, &test.binding, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); !test.errOK(err) {
+		name := test.binding.Name
+		if test.badNameInURL {
+			name += "badNameInURL"
+		}
+		if _, err := bindingStorage.Create(ctx, name, &test.binding, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); !test.errOK(err) {
 			t.Errorf("%s: unexpected error: %v", k, err)
 		} else if err == nil {
 			// If bind succeeded, verify Host field in pod's Spec.

pkg/registry/core/rest/storage_core.go

@@ -248,7 +248,7 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi
 		"pods/portforward": podStorage.PortForward,
 		"pods/proxy":       podStorage.Proxy,
 		"pods/binding":     podStorage.Binding,
-		"bindings":         podStorage.Binding,
+		"bindings":         podStorage.LegacyBinding,
 
 		"podTemplates": podTemplateStorage,
 

plugin/pkg/admission/noderestriction/admission.go

@@ -361,11 +361,6 @@ func (p *Plugin) admitNode(nodeName string, a admission.Attributes) error {
 		if forbiddenUpdateLabels := p.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 {
 			klog.Warningf("node %q added disallowed labels on node creation: %s", nodeName, strings.Join(forbiddenUpdateLabels.List(), ", "))
 		}
-
-		// On create, get name from new object if unset in admission
-		if len(requestedName) == 0 {
-			requestedName = node.Name
-		}
 	}
 	if requestedName != nodeName {
 		return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to modify node %q", nodeName, requestedName))

plugin/pkg/admission/noderestriction/admission_test.go

@@ -826,19 +826,19 @@ func Test_nodePlugin_Admit(t *testing.T) {
 		{
 			name:       "allow create of my node pulling name from object",
 			podsGetter: noExistingPods,
-			attributes: admission.NewAttributesRecord(mynodeObj, nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
+			attributes: admission.NewAttributesRecord(mynodeObj, nil, nodeKind, mynodeObj.Namespace, "mynode", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
 			err:        "",
 		},
 		{
 			name:       "allow create of my node with taints",
 			podsGetter: noExistingPods,
-			attributes: admission.NewAttributesRecord(mynodeObjTaintA, nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
+			attributes: admission.NewAttributesRecord(mynodeObjTaintA, nil, nodeKind, mynodeObj.Namespace, "mynode", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
 			err:        "",
 		},
 		{
 			name:       "allow create of my node with labels",
 			podsGetter: noExistingPods,
-			attributes: admission.NewAttributesRecord(setAllowedCreateLabels(mynodeObj, ""), nil, nodeKind, mynodeObj.Namespace, "", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
+			attributes: admission.NewAttributesRecord(setAllowedCreateLabels(mynodeObj, ""), nil, nodeKind, mynodeObj.Namespace, "mynode", nodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
 			err:        "",
 		},
 		{

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

@@ -3665,7 +3665,7 @@ func TestParentResourceIsRequired(t *testing.T) {
 	}
 }
 
-func TestCreateWithName(t *testing.T) {
+func TestNamedCreaterWithName(t *testing.T) {
 	pathName := "helloworld"
 	storage := &NamedCreaterRESTStorage{SimpleRESTStorage: &SimpleRESTStorage{}}
 	handler := handle(map[string]rest.Storage{
@@ -3697,6 +3697,145 @@ func TestCreateWithName(t *testing.T) {
 	}
 }
 
+func TestNamedCreaterWithoutName(t *testing.T) {
+	storage := &NamedCreaterRESTStorage{
+		SimpleRESTStorage: &SimpleRESTStorage{
+			injectedFunction: func(obj runtime.Object) (runtime.Object, error) {
+				time.Sleep(5 * time.Millisecond)
+				return obj, nil
+			},
+		},
+	}
+
+	selfLinker := &setTestSelfLinker{
+		t:           t,
+		name:        "bar",
+		namespace:   "default",
+		expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/foo",
+	}
+	handler := handleLinker(map[string]rest.Storage{"foo": storage}, selfLinker)
+	server := httptest.NewServer(handler)
+	defer server.Close()
+	client := http.Client{}
+
+	simple := &genericapitesting.Simple{
+		Other: "bar",
+	}
+	data, err := runtime.Encode(testCodec, simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	request, err := http.NewRequest("POST", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/foo", bytes.NewBuffer(data))
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	wg := sync.WaitGroup{}
+	wg.Add(1)
+	var response *http.Response
+	go func() {
+		response, err = client.Do(request)
+		wg.Done()
+	}()
+	wg.Wait()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	// empty name is not allowed for NamedCreater
+	if response.StatusCode != http.StatusBadRequest {
+		t.Errorf("Unexpected response %#v", response)
+	}
+}
+
+type namePopulatorAdmissionControl struct {
+	t            *testing.T
+	populateName string
+}
+
+func (npac *namePopulatorAdmissionControl) Validate(a admission.Attributes, o admission.ObjectInterfaces) (err error) {
+	if a.GetName() != npac.populateName {
+		npac.t.Errorf("Unexpected name: got %q, expected %q", a.GetName(), npac.populateName)
+	}
+	return nil
+}
+
+func (npac *namePopulatorAdmissionControl) Handles(operation admission.Operation) bool {
+	return true
+}
+
+func TestNamedCreaterWithGenerateName(t *testing.T) {
+	populateName := "bar"
+	storage := &SimpleRESTStorage{
+		injectedFunction: func(obj runtime.Object) (runtime.Object, error) {
+			time.Sleep(5 * time.Millisecond)
+			if metadata, err := meta.Accessor(obj); err == nil {
+				if len(metadata.GetName()) != 0 {
+					t.Errorf("Unexpected name %q", metadata.GetName())
+				}
+				metadata.SetName(populateName)
+			} else {
+				return nil, err
+			}
+			return obj, nil
+		},
+	}
+
+	selfLinker := &setTestSelfLinker{
+		t:           t,
+		namespace:   "default",
+		expectedSet: "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/foo",
+	}
+
+	ac := &namePopulatorAdmissionControl{
+		t:            t,
+		populateName: populateName,
+	}
+
+	handler := handleInternal(map[string]rest.Storage{"foo": storage}, ac, selfLinker, nil)
+	server := httptest.NewServer(handler)
+	defer server.Close()
+	client := http.Client{}
+
+	simple := &genericapitesting.Simple{
+		Other: "bar",
+	}
+	data, err := runtime.Encode(testCodec, simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	request, err := http.NewRequest("POST", server.URL+"/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/namespaces/default/foo", bytes.NewBuffer(data))
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	wg := sync.WaitGroup{}
+	wg.Add(1)
+	var response *http.Response
+	go func() {
+		response, err = client.Do(request)
+		wg.Done()
+	}()
+	wg.Wait()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if response.StatusCode != http.StatusCreated {
+		t.Errorf("Unexpected status: %d, Expected: %d, %#v", response.StatusCode, http.StatusOK, response)
+	}
+
+	var itemOut genericapitesting.Simple
+	body, err := extractBody(response, &itemOut)
+	if err != nil {
+		t.Errorf("unexpected error: %v %#v", err, response)
+	}
+
+	itemOut.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	simple.Name = populateName
+	if !reflect.DeepEqual(&itemOut, simple) {
+		t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simple, string(body))
+	}
+}
+
 func TestUpdateChecksDecode(t *testing.T) {
 	handler := handle(map[string]rest.Storage{"simple": &SimpleRESTStorage{}})
 	server := httptest.NewServer(handler)
@@ -3879,6 +4018,7 @@ func TestCreateYAML(t *testing.T) {
 		t.Errorf("Never set self link")
 	}
 }
+
 func TestCreateInNamespace(t *testing.T) {
 	storage := SimpleRESTStorage{
 		injectedFunction: func(obj runtime.Object) (runtime.Object, error) {

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go

@@ -57,18 +57,20 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 		// TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer)
 		timeout := parseTimeout(req.URL.Query().Get("timeout"))
 
-		var (
+		namespace, name, err := scope.Namer.Name(req)
-			namespace, name string
-			err             error
-		)
-		if includeName {
-			namespace, name, err = scope.Namer.Name(req)
-		} else {
-			namespace, err = scope.Namer.Namespace(req)
-		}
 		if err != nil {
-			scope.err(err, w, req)
+			if includeName {
-			return
+				// name was required, return
+				scope.err(err, w, req)
+				return
+			}
+
+			// otherwise attempt to look up the namespace
+			namespace, err = scope.Namer.Namespace(req)
+			if err != nil {
+				scope.err(err, w, req)
+				return
+			}
 		}
 
 		ctx, cancel := context.WithTimeout(req.Context(), timeout)
@@ -130,6 +132,11 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
 		audit.LogRequestObject(ae, obj, scope.Resource, scope.Subresource, scope.Serializer)
 
 		userInfo, _ := request.UserFrom(ctx)
+
+		// On create, get name from new object if unset
+		if len(name) == 0 {
+			_, name, _ = scope.Namer.ObjectName(obj)
+		}
 		admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, options, dryrun.IsDryRun(options.DryRun), userInfo)
 		if mutatingAdmission, ok := admit.(admission.MutationInterface); ok && mutatingAdmission.Handles(admission.Create) {
 			err = mutatingAdmission.Admit(ctx, admissionAttributes, scope)

staging/src/k8s.io/apiserver/pkg/registry/rest/create.go

@@ -163,12 +163,20 @@ func AdmissionToValidateObjectFunc(admit admission.Interface, staticAttributes a
 		return func(ctx context.Context, obj runtime.Object) error { return nil }
 	}
 	return func(ctx context.Context, obj runtime.Object) error {
+		name := staticAttributes.GetName()
+		// in case the generated name is populated
+		if len(name) == 0 {
+			if metadata, err := meta.Accessor(obj); err == nil {
+				name = metadata.GetName()
+			}
+		}
+
 		finalAttributes := admission.NewAttributesRecord(
 			obj,
 			staticAttributes.GetOldObject(),
 			staticAttributes.GetKind(),
 			staticAttributes.GetNamespace(),
-			staticAttributes.GetName(),
+			name,
 			staticAttributes.GetResource(),
 			staticAttributes.GetSubresource(),
 			staticAttributes.GetOperation(),

test/integration/apiserver/admissionwebhook/admission_test.go

@@ -295,9 +295,6 @@ func (h *holder) record(version string, phase string, converted bool, request *a
 	}
 
 	name := request.Name
-	if name == "" && request.Object.Object != nil {
-		name = request.Object.Object.(*unstructured.Unstructured).GetName()
-	}
 	if name != h.recordName {
 		if debug {
 			h.t.Log(name, "!=", h.recordName)