apiexports: add a new API to track APIExport resource scopes by neolit123 · Pull Request #4377 · kcp-dev/kcp

neolit123 · 2026-09-17T15:27:39Z

Summary

As discovered earlier if the scope of a resource changed from cluster to namespaced, and this resource is re-introduced in the same APIExport, existing consumer CR instances can cause some confusion in kcp (high MEM/CPU usage).

Add a new API that tracks the APIExport resource scopes, and block on admission if the scope of a previously known resource changed.

If the provider wants to workaround this problem, they need to either remove the APIExport or introduce a different GVR.

slack thread:
https://kubernetes.slack.com/archives/C09C7UP1VLM/p1789467382474599

What Type of PR Is This?

/kind bug feature

Related Issue(s)

Fixes #4330

Release Notes

Block on admission if for an existing APIExport a previously known resource is re-introduced with a different scope (namespaced/cluster). Either the APIExport must be deleted or the GVR of the resource must be changed.

Copilot

🟡 Changes recommended

Admission race and bypass paths can still permit scope changes, while migration failures can advance to source cleanup before histories are copied.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds persistent APIExport scope history and admission enforcement to prevent previously served resources from changing between cluster and namespace scope.

Changes:

Introduces the APIExportHistory API and generated SDK support.
Adds history reconciliation, admission validation, and migration handling.
Adds unit and end-to-end coverage.

File summaries

File	Description
`test/e2e/reconciler/apiexport/apiexporthistory_test.go`	Tests scope-change rejection end to end.
`staging/src/github.com/kcp-dev/sdk/openapi/zz_generated.openapi.go`	Adds generated OpenAPI definitions.
`staging/src/github.com/kcp-dev/sdk/client/listers/apis/v1alpha2/expansion_generated.go`	Adds lister expansion interfaces.
`staging/src/github.com/kcp-dev/sdk/client/listers/apis/v1alpha2/apiexporthistory.go`	Adds generated history listers.
`staging/src/github.com/kcp-dev/sdk/client/informers/externalversions/generic.go`	Registers generic history informers.
`staging/src/github.com/kcp-dev/sdk/client/informers/externalversions/apis/v1alpha2/interface.go`	Exposes history informers.
`staging/src/github.com/kcp-dev/sdk/client/informers/externalversions/apis/v1alpha2/apiexporthistory.go`	Implements generated history informers.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/generated_expansion.go`	Adds client expansion interface.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/fake/fake_apis_client.go`	Exposes the fake history client.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/fake/fake_apiexporthistory.go`	Implements the fake history client.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apis_client.go`	Exposes the typed history client.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apiexporthistory.go`	Implements the typed history client.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/generated_expansion.go`	Adds cluster-client expansion.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apis_client.go`	Exposes fake cluster history clients.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apiexporthistory.go`	Implements fake cluster history clients.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apis_client.go`	Exposes the cluster history client.
`staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apiexporthistory.go`	Implements the cluster history client.
`staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/utils.go`	Registers history apply configurations.
`staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/resourcehistory.go`	Adds resource-history apply support.
`staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistorystatus.go`	Adds status apply support.
`staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryspec.go`	Adds spec apply support.
`staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryref.go`	Adds reference apply support.
`staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistory.go`	Adds resource apply support.
`staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.model_name.go`	Adds generated model names.
`staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.deepcopy.go`	Adds generated deep-copy methods.
`staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexporthistory.go`	Defines the history API types.
`staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/register.go`	Registers history API types.
`pkg/server/server.go`	Installs the history controller.
`pkg/server/controllers.go`	Configures controller informers and indexes.
`pkg/reconciler/migration/logicalclustermigration/reconciler.go`	Integrates history migration.
`pkg/reconciler/migration/logicalclustermigration/apiexporthistories.go`	Copies histories between shards.
`pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile.go`	Records and cleans up scope history.
`pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile_test.go`	Tests history reconciliation.
`pkg/reconciler/apis/apiexporthistory/apiexporthistory_controller.go`	Implements the history controller.
`pkg/indexers/apiexport.go`	Adds history-to-export indexing.
`pkg/admission/apiexport/history_admission_test.go`	Tests history admission behavior.
`pkg/admission/apiexport/admission.go`	Rejects incompatible scope changes.
`config/system-crds/bootstrap.go`	Bootstraps the history CRD.
`config/crds/apis.kcp.io_apiexporthistories.yaml`	Defines the history CRD.

Review details

Files not reviewed (16)

staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.deepcopy.go: Generated file
staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.model_name.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryref.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryspec.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistorystatus.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/resourcehistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/utils.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/generated_expansion.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/fake/fake_apiexporthistory.go: Generated file

Files reviewed: 23/39 changed files
Comments generated: 5
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

History migration uses an incompatible endpoint, and admission still permits a remove-and-readd race.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (14)

staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.deepcopy.go: Generated file
staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.model_name.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryref.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryspec.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistorystatus.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/resourcehistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/utils.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/generated_expansion.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apiexporthistory.go: Generated file

Files reviewed: 27/41 changed files
Comments generated: 2
Review effort level: Balanced

Copilot

🟡 Changes recommended

Admission and informer races can still allow prohibited scope changes or leave histories permanently unpopulated.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (15)

staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.deepcopy.go: Generated file
staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.model_name.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryref.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryspec.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistorystatus.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/resourcehistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/utils.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/generated_expansion.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apis_client.go: Generated file

Suppressed comments (2)

pkg/admission/apiexport/admission.go:230

Comparing only the immediately previous APIExport does not close the missing-history race. Before the asynchronous controller records the initial scope, a client can first remove the resource and then re-add the same GVR with the opposite scope; on the second update the history is absent and the old object has no resource, so recorded is empty and the change is admitted. The initial baseline must be established synchronously, or updates/removals must be rejected until the history is available; this two-update sequence should also be covered by a test.

	if old != nil {
		for gr, scope := range e.servedScopes(clusterName, old) {
			if _, ok := recorded[gr]; !ok {
				recorded[gr] = scope
			}
		}

pkg/admission/apiexport/admission.go:240

Treating a cache miss as permission to continue creates another bypass even when history already exists. A provider can create an opposite-scope schema and immediately update the APIExport before the schema informer observes it; this NotFound branch admits the update, and the history controller later preserves the old scope rather than repairing the now-conflicting export. For a resource whose declared group/name already appears in recorded, reject or retry while its schema cannot be resolved (or use a live read).

	for i, resource := range ae.Spec.Resources {
		ars, err := e.apiResourceSchemaLister.Cluster(clusterName).Get(resource.Schema)
		if apierrors.IsNotFound(err) {
			continue

Files reviewed: 25/40 changed files
Comments generated: 2
Review effort level: Balanced

Copilot

🟡 Changes recommended

Shard-local histories are not preserved through logical-cluster migration, allowing prior scope constraints to be lost.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (15)

staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.deepcopy.go: Generated file
staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/zz_generated.model_name.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryref.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistoryspec.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/apiexporthistorystatus.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/apis/v1alpha2/resourcehistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/utils.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/fake/apis_client.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/apis/v1alpha2/generated_expansion.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/typed/apis/v1alpha2/apis_client.go: Generated file

Files reviewed: 25/40 changed files
Comments generated: 1
Review effort level: Balanced

mjudeikis

First rould of review. Some changes will tune the implementation a bit

neolit123 · 2026-09-29T17:43:44Z

/retest

mjudeikis

few comment for mysefl to check

mjudeikis · 2026-10-13T11:59:03Z

+		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)
+		}


TODO: check if need converted here.

Checked: no conversion needed here. For an UPDATE the old object is handed to admission in the request version, so a.GetKind().GroupVersion() applies to both old and new; the v1alpha1 branch above already converts and Convert_v1alpha1_APIExport_To_v1alpha2_APIExport folds in the overhanging ResourceSchemasAnnotation. Resolving.

mjudeikis · 2026-10-13T12:04:13Z

+	})
+
+	_, _ = apiResourceSchemaInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc: func(obj interface{}) { c.enqueueAPIResourceSchema(obj) },


TODO: check if we can do noop schema update (change labels)

Checked: APIResourceSchema spec is fully immutable on update (ValidateAPIResourceSchemaUpdate), so a labels-only update cannot change scope and AddFunc alone is sufficient. Resolving.

mjudeikis

Second round. Overall shape is good now (admin group, hashed name, no spec, no migration). Two things I think need fixing before this merges: the stale-history case on APIExport delete+recreate, and the lister-miss skip in APIExport admission. Details inline.

mjudeikis · 2026-10-13T12:25:35Z

@neolit123 did a manual review and AI-assisted, as this one is a big one :/ few comments before merging

Copilot

🟡 Changes recommended

A failed status write during history rebinding can permanently retain a deleted export’s scopes and block valid updates.

2 open issues

2 resolved since last review

This stores the history only in the current shard's system:bound-crds cluster, but…
Skipping an unresolved schema provides a non-racy bypass: update the export to reference a…

12 files not reviewed

staging/src/github.com/kcp-dev/sdk/apis/admin/v1alpha1/zz_generated.deepcopy.go: Generated file
staging/src/github.com/kcp-dev/sdk/apis/admin/v1alpha1/zz_generated.model_name.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/admin/v1alpha1/apiexporthistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/admin/v1alpha1/apiexporthistorystatus.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/admin/v1alpha1/resourcehistory.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/applyconfiguration/utils.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/clientset.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/clientset.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/fake/clientset.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/fake/register.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/scheme/register.go: Generated file
staging/src/github.com/kcp-dev/sdk/client/clientset/versioned/cluster/typed/admin/v1alpha1/admin_client.go: Generated file

_{Review effort: Balanced}

neolit123 · 2026-10-19T15:36:08Z

/hold

As discovered earlier if the scope of a resource changed from cluster to namespaced, and this resource is re-introduced in the same APIExport, existing consumer CR instances can cause some confusion in kcp. Add a new API that tracks the APIExport resource scopes, and block on admission if the scope of a previously known resource changed. If the provider wants to workaround this problem, they need to either remove the APIExport or introduce a different GVR. Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>

kcp-ci-bot · 2026-10-27T16:21:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ntnn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS
config/crds/OWNERS
pkg/admission/OWNERS
staging/src/github.com/kcp-dev/sdk/apis/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

neolit123 · 2026-10-27T16:48:00Z

had to rebase the PR.

/retest

neolit123 · 2026-10-27T18:32:10Z

/retest

mjudeikis · 2026-11-03T07:35:46Z

@mjudeikis-bot do final review.

mjudeikis-bot

Review of head b8dec06. Correctness of the core design holds (UID-bound history, status-then-annotation rebind, schema-create admission). Findings below are about the remaining windows in which the fix for #4330 can still be bypassed, plus two notes.

Not anchored

Note: histories live only in the shard-local system:bound-crds cluster and the migration copy was dropped from this PR. After a logical-cluster migration the destination shard rebuilds history from the currently referenced schemas, so any GVR that was removed before the move loses its recorded scope and can be re-added with the opposite scope. Fine as a deliberate trade-off, but it should be stated in the release note or docs and tracked in a follow-up.
Note: histories are deleted only from the APIExport delete event (process -> deleteHistories). If an APIExport (or its whole workspace) is deleted while the controller is not running, nothing ever re-queues that key, so the history object is orphaned in system:bound-crds for good. Small objects, but unbounded over time; a startup sweep that lists histories and drops the ones without a live APIExport UID would close it.

Checked

Read beyond the diff: pkg/admission/apiexport/admission.go (whole Validate, status path, isSystemPrivileged), pkg/admission/apiresourceschema/admission.go, the history controller, reconcile and their tests, pkg/server/controllers.go wiring and addIndexersToInformers, the e2e test, the existing review threads (Copilot, neolit123, mjudeikis) so already-resolved items are not re-raised.
Correctness: stale-UID rebind is recoverable (status write first, annotation second; a failed second write retries as stale, covered by TestReconcile); mergeScopes never replaces a scope; conflicts and AlreadyExists are returned for retry. One finding on the remove/re-add window (below).
Concurrency: single keyed workqueue, listers only read; no shared mutable state in the plugins. N/A beyond that.
kcp specifics: all lister calls are cluster-scoped (Cluster(clusterName), histories under SystemBoundCRDsClusterName); history name hashes cluster+name so APIExports with equal names in different workspaces do not collide; no cross-workspace read exposed to users (CRD only in system:bound-crds, root-phase0 export dropped). Admission uses local informers only, no cache-server dependency.
API/storage: new admin.kcp.io group, +required/+optional and enum markers match the code, CRD yaml present in config/crds, bootstrap registered; I did not re-run make codegen, I only confirmed everything compiles and the generated files are in the diff.
Security: no request-derived strings reach logs beyond object names; bootstrap exemption is limited to system:masters (isSystemPrivileged), others fail closed while informers are unsynced.
Tests: unit tests for merge/reconcile/stale, both admission plugins (incl. old-object fallback, unsynced informers) and an e2e. The e2e does not prove persistence (Note below).
Commands: go build ./pkg/... ./cmd/... ok; go vet ./pkg/admission/... ./pkg/reconciler/apis/apiexporthistory/... clean; go test ./pkg/admission/apiexport/... ./pkg/admission/apiresourceschema/... ./pkg/reconciler/apis/apiexporthistory/... ok; make build-e2e ok; go test ./test/e2e/reconciler/apiexport -run TestAPIExportHistoryScope passed (14.9s). Linter (golangci-lint) not run.

mjudeikis-bot · 2026-11-03T07:39:39Z

+		return setSchemas(resourceSchema(name))
+	}
+
+	t.Logf("Wait for the scope to be recorded, swapping in the cluster scoped schema must be rejected")


Note: the e2e does not prove the history was persisted, and the remove/re-add step is racy
The first loop is satisfied by the old-object fallback in validateHistory, so it passes even if the controller never writes anything. The later "Removing ... / Re-adding with the opposite scope" steps run immediately after, so they depend on the controller having recorded sheriffs by then; if it has not, the re-add is admitted (see the admission comment) and the test flakes. Wait for the persisted history before removing, for example:

kcptestinghelpers.Eventually(t, func() (bool, string) { h, err := kcpClusterClient.Cluster(apibinding.SystemBoundCRDsClusterName.Path()).AdminV1alpha1().APIExportHistories().Get(t.Context(), apiexporthistory.HistoryName(logicalcluster.Name(providerCluster), "wildwest-sheriffs"), metav1.GetOptions{}) return err == nil && len(h.Status.Resources) == 1, fmt.Sprint(err) }, wait.ForeverTestTimeout, 100*time.Millisecond)

We need this addressed

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from c675d78 to d9afbba Compare September 18, 2026 12:19

neolit123 requested a balanced review from Copilot September 18, 2026 12:19

Copilot started reviewing on behalf of neolit123 September 18, 2026 12:19

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from d9afbba to c66dcee Compare September 18, 2026 12:43

neolit123 requested a balanced review from Copilot September 18, 2026 12:43

Copilot started reviewing on behalf of neolit123 September 18, 2026 12:44

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread pkg/reconciler/migration/logicalclustermigration/apiexporthistories.go Resolved

Comment thread pkg/admission/apiexport/admission.go Resolved

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from c66dcee to ec57994 Compare September 18, 2026 13:16

neolit123 requested a balanced review from Copilot September 18, 2026 13:19

Copilot started reviewing on behalf of neolit123 September 18, 2026 13:19

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread pkg/admission/apiexport/admission.go Resolved

Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_controller.go Resolved

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from ec57994 to a793552 Compare September 18, 2026 13:37

neolit123 requested a balanced review from Copilot September 18, 2026 13:37

Copilot started reviewing on behalf of neolit123 September 18, 2026 13:39

View session

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_controller.go Resolved

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from a793552 to d024dfc Compare September 18, 2026 14:04

mjudeikis reviewed Sep 28, 2026

View reviewed changes

Comment thread config/crds/apis.kcp.io_apiexporthistories.yaml Resolved

mjudeikis reviewed Sep 28, 2026

View reviewed changes

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from d024dfc to 9917564 Compare September 29, 2026 14:59

neolit123 changed the title ~~WIP: apiexports: add a new API to track APIExport resource scopes~~ apiexports: add a new API to track APIExport resource scopes Sep 29, 2026

kcp-ci-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 29, 2026

mjudeikis reviewed Oct 13, 2026

View reviewed changes

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from 9917564 to 460eb65 Compare October 19, 2026 13:30

neolit123 requested a balanced review from Copilot October 19, 2026 13:32

Copilot started reviewing on behalf of neolit123 October 19, 2026 13:33

View session

Copilot AI reviewed Oct 19, 2026

View reviewed changes

Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile.go Resolved

Comment thread test/e2e/reconciler/apiexport/apiexporthistory_test.go Resolved

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from 460eb65 to d8636c4 Compare October 19, 2026 14:26

kcp-ci-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2026

kcp-ci-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 27, 2026

neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from d8636c4 to b8dec06 Compare October 27, 2026 16:21

kcp-ci-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 27, 2026

mjudeikis-bot reviewed Nov 3, 2026

View reviewed changes

Conversation

neolit123 commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What Type of PR Is This?

Related Issue(s)

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (16)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (14)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (15)

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Files not reviewed (15)

Uh oh!

Uh oh!

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neolit123 commented Sep 29, 2026

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mjudeikis Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

mjudeikis Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

mjudeikis Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

mjudeikis Oct 13, 2026

Choose a reason for hiding this comment

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjudeikis commented Oct 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

neolit123 commented Sep 17, 2026 •

edited

Loading