Merge pull request kcp-dev#2118 from hasheddan/reservednames-admission

✨ Add reservednames admission plugin to surface clearer error messages

pkg/admission/plugins.go

@@ -55,6 +55,7 @@ import (
 	"github.com/kcp-dev/kcp/pkg/admission/reservedcrdannotations"
 	"github.com/kcp-dev/kcp/pkg/admission/reservedcrdgroups"
 	"github.com/kcp-dev/kcp/pkg/admission/reservedmetadata"
+	"github.com/kcp-dev/kcp/pkg/admission/reservednames"
 	kcpvalidatingwebhook "github.com/kcp-dev/kcp/pkg/admission/validatingwebhook"
 )
 
@@ -73,6 +74,7 @@ var AllOrderedPlugins = beforeWebhooks(kubeapiserveroptions.AllOrderedPlugins,
 	kcpmutatingwebhook.PluginName,
 	reservedcrdannotations.PluginName,
 	reservedcrdgroups.PluginName,
+	reservednames.PluginName,
 	crdnooverlappinggvr.PluginName,
 	reservedmetadata.PluginName,
 	permissionclaims.PluginName,
@@ -107,6 +109,7 @@ func RegisterAllKcpAdmissionPlugins(plugins *admission.Plugins) {
 	kcpmutatingwebhook.Register(plugins)
 	reservedcrdannotations.Register(plugins)
 	reservedcrdgroups.Register(plugins)
+	reservednames.Register(plugins)
 	crdnooverlappinggvr.Register(plugins)
 	reservedmetadata.Register(plugins)
 	permissionclaims.Register(plugins)
@@ -133,6 +136,7 @@ var defaultOnPluginsInKcp = sets.NewString(
 	kcpmutatingwebhook.PluginName,
 	reservedcrdannotations.PluginName,
 	reservedcrdgroups.PluginName,
+	reservednames.PluginName,
 	permissionclaims.PluginName,
 	kubequota.PluginName,
 )

pkg/admission/reservednames/OWNERS

@@ -0,0 +1,2 @@
+reviewers:
+- hasheddan

pkg/admission/reservednames/admission.go

@@ -0,0 +1,107 @@
+/*
+Copyright 2022 The KCP Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package reservednames
+
+import (
+	"context"
+	"io"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/apiserver/pkg/admission"
+
+	tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
+)
+
+// PluginName is the name used to identify this admission webhook.
+const PluginName = "apis.kcp.dev/ReservedNames"
+
+// Register registers the reserved name admission webhook.
+func Register(plugins *admission.Plugins) {
+	plugins.Register(PluginName,
+		func(_ io.Reader) (admission.Interface, error) {
+			return NewReservedNames(), nil
+		})
+}
+
+// A reservedNameFn determines whether the name of the object is reserved.
+type reservedNameFn func(resource schema.GroupResource, kind schema.GroupKind, name string) bool
+
+// newReservedNameFunc builds a reserved name check for the given resource and
+// kind.
+func newReservedNameFn(resource schema.GroupResource, kind schema.GroupKind, reserved ...string) reservedNameFn {
+	res := make(map[string]struct{}, len(reserved))
+	for _, r := range reserved {
+		res[r] = struct{}{}
+	}
+	check := func(name string) bool {
+		_, reserved := res[name]
+		return reserved
+	}
+	return func(objResource schema.GroupResource, objKind schema.GroupKind, objName string) bool {
+		if objResource != resource {
+			return false
+		}
+		if objKind != kind {
+			return false
+		}
+		return check(objName)
+	}
+}
+
+// ReservedNames is an admission plugin for checking reserved object names.
+type ReservedNames struct {
+	*admission.Handler
+
+	reservedNameFns []reservedNameFn
+}
+
+// NewReservedNames constructs a new ReservedNames admission plugin.
+func NewReservedNames() *ReservedNames {
+	return &ReservedNames{
+		Handler: admission.NewHandler(admission.Create),
+		reservedNameFns: []reservedNameFn{
+			newReservedNameFn(
+				tenancyv1alpha1.Resource("clusterworkspaces"),
+				tenancyv1alpha1.Kind("ClusterWorkspace"),
+				tenancyv1alpha1.ClusterWorkspaceReservedNames()...,
+			),
+			newReservedNameFn(
+				tenancyv1alpha1.Resource("clusterworkspacetypes"),
+				tenancyv1alpha1.Kind("ClusterWorkspaceType"),
+				tenancyv1alpha1.ClusterWorkspaceTypeReservedNames()...,
+			),
+		},
+	}
+}
+
+// Ensure that the required admission interfaces are implemented.
+// NOTE(hasheddan): we must use mutation rather than validation because OpenAPI
+// validation will occur prior to validation webhook.
+var _ = admission.MutationInterface(&ReservedNames{})
+
+// Admit ensures that the object does not violate reserved name constraints.
+func (o *ReservedNames) Admit(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) {
+	resource, kind := a.GetResource().GroupResource(), a.GetKind().GroupKind()
+	for _, isReserved := range o.reservedNameFns {
+		if isReserved(resource, kind, a.GetName()) {
+			return admission.NewForbidden(a, field.Invalid(field.NewPath("metadata").Child("name"), a.GetName(), "name is reserved"))
+		}
+	}
+
+	return nil
+}

pkg/admission/reservednames/admission_test.go

@@ -0,0 +1,96 @@
+/*
+Copyright 2022 The KCP Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package reservednames
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/authentication/user"
+
+	tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1"
+)
+
+func createAttr(name string, obj runtime.Object, kind, resource string) admission.Attributes {
+	return admission.NewAttributesRecord(
+		obj,
+		obj,
+		tenancyv1alpha1.Kind(kind).WithVersion("v1alpha1"),
+		"",
+		name,
+		tenancyv1alpha1.Resource(resource).WithVersion("v1alpha1"),
+		"",
+		admission.Create,
+		&metav1.CreateOptions{},
+		false,
+		&user.DefaultInfo{},
+	)
+}
+
+func TestAdmission(t *testing.T) {
+	cases := map[string]struct {
+		attr admission.Attributes
+		want error
+	}{
+		"ForbiddenRootCW": {
+			attr: createAttr("root", &tenancyv1alpha1.ClusterWorkspace{}, "ClusterWorkspace", "clusterworkspaces"),
+			want: field.Invalid(field.NewPath("metadata").Child("name"), "root", "name is reserved"),
+		},
+		"ForbiddenSystemCW": {
+			attr: createAttr("system", &tenancyv1alpha1.ClusterWorkspace{}, "ClusterWorkspace", "clusterworkspaces"),
+			want: field.Invalid(field.NewPath("metadata").Child("name"), "system", "name is reserved"),
+		},
+		"ValidCW": {
+			attr: createAttr("cool-cw", &tenancyv1alpha1.ClusterWorkspace{}, "ClusterWorkspace", "clusterworkspaces"),
+		},
+		"ForbiddenAnyCWT": {
+			attr: createAttr("any", &tenancyv1alpha1.ClusterWorkspaceType{}, "ClusterWorkspaceType", "clusterworkspacetypes"),
+			want: field.Invalid(field.NewPath("metadata").Child("name"), "any", "name is reserved"),
+		},
+		"ForbiddenSystemCWT": {
+			attr: createAttr("system", &tenancyv1alpha1.ClusterWorkspaceType{}, "ClusterWorkspaceType", "clusterworkspacetypes"),
+			want: field.Invalid(field.NewPath("metadata").Child("name"), "system", "name is reserved"),
+		},
+		"ValidCWT": {
+			attr: createAttr("cool-cwt", &tenancyv1alpha1.ClusterWorkspaceType{}, "ClusterWorkspaceType", "clusterworkspacetypes"),
+		},
+		"NotApplicableResource": {
+			attr: createAttr("root", &tenancyv1alpha1.ClusterWorkspace{}, "ClusterWorkspace", "notacworcwt"),
+		},
+		"NotApplicableKind": {
+			attr: createAttr("root", &tenancyv1alpha1.ClusterWorkspace{}, "NotaCWorCWT", "clusterworkspaces"),
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			plugin := NewReservedNames()
+			if err := plugin.Admit(context.Background(), tc.attr, nil); err != nil {
+				require.Contains(t, err.Error(), tc.want.Error())
+				return
+			}
+			if tc.want != nil {
+				t.Errorf("no error returned but expected: %s", tc.want.Error())
+			}
+		})
+	}
+}

pkg/apis/tenancy/v1alpha1/types.go

@@ -34,6 +34,17 @@ var RootCluster = logicalcluster.New("root")
 // RootShard holds a name of the root shard.
 var RootShard = "root"
 
+// ClusterWorkspaceReservedNames defines the set of names that may not be used
+// on user-supplied ClusterWorkspaces.
+// TODO(hasheddan): tie this definition of reserved names to the patches used to
+// apply the same restrictions to the OpenAPISchema.
+func ClusterWorkspaceReservedNames() []string {
+	return []string{
+		"root",
+		"system",
+	}
+}
+
 // ClusterWorkspace defines a Kubernetes-cluster-like endpoint that holds a default set
 // of resources and exhibits standard Kubernetes API semantics of CRUD operations. It represents
 // the full life-cycle of the persisted data in this workspace in a KCP installation.
@@ -145,6 +156,17 @@ func (r ClusterWorkspaceTypeReference) Equal(other ClusterWorkspaceTypeReference
 	return r.Name == other.Name && r.Path == other.Path
 }
 
+// ClusterWorkspaceTypeReservedNames defines the set of names that may not be
+// used on user-supplied ClusterWorkspaceTypes.
+// TODO(hasheddan): tie this definition of reserved names to the patches used to
+// apply the same restrictions to the OpenAPISchema.
+func ClusterWorkspaceTypeReservedNames() []string {
+	return []string{
+		"any",
+		"system",
+	}
+}
+
 // ClusterWorkspaceType specifies behaviour of workspaces of this type.
 //
 // +crd