apiexports: add a new API to track APIExport resource scopes

config/crds/admin.kcp.io_apiexporthistories.yaml

@@ -0,0 +1,93 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: v0.17.3
+  name: apiexporthistories.admin.kcp.io
+spec:
+  group: admin.kcp.io
+  names:
+    categories:
+    - kcp
+    kind: APIExportHistory
+    listKind: APIExportHistoryList
+    plural: apiexporthistories
+    singular: apiexporthistory
+  scope: Cluster
+  versions:
+  - additionalPrinterColumns:
+    - jsonPath: .metadata.creationTimestamp
+      name: Age
+      type: date
+    name: v1alpha1
+    schema:
+      openAPIV3Schema:
+        description: |-
+          APIExportHistory records what an APIExport has served in the past, so that a schema
+          cannot be swapped for one that changes a property of an already existing group
+          resource. Today only the resource scope is recorded.
+
+          Its name is the SHA256 of "<cluster>|<name>" of the APIExport it belongs to and it is
+          maintained by kcp in the system:bound-crds logical cluster. It is not meant to be
+          created or edited by users.
+        properties:
+          apiVersion:
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
+            type: string
+          kind:
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
+            type: string
+          metadata:
+            type: object
+          status:
+            description: Status communicates the observed state.
+            properties:
+              resources:
+                description: |-
+                  Resources lists every group resource ever served by the APIExport, together
+                  with the properties it was first served with.
+                items:
+                  description: ResourceHistory records what a single group resource
+                    has been served with.
+                  properties:
+                    group:
+                      description: Group is the API group of the recorded resource.
+                        Empty string means the core group.
+                      type: string
+                    resource:
+                      description: Resource is the plural name of the recorded resource.
+                      minLength: 1
+                      type: string
+                    scope:
+                      description: Scope is the resource scope this group resource
+                        was first served with.
+                      enum:
+                      - Cluster
+                      - Namespaced
+                      type: string
+                  required:
+                  - group
+                  - resource
+                  - scope
+                  type: object
+                type: array
+                x-kubernetes-list-map-keys:
+                - group
+                - resource
+                x-kubernetes-list-type: map
+            type: object
+        type: object
+    served: true
+    storage: true
+    subresources:
+      status: {}

config/system-crds/bootstrap.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/dynamic"
 	"k8s.io/klog/v2"
 
+	"github.com/kcp-dev/sdk/apis/admin"
 	"github.com/kcp-dev/sdk/apis/apis"
 	"github.com/kcp-dev/sdk/apis/cache"
 	"github.com/kcp-dev/sdk/apis/core"
@@ -56,6 +57,7 @@ func Bootstrap(ctx context.Context, crdClient apiextensionsclient.Interface, dis
 		{Group: apis.GroupName, Resource: "apibindings"},
 		{Group: apis.GroupName, Resource: "apiresourceschemas"},
 		{Group: apis.GroupName, Resource: "apiexportendpointslices"},
+		{Group: admin.GroupName, Resource: "apiexporthistories"},
 		{Group: core.GroupName, Resource: "logicalclusters"},
 		{Group: apis.GroupName, Resource: "apiconversions"},
 		{Group: cache.GroupName, Resource: "clustercachedresources"},

hack/update-codegen-crds.sh

@@ -48,7 +48,7 @@ done
 (
   ${KCP_APIGEN_GEN} --input-dir "${REPO_ROOT}"/config/crds --output-dir "${REPO_ROOT}"/config/root-phase0 \
   --ignore-export-schemas cachedobjects.cache.kcp.io \
-  --ignore-schemas apiidentities.admin.kcp.io
+  --ignore-schemas apiidentities.admin.kcp.io,apiexporthistories.admin.kcp.io
 )
 
 

pkg/admission/apiexport/admission.go

@@ -23,16 +23,28 @@ import (
 	"io"
 	"strings"
 
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/endpoints/request"
 
+	"github.com/kcp-dev/logicalcluster/v3"
+	adminv1alpha1 "github.com/kcp-dev/sdk/apis/admin/v1alpha1"
 	"github.com/kcp-dev/sdk/apis/apis"
 	apisv1alpha1 "github.com/kcp-dev/sdk/apis/apis/v1alpha1"
 	apisv1alpha2 "github.com/kcp-dev/sdk/apis/apis/v1alpha2"
+	kcpinformers "github.com/kcp-dev/sdk/client/informers/externalversions"
+	adminv1alpha1listers "github.com/kcp-dev/sdk/client/listers/admin/v1alpha1"
+	apisv1alpha1listers "github.com/kcp-dev/sdk/client/listers/apis/v1alpha1"
 
+	"github.com/kcp-dev/kcp/pkg/admission/initializers"
+	"github.com/kcp-dev/kcp/pkg/reconciler/apis/apibinding"
+	"github.com/kcp-dev/kcp/pkg/reconciler/apis/apiexporthistory"
 	builtinapiexport "github.com/kcp-dev/kcp/pkg/virtual/apiexport/schemas/builtin"
 )
 
@@ -52,6 +64,10 @@ type APIExportAdmission struct {
 	*admission.Handler
 
 	isBuiltIn func(apis.GroupResource) bool
+
+	hasSynced               func() bool
+	apiResourceSchemaLister apisv1alpha1listers.APIResourceSchemaClusterLister
+	historyLister           adminv1alpha1listers.APIExportHistoryClusterLister
 }
 
 // NewAPIExportAdmission constructs a new APIExportAdmission admission plugin.
@@ -64,6 +80,34 @@ func NewAPIExportAdmission(isBuiltIn func(apis.GroupResource) bool) *APIExportAd
 
 // Ensure that the required admission interfaces are implemented.
 var _ = admission.ValidationInterface(&APIExportAdmission{})
+var _ = admission.InitializationValidator(&APIExportAdmission{})
+var _ = initializers.WantsKcpInformers(&APIExportAdmission{})
+
+// SetKcpInformers implements initializers.WantsKcpInformers.
+func (e *APIExportAdmission) SetKcpInformers(local, _ kcpinformers.SharedInformerFactory) {
+	apiResourceSchemas := local.Apis().V1alpha1().APIResourceSchemas()
+	histories := local.Admin().V1alpha1().APIExportHistories()
+
+	e.hasSynced = func() bool {
+		return apiResourceSchemas.Informer().HasSynced() && histories.Informer().HasSynced()
+	}
+	e.apiResourceSchemaLister = apiResourceSchemas.Lister()
+	e.historyLister = histories.Lister()
+}
+
+// ValidateInitialization implements admission.InitializationValidator.
+func (e *APIExportAdmission) ValidateInitialization() error {
+	if e.apiResourceSchemaLister == nil {
+		return fmt.Errorf(PluginName + " plugin needs an APIResourceSchema lister")
+	}
+	if e.historyLister == nil {
+		return fmt.Errorf(PluginName + " plugin needs an APIExportHistory lister")
+	}
+	if e.hasSynced == nil {
+		return fmt.Errorf(PluginName + " plugin needs an informer sync check")
+	}
+	return nil
+}
 
 // Validate ensures that the APIExport is valid.
 func (e *APIExportAdmission) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) {
@@ -172,7 +216,7 @@ func isSystemPrivileged(a admission.Attributes) bool {
 	return false
 }
 
-func (e *APIExportAdmission) validatev1alpha2(_ context.Context, a admission.Attributes, ae *apisv1alpha2.APIExport) (err error) {
+func (e *APIExportAdmission) validatev1alpha2(ctx context.Context, a admission.Attributes, ae *apisv1alpha2.APIExport) (err error) {
 	for i, pc := range ae.Spec.PermissionClaims {
 		if pc.IdentityHash == "" && !e.isBuiltIn(pc.GroupResource) && pc.Group != apis.GroupName {
 			return admission.NewForbidden(a,
@@ -192,9 +236,132 @@ func (e *APIExportAdmission) validatev1alpha2(_ context.Context, a admission.Att
 		}
 	}
 
+	return e.validateHistory(ctx, a, ae)
+}
+
+// validateHistory rejects schemas that change the scope of a group resource the
+// APIExport has served before.
+func (e *APIExportAdmission) validateHistory(ctx context.Context, a admission.Attributes, ae *apisv1alpha2.APIExport) error {
+	if a.GetOperation() != admission.Update {
+		return nil // a newly created APIExport cannot have a history yet
+	}
+
+	// Do not wait for the informers here: the system APIExports are written while
+	// bootstrapping, before the kcp informers this plugin needs are started. Only
+	// those privileged writes are exempt, everybody else fails closed.
+	if e.hasSynced == nil {
+		return nil
+	}
+	if !e.hasSynced() {
+		if isSystemPrivileged(a) {
+			return nil
+		}
+		return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request"))
+	}
+
+	clusterName, err := request.ClusterNameFrom(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to retrieve cluster from context: %w", err)
+	}
+
+	recorded := map[schema.GroupResource]apiextensionsv1.ResourceScope{}
+
+	history, err := e.historyLister.Cluster(apibinding.SystemBoundCRDsClusterName).Get(apiexporthistory.HistoryName(clusterName, ae.Name))
+	if err != nil && !apierrors.IsNotFound(err) {
+		return fmt.Errorf("failed to get scope history for APIExport %s: %w", ae.Name, err)
+	} else if err == nil && history.Annotations[adminv1alpha1.APIExportUIDAnnotationKey] == string(ae.UID) {
+		for _, resource := range history.Status.Resources {
+			recorded[schema.GroupResource{Group: resource.Group, Resource: resource.Resource}] = resource.Scope
+		}
+	}
+
+	old, err := e.oldAPIExport(a)
+	if err != nil {
+		return err
+	}
+	if old != nil {
+		for gr, scope := range e.servedScopes(clusterName, old) {
+			if _, ok := recorded[gr]; !ok {
+				recorded[gr] = scope
+			}
+		}
+	}
+
+	if len(recorded) == 0 {
+		return nil
+	}
+
+	for i, resource := range ae.Spec.Resources {
+		ars, err := e.apiResourceSchemaLister.Cluster(clusterName).Get(resource.Schema)
+		if apierrors.IsNotFound(err) {
+			if _, ok := recorded[schema.GroupResource{Group: resource.Group, Resource: resource.Name}]; !ok {
+				continue
+			}
+			return admission.NewForbidden(a, field.Invalid(
+				field.NewPath("spec").Child("resources").Index(i).Child("schema"),
+				resource.Schema,
+				fmt.Sprintf("APIResourceSchema %q is not known yet and %s.%s has a recorded resource scope, retry once the APIResourceSchema is observed",
+					resource.Schema, resource.Name, resource.Group)))
+		} else if err != nil {
+			return fmt.Errorf("failed to get APIResourceSchema %s: %w", resource.Schema, err)
+		}
+
+		gr := schema.GroupResource{Group: ars.Spec.Group, Resource: ars.Spec.Names.Plural}
+		scope, ok := recorded[gr]
+		if !ok || scope == ars.Spec.Scope {
+			continue
+		}
+
+		return admission.NewForbidden(a, field.Invalid(
+			field.NewPath("spec").Child("resources").Index(i).Child("schema"),
+			resource.Schema,
+			fmt.Sprintf("%s has been served with scope %q by this APIExport, it cannot be served with scope %q; "+
+				"use a different group resource or a new APIExport instead", gr, scope, ars.Spec.Scope)))
+	}
+
 	return nil
 }
 
+func (e *APIExportAdmission) oldAPIExport(a admission.Attributes) (*apisv1alpha2.APIExport, error) {
+	u, ok := a.GetOldObject().(*unstructured.Unstructured)
+	if !ok {
+		return nil, nil
+	}
+
+	switch a.GetKind().GroupVersion().Version {
+	case apisv1alpha1.SchemeGroupVersion.Version:
+		old := &apisv1alpha1.APIExport{}
+		if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, old); err != nil {
+			return nil, fmt.Errorf("failed to convert unstructured to APIExport: %w", err)
+		}
+		v2 := new(apisv1alpha2.APIExport)
+		if err := apisv1alpha2.Convert_v1alpha1_APIExport_To_v1alpha2_APIExport(old, v2, nil); err != nil {
+			return nil, fmt.Errorf("failed to convert v1alpha1 APIExport to v1alpha2: %w", err)
+		}
+		return v2, nil
+	case apisv1alpha2.SchemeGroupVersion.Version:
+		old := &apisv1alpha2.APIExport{}
+		if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, old); err != nil {
+			return nil, fmt.Errorf("failed to convert unstructured to APIExport: %w", err)
+		}
+		return old, nil
+	}
+
+	return nil, nil
+}
+
+func (e *APIExportAdmission) servedScopes(clusterName logicalcluster.Name, ae *apisv1alpha2.APIExport) map[schema.GroupResource]apiextensionsv1.ResourceScope {
+	scopes := make(map[schema.GroupResource]apiextensionsv1.ResourceScope, len(ae.Spec.Resources))
+	for _, resource := range ae.Spec.Resources {
+		ars, err := e.apiResourceSchemaLister.Cluster(clusterName).Get(resource.Schema)
+		if err != nil {
+			continue
+		}
+		scopes[schema.GroupResource{Group: ars.Spec.Group, Resource: ars.Spec.Names.Plural}] = ars.Spec.Scope
+	}
+	return scopes
+}
+
 func validateResourceSchema(resourceSchema apisv1alpha2.ResourceSchema, path *field.Path) *field.Error {
 	group := resourceSchema.Group
 	if group == "" {

pkg/admission/apiexport/admission_test.go

@@ -68,6 +68,22 @@ func updateAttr(name string, obj runtime.Object, kind, resource string) admissio
 	)
 }
 
+func updateAttrWithOld(name string, obj, old runtime.Object, kind, resource string) admission.Attributes {
+	return admission.NewAttributesRecord(
+		helpers.ToUnstructuredOrDie(obj),
+		helpers.ToUnstructuredOrDie(old),
+		apisv1alpha2.Kind(kind).WithVersion("v1alpha2"),
+		"",
+		name,
+		apisv1alpha2.Resource(resource).WithVersion("v1alpha2"),
+		"",
+		admission.Update,
+		&metav1.UpdateOptions{},
+		false,
+		&user.DefaultInfo{},
+	)
+}
+
 func TestAdmission(t *testing.T) {
 	t.Parallel()
 	cases := map[string]struct {