set/validate object namespace before admission

This commit is contained in:
Jordan Liggitt 2020-09-09 01:12:31 -04:00
parent f0ef426238
commit 92422a7305
8 changed files with 147 additions and 52 deletions

View File

@ -47,7 +47,7 @@ import (
utiltrace "k8s.io/utils/trace"
)
var namespaceGVK = schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"}
var namespaceGVR = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"}
func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Interface, includeName bool) http.HandlerFunc {
return func(w http.ResponseWriter, req *http.Request) {
@ -152,7 +152,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
if len(name) == 0 {
_, name, _ = scope.Namer.ObjectName(obj)
}
if len(namespace) == 0 && *gvk == namespaceGVK {
if len(namespace) == 0 && scope.Resource == namespaceGVR {
namespace = name
}
ctx = request.WithNamespace(ctx, namespace)
@ -163,6 +163,15 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
userInfo, _ := request.UserFrom(ctx)
// if this object supports namespace info
if objectMeta, err := meta.Accessor(obj); err == nil {
// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
if err := rest.EnsureObjectNamespaceMatchesRequestNamespace(rest.ExpectedNamespaceForResource(namespace, scope.Resource), objectMeta); err != nil {
scope.err(err, w, req)
return
}
}
trace.Step("About to store object in database")
admissionAttributes := admission.NewAttributesRecord(obj, nil, scope.Kind, namespace, name, scope.Resource, scope.Subresource, admission.Create, options, dryrun.IsDryRun(options.DryRun), userInfo)
requestFunc := func() (runtime.Object, error) {

View File

@ -576,6 +576,14 @@ func (p *patcher) applyPatch(ctx context.Context, _, currentObject runtime.Objec
return nil, errors.NewConflict(p.resource.GroupResource(), p.name, fmt.Errorf("uid mismatch: the provided object specified uid %s, and no existing object was found", accessor.GetUID()))
}
// if this object supports namespace info
if objectMeta, err := meta.Accessor(objToUpdate); err == nil {
// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
if err := rest.EnsureObjectNamespaceMatchesRequestNamespace(rest.ExpectedNamespaceForResource(p.namespace, p.resource), objectMeta); err != nil {
return nil, err
}
}
if err := checkName(objToUpdate, p.name, p.namespace, p.namer); err != nil {
return nil, err
}

View File

@ -139,6 +139,15 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
audit.LogRequestObject(req.Context(), obj, objGV, scope.Resource, scope.Subresource, scope.Serializer)
admit = admission.WithAudit(admit, ae)
// if this object supports namespace info
if objectMeta, err := meta.Accessor(obj); err == nil {
// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
if err := rest.EnsureObjectNamespaceMatchesRequestNamespace(rest.ExpectedNamespaceForResource(namespace, scope.Resource), objectMeta); err != nil {
scope.err(err, w, req)
return
}
}
if err := checkName(obj, name, namespace, scope.Namer); err != nil {
scope.err(err, w, req)
return

View File

@ -18,6 +18,7 @@ package rest
import (
"context"
"fmt"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
@ -99,13 +100,15 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime.
return kerr
}
if strategy.NamespaceScoped() {
if !ValidNamespace(ctx, objectMeta) {
return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request")
// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
requestNamespace, ok := genericapirequest.NamespaceFrom(ctx)
if !ok {
return errors.NewInternalError(fmt.Errorf("no namespace information found in request context"))
}
} else if len(objectMeta.GetNamespace()) > 0 {
objectMeta.SetNamespace(metav1.NamespaceNone)
if err := EnsureObjectNamespaceMatchesRequestNamespace(ExpectedNamespaceForScope(requestNamespace, strategy.NamespaceScoped()), objectMeta); err != nil {
return err
}
objectMeta.SetDeletionTimestamp(nil)
objectMeta.SetDeletionGracePeriodSeconds(nil)
strategy.PrepareForCreate(ctx, obj)

View File

@ -17,11 +17,10 @@ limitations under the License.
package rest
import (
"context"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/uuid"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
)
// FillObjectMetaSystemFields populates fields that are managed by the system on ObjectMeta.
@ -31,13 +30,44 @@ func FillObjectMetaSystemFields(meta metav1.Object) {
meta.SetSelfLink("")
}
// ValidNamespace returns false if the namespace on the context differs from
// the resource. If the resource has no namespace, it is set to the value in
// the context.
func ValidNamespace(ctx context.Context, resource metav1.Object) bool {
ns, ok := genericapirequest.NamespaceFrom(ctx)
if len(resource.GetNamespace()) == 0 {
resource.SetNamespace(ns)
// EnsureObjectNamespaceMatchesRequestNamespace returns an error if obj.Namespace and requestNamespace
// are both populated and do not match. If either is unpopulated, it modifies obj as needed to ensure
// obj.GetNamespace() == requestNamespace.
func EnsureObjectNamespaceMatchesRequestNamespace(requestNamespace string, obj metav1.Object) error {
objNamespace := obj.GetNamespace()
switch {
case objNamespace == requestNamespace:
// already matches, no-op
return nil
case objNamespace == metav1.NamespaceNone:
// unset, default to request namespace
obj.SetNamespace(requestNamespace)
return nil
case requestNamespace == metav1.NamespaceNone:
// cluster-scoped, clear namespace
obj.SetNamespace(metav1.NamespaceNone)
return nil
default:
// mismatch, error
return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request")
}
return ns == resource.GetNamespace() && ok
}
// ExpectedNamespaceForScope returns the expected namespace for a resource, given the request namespace and resource scope.
func ExpectedNamespaceForScope(requestNamespace string, namespaceScoped bool) string {
if namespaceScoped {
return requestNamespace
}
return ""
}
// ExpectedNamespaceForResource returns the expected namespace for a resource, given the request namespace.
func ExpectedNamespaceForResource(requestNamespace string, resource schema.GroupVersionResource) string {
if resource.Resource == "namespaces" && resource.Group == "" {
return ""
}
return requestNamespace
}

View File

@ -21,8 +21,6 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apiserver/pkg/apis/example"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
)
// TestFillObjectMetaSystemFields validates that system populated fields are set on an object
@ -55,30 +53,65 @@ func TestHasObjectMetaSystemFieldValues(t *testing.T) {
}
}
// TestValidNamespace validates that namespace rules are enforced on a resource prior to create or update
func TestValidNamespace(t *testing.T) {
ctx := genericapirequest.NewDefaultContext()
namespace, _ := genericapirequest.NamespaceFrom(ctx)
// TODO: use some genericapiserver type here instead of clientapiv1
resource := example.Pod{}
if !ValidNamespace(ctx, &resource.ObjectMeta) {
t.Fatalf("expected success")
func TestEnsureObjectNamespaceMatchesRequestNamespace(t *testing.T) {
testcases := []struct {
name string
reqNS string
objNS string
expectErr bool
expectObjNS string
}{
{
name: "cluster-scoped req, cluster-scoped obj",
reqNS: "",
objNS: "",
expectErr: false,
expectObjNS: "",
},
{
name: "cluster-scoped req, namespaced obj",
reqNS: "",
objNS: "foo",
expectErr: false,
expectObjNS: "", // no error, object is forced to cluster-scoped for backwards compatibility
},
{
name: "namespaced req, no-namespace obj",
reqNS: "foo",
objNS: "",
expectErr: false,
expectObjNS: "foo", // no error, object is updated to match request for backwards compatibility
},
{
name: "namespaced req, matching obj",
reqNS: "foo",
objNS: "foo",
expectErr: false,
expectObjNS: "foo",
},
{
name: "namespaced req, mis-matched obj",
reqNS: "foo",
objNS: "bar",
expectErr: true,
},
}
if namespace != resource.Namespace {
t.Fatalf("expected resource to have the default namespace assigned during validation")
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
obj := metav1.ObjectMeta{Namespace: tc.objNS}
err := EnsureObjectNamespaceMatchesRequestNamespace(tc.reqNS, &obj)
if tc.expectErr {
if err == nil {
t.Fatal("expected err, got none")
}
resource = example.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "other"}}
if ValidNamespace(ctx, &resource.ObjectMeta) {
t.Fatalf("Expected error that resource and context errors do not match because resource has different namespace")
}
ctx = genericapirequest.NewContext()
if ValidNamespace(ctx, &resource.ObjectMeta) {
t.Fatalf("Expected error that resource and context errors do not match since context has no namespace")
return
} else if err != nil {
t.Fatalf("unexpected err: %v", err)
}
ctx = genericapirequest.NewContext()
ns := genericapirequest.NamespaceValue(ctx)
if ns != "" {
t.Fatalf("Expected the empty string")
if obj.Namespace != tc.expectObjNS {
t.Fatalf("expected obj ns %q, got %q", tc.expectObjNS, obj.Namespace)
}
})
}
}

View File

@ -28,6 +28,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/admission"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiserver/pkg/warning"
@ -110,12 +111,14 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx context.Context, obj, old run
if kerr != nil {
return kerr
}
if strategy.NamespaceScoped() {
if !ValidNamespace(ctx, objectMeta) {
return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request")
// ensure namespace on the object is correct, or error if a conflicting namespace was set in the object
requestNamespace, ok := genericapirequest.NamespaceFrom(ctx)
if !ok {
return errors.NewInternalError(fmt.Errorf("no namespace information found in request context"))
}
} else if len(objectMeta.GetNamespace()) > 0 {
objectMeta.SetNamespace(metav1.NamespaceNone)
if err := EnsureObjectNamespaceMatchesRequestNamespace(ExpectedNamespaceForScope(requestNamespace, strategy.NamespaceScoped()), objectMeta); err != nil {
return err
}
// Ensure requests cannot update generation

View File

@ -203,8 +203,8 @@ func TestServiceAccountTokenCreate(t *testing.T) {
treqWithBadNamespace := treq.DeepCopy()
treqWithBadNamespace.Namespace = "invalid-namespace"
if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treqWithBadNamespace, metav1.CreateOptions{}); err == nil || !strings.Contains(err.Error(), "must match the service account namespace") {
t.Fatalf("expected err creating token with mismatched namespace but got: %#v", resp)
if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treqWithBadNamespace, metav1.CreateOptions{}); err == nil || !strings.Contains(err.Error(), "does not match the namespace") {
t.Fatalf("expected err creating token with mismatched namespace but got: %#v, %v", resp, err)
}
warningHandler.clear()