diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 20596cdab4c..c4870d27a23 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/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)) diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index 656fe363db3..6e009aa4d82 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/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: "", }, { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index dd6a8224e5e..cddd43344db 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/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) { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 8f1201c28f8..6dc143a7b06 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/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 string - err error - ) - if includeName { - namespace, name, err = scope.Namer.Name(req) - } else { - namespace, err = scope.Namer.Namespace(req) - } + namespace, name, err := scope.Namer.Name(req) if err != nil { - scope.err(err, w, req) - return + if includeName { + // 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) diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go index e5ae7562daf..dd70a4eb780 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/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(),