Add a not found error to admission control

pkg/admission/errors.go

@@ -19,22 +19,17 @@ package admission
 import (
 	"k8s.io/kubernetes/pkg/api"
 	apierrors "k8s.io/kubernetes/pkg/api/errors"
+	errs "k8s.io/kubernetes/pkg/util/errors"
 )
 
-// NewForbidden is a utility function to return a well-formatted admission control error response
+func extractKindName(a Attributes) (name, kind string, err error) {
-func NewForbidden(a Attributes, internalError error) error {
+	name = "Unknown"
-	// do not double wrap an error of same type
+	kind = a.GetKind()
-	if apierrors.IsForbidden(internalError) {
-		return internalError
-	}
-
-	name := "Unknown"
-	kind := a.GetKind()
 	obj := a.GetObject()
 	if obj != nil {
 		objectMeta, err := api.ObjectMetaFor(obj)
 		if err != nil {
-			return apierrors.NewForbidden(kind, name, internalError)
+			return "", "", err
 		}
 
 		// this is necessary because name object name generation has not occurred yet
@@ -44,5 +39,27 @@ func NewForbidden(a Attributes, internalError error) error {
 			name = objectMeta.GenerateName
 		}
 	}
+	return name, kind, nil
+}
+
+// NewForbidden is a utility function to return a well-formatted admission control error response
+func NewForbidden(a Attributes, internalError error) error {
+	// do not double wrap an error of same type
+	if apierrors.IsForbidden(internalError) {
+		return internalError
+	}
+	name, kind, err := extractKindName(a)
+	if err != nil {
+		return apierrors.NewInternalError(errs.NewAggregate([]error{internalError, err}))
+	}
 	return apierrors.NewForbidden(kind, name, internalError)
 }
+
+// NewNotFound is a utility function to return a well-formatted admission control error response
+func NewNotFound(a Attributes) error {
+	name, kind, err := extractKindName(a)
+	if err != nil {
+		return apierrors.NewInternalError(err)
+	}
+	return apierrors.NewNotFound(kind, name)
+}

plugin/pkg/admission/namespace/exists/admission.go

@@ -17,12 +17,12 @@ limitations under the License.
 package exists
 
 import (
-	"fmt"
 	"io"
 	"time"
 
 	"k8s.io/kubernetes/pkg/admission"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/client/cache"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
@@ -50,11 +50,11 @@ type exists struct {
 func (e *exists) Admit(a admission.Attributes) (err error) {
 	defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource())
 	if err != nil {
-		return admission.NewForbidden(a, err)
+		return errors.NewInternalError(err)
 	}
 	mapping, err := api.RESTMapper.RESTMapping(kind, defaultVersion)
 	if err != nil {
-		return admission.NewForbidden(a, err)
+		return errors.NewInternalError(err)
 	}
 	if mapping.Scope.Name() != meta.RESTScopeNameNamespace {
 		return nil
@@ -68,7 +68,7 @@ func (e *exists) Admit(a admission.Attributes) (err error) {
 	}
 	_, exists, err := e.store.Get(namespace)
 	if err != nil {
-		return admission.NewForbidden(a, err)
+		return errors.NewInternalError(err)
 	}
 	if exists {
 		return nil
@@ -77,7 +77,10 @@ func (e *exists) Admit(a admission.Attributes) (err error) {
 	// in case of latency in our caches, make a call direct to storage to verify that it truly exists or not
 	_, err = e.client.Namespaces().Get(a.GetNamespace())
 	if err != nil {
-		return admission.NewForbidden(a, fmt.Errorf("Namespace %s does not exist", a.GetNamespace()))
+		if errors.IsNotFound(err) {
+			return err
+		}
+		return errors.NewInternalError(err)
 	}
 
 	return nil

plugin/pkg/admission/namespace/lifecycle/admission.go

@@ -58,11 +58,11 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
 
 	defaultVersion, kind, err := api.RESTMapper.VersionAndKindForResource(a.GetResource())
 	if err != nil {
-		return admission.NewForbidden(a, err)
+		return errors.NewInternalError(err)
 	}
 	mapping, err := api.RESTMapper.RESTMapping(kind, defaultVersion)
 	if err != nil {
-		return admission.NewForbidden(a, err)
+		return errors.NewInternalError(err)
 	}
 	if mapping.Scope.Name() != meta.RESTScopeNameNamespace {
 		return nil
@@ -74,7 +74,7 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
 		},
 	})
 	if err != nil {
-		return admission.NewForbidden(a, err)
+		return errors.NewInternalError(err)
 	}
 
 	// refuse to operate on non-existent namespaces
@@ -82,7 +82,7 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
 		// in case of latency in our caches, make a call direct to storage to verify that it truly exists or not
 		namespaceObj, err = l.client.Namespaces().Get(a.GetNamespace())
 		if err != nil {
-			return admission.NewForbidden(a, fmt.Errorf("Namespace %s does not exist", a.GetNamespace()))
+			return admission.NewNotFound(a)
 		}
 	}
 
@@ -93,6 +93,7 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) {
 			return nil
 		}
 
+		// TODO: This should probably not be a 403
 		return admission.NewForbidden(a, fmt.Errorf("Unable to create new content in namespace %s because it is being terminated.", a.GetNamespace()))
 	}