diff --git a/staging/src/k8s.io/client-go/rest/request.go b/staging/src/k8s.io/client-go/rest/request.go index 491f8bbd1e3..556061de0f6 100644 --- a/staging/src/k8s.io/client-go/rest/request.go +++ b/staging/src/k8s.io/client-go/rest/request.go @@ -691,6 +691,33 @@ func (r *Request) Stream() (io.ReadCloser, error) { } } +// requestPreflightCheck looks for common programmer errors on Request. +// +// We tackle here two programmer mistakes. The first one is to try to create +// something(POST) using an empty string as namespace with namespaceSet as +// true. If namespaceSet is true then namespace should also be defined. The +// second mistake is, when under the same circumstances, the programmer tries +// to GET, PUT or DELETE a named resource(resourceName != ""), again, if +// namespaceSet is true then namespace must not be empty. +func (r *Request) requestPreflightCheck() error { + if !r.namespaceSet { + return nil + } + if len(r.namespace) > 0 { + return nil + } + + switch r.verb { + case "POST": + return fmt.Errorf("an empty namespace may not be set during creation") + case "GET", "PUT", "DELETE": + if len(r.resourceName) > 0 { + return fmt.Errorf("an empty namespace may not be set when a resource name is provided") + } + } + return nil +} + // request connects to the server and invokes the provided function when a server response is // received. It handles retry behavior and up front validation of requests. It will invoke // fn at most once. It will return an error if a problem occurred prior to connecting to the @@ -707,12 +734,8 @@ func (r *Request) request(fn func(*http.Request, *http.Response)) error { return r.err } - // TODO: added to catch programmer errors (invoking operations with an object with an empty namespace) - if (r.verb == "GET" || r.verb == "PUT" || r.verb == "DELETE") && r.namespaceSet && len(r.resourceName) > 0 && len(r.namespace) == 0 { - return fmt.Errorf("an empty namespace may not be set when a resource name is provided") - } - if (r.verb == "POST") && r.namespaceSet && len(r.namespace) == 0 { - return fmt.Errorf("an empty namespace may not be set during creation") + if err := r.requestPreflightCheck(); err != nil { + return err } client := r.client @@ -815,8 +838,6 @@ func (r *Request) request(fn func(*http.Request, *http.Response)) error { // processing. // // Error type: -// * If the request can't be constructed, or an error happened earlier while building its -// arguments: *RequestConstructionError // * If the server responds with a status: *errors.StatusError or *errors.UnexpectedObjectError // * http.Client.Do errors are returned directly. func (r *Request) Do() Result { diff --git a/staging/src/k8s.io/client-go/rest/request_test.go b/staging/src/k8s.io/client-go/rest/request_test.go index d88a0a35288..4c2481dc46b 100644 --- a/staging/src/k8s.io/client-go/rest/request_test.go +++ b/staging/src/k8s.io/client-go/rest/request_test.go @@ -1994,3 +1994,66 @@ func defaultResourcePathWithPrefix(prefix, resource, namespace, name string) str } return path } + +func TestRequestPreflightCheck(t *testing.T) { + for _, tt := range []struct { + name string + verbs []string + namespace string + resourceName string + namespaceSet bool + expectsErr bool + }{ + { + name: "no namespace set", + verbs: []string{"GET", "PUT", "DELETE", "POST"}, + namespaceSet: false, + expectsErr: false, + }, + { + name: "empty resource name and namespace", + verbs: []string{"GET", "PUT", "DELETE"}, + namespaceSet: true, + expectsErr: false, + }, + { + name: "resource name with empty namespace", + verbs: []string{"GET", "PUT", "DELETE"}, + namespaceSet: true, + resourceName: "ResourceName", + expectsErr: true, + }, + { + name: "post empty resource name and namespace", + verbs: []string{"POST"}, + namespaceSet: true, + expectsErr: true, + }, + { + name: "working requests", + verbs: []string{"GET", "PUT", "DELETE", "POST"}, + namespaceSet: true, + resourceName: "ResourceName", + namespace: "Namespace", + expectsErr: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + for _, verb := range tt.verbs { + r := &Request{ + verb: verb, + namespace: tt.namespace, + resourceName: tt.resourceName, + namespaceSet: tt.namespaceSet, + } + + err := r.requestPreflightCheck() + hasErr := err != nil + if hasErr == tt.expectsErr { + return + } + t.Errorf("%s: expects error: %v, has error: %v", verb, tt.expectsErr, hasErr) + } + }) + } +}