Fix forbidden message format

Before this change: # kubectl get pods --as=tom Error from server (Forbidden): pods "" is forbidden: User "tom" cannot list pods in the namespace "default". After this change: # kubectl get pods --as=tom Error from server (Forbidden): pods is forbidden: User "tom" cannot list pods in the namespace "default".
kubernetes · Aug 25, 2017 · 82a3dd7 · 82a3dd7
1 parent 657db0e
commit 82a3dd7
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 6 deletions.
diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go
@@ -128,6 +128,15 @@ func NewUnauthorized(reason string) *StatusError {
 
 // NewForbidden returns an error indicating the requested action was forbidden
 func NewForbidden(qualifiedResource schema.GroupResource, name string, err error) *StatusError {
+	var message string
+	res := qualifiedResource.String()
+	if res == "" {
+		message = fmt.Sprintf("forbidden: %v", err)
+	} else if name == "" {
+		message = fmt.Sprintf("%s is forbidden: %v", res, err)
+	} else {
+		message = fmt.Sprintf("%s %q is forbidden: %v", res, name, err)
+	}
 	return &StatusError{metav1.Status{
 		Status: metav1.StatusFailure,
 		Code:   http.StatusForbidden,
@@ -137,7 +146,7 @@ func NewForbidden(qualifiedResource schema.GroupResource, name string, err error
 			Kind:  qualifiedResource.Resource,
 			Name:  name,
 		},
-		Message: fmt.Sprintf("%s %q is forbidden: %v", qualifiedResource.String(), name, err),
+		Message: message,
 	}}
 }
 

diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/errors_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/responsewriters/errors_test.go
@@ -68,13 +68,15 @@ func TestForbidden(t *testing.T) {
 		reason      string
 		contentType string
 	}{
-		{`{"metadata":{},"status":"Failure","message":" \"\" is forbidden: User \"NAME\" cannot GET path \"/whatever\".","reason":"Forbidden","details":{},"code":403}
+		{`{"metadata":{},"status":"Failure","message":"forbidden: User \"NAME\" cannot GET path \"/whatever\".","reason":"Forbidden","details":{},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/whatever"}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":" \"\" is forbidden: User \"NAME\" cannot GET path \"/\u0026lt;script\u0026gt;\".","reason":"Forbidden","details":{},"code":403}
+		{`{"metadata":{},"status":"Failure","message":"forbidden: User \"NAME\" cannot GET path \"/\u0026lt;script\u0026gt;\".","reason":"Forbidden","details":{},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Path: "/<script>"}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":"pod \"\" is forbidden: User \"NAME\" cannot GET pod at the cluster scope.","reason":"Forbidden","details":{"kind":"pod"},"code":403}
+		{`{"metadata":{},"status":"Failure","message":"pod is forbidden: User \"NAME\" cannot GET pod at the cluster scope.","reason":"Forbidden","details":{"kind":"pod"},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true}, "", "application/json"},
-		{`{"metadata":{},"status":"Failure","message":"pod.v2 \"\" is forbidden: User \"NAME\" cannot GET pod.v2/quota in the namespace \"test\".","reason":"Forbidden","details":{"group":"v2","kind":"pod"},"code":403}
+		{`{"metadata":{},"status":"Failure","message":"pod \"mypod\" is forbidden: User \"NAME\" cannot GET pod at the cluster scope.","reason":"Forbidden","details":{"name":"mypod","kind":"pod"},"code":403}
+`, authorizer.AttributesRecord{User: u, Verb: "GET", Resource: "pod", ResourceRequest: true, Name: "mypod"}, "", "application/json"},
+		{`{"metadata":{},"status":"Failure","message":"pod.v2 is forbidden: User \"NAME\" cannot GET pod.v2/quota in the namespace \"test\".","reason":"Forbidden","details":{"group":"v2","kind":"pod"},"code":403}
 `, authorizer.AttributesRecord{User: u, Verb: "GET", Namespace: "test", APIGroup: "v2", Resource: "pod", Subresource: "quota", ResourceRequest: true}, "", "application/json"},
 	}
 	for _, test := range cases {

diff --git a/test/integration/master/master_test.go b/test/integration/master/master_test.go
@@ -171,7 +171,7 @@ func TestStatus(t *testing.T) {
 			statusCode:   http.StatusForbidden,
 			reqPath:      "/apis",
 			reason:       "Forbidden",
-			message:      ` "" is forbidden: User "" cannot get path "/apis".: "Everything is forbidden."`,
+			message:      `forbidden: User "" cannot get path "/apis".: "Everything is forbidden."`,
 		},
 		{
 			name:         "401",