Skip to content

apiexports: add a new API to track APIExport resource scopes - #4377

Open
neolit123 wants to merge 1 commit into
kcp-dev:mainfrom
neolit123:4330-track-scope-of-apiexport-ars
Open

neolit123 wants to merge 1 commit into
kcp-dev:mainfrom
neolit123:4330-track-scope-of-apiexport-ars

Conversation

@neolit123

@neolit123 neolit123 commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

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.

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/feature Categorizes issue or PR as related to a new feature. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 17, 2026
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from c675d78 to d9afbba Compare September 18, 2026 12:19
@neolit123
neolit123 requested a balanced review from Copilot September 18, 2026 12:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Comment thread pkg/admission/apiexport/admission.go Outdated
Comment thread pkg/admission/apiexport/admission.go Outdated
Comment thread pkg/admission/apiexport/admission.go
Comment thread pkg/reconciler/migration/logicalclustermigration/reconciler.go Outdated
Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile.go Outdated
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from d9afbba to c66dcee Compare September 18, 2026 12:43
@neolit123
neolit123 requested a balanced review from Copilot September 18, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread pkg/reconciler/migration/logicalclustermigration/apiexporthistories.go Outdated
Comment thread pkg/admission/apiexport/admission.go
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from c66dcee to ec57994 Compare September 18, 2026 13:16
@neolit123
neolit123 requested a balanced review from Copilot September 18, 2026 13:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread pkg/admission/apiexport/admission.go
Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_controller.go Outdated
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from ec57994 to a793552 Compare September 18, 2026 13:37
@neolit123
neolit123 requested a balanced review from Copilot September 18, 2026 13:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_controller.go Outdated
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from a793552 to d024dfc Compare September 18, 2026 14:04
Comment thread config/crds/apis.kcp.io_apiexporthistories.yaml Outdated

@mjudeikis mjudeikis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread pkg/admission/apiexport/admission.go Outdated
Comment thread pkg/server/migrationdump/server.go Outdated
Comment thread staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexporthistory.go Outdated
Comment thread staging/src/github.com/kcp-dev/sdk/apis/apis/v1alpha2/types_apiexporthistory.go Outdated
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from d024dfc to 9917564 Compare September 29, 2026 14:59
@neolit123

Copy link
Copy Markdown
Contributor Author

/retest

@neolit123 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 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 mjudeikis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few comment for mysefl to check

Comment thread pkg/admission/apiexport/admission.go
Comment thread pkg/admission/apiexport/admission.go
Comment on lines +279 to +292
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)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: check if need converted here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

})

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@mjudeikis mjudeikis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile.go
Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile.go Outdated
Comment thread test/e2e/reconciler/apiexport/apiexporthistory_test.go
Comment thread config/root-phase0/apiexport-admin.kcp.io.yaml Outdated
@mjudeikis

Copy link
Copy Markdown
Contributor

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

@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from 9917564 to 460eb65 Compare October 19, 2026 13:30
@neolit123
neolit123 requested a balanced review from Copilot October 19, 2026 13:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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
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

Comment thread pkg/reconciler/apis/apiexporthistory/apiexporthistory_reconcile.go Outdated
Comment thread test/e2e/reconciler/apiexport/apiexporthistory_test.go
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from 460eb65 to d8636c4 Compare October 19, 2026 14:26
@neolit123

Copy link
Copy Markdown
Contributor Author

/hold

@kcp-ci-bot 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 kcp-ci-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 27, 2026
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>
@neolit123
neolit123 force-pushed the 4330-track-scope-of-apiexport-ars branch from d8636c4 to b8dec06 Compare October 27, 2026 16:21
@kcp-ci-bot kcp-ci-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 27, 2026
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

[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:

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

@neolit123

Copy link
Copy Markdown
Contributor Author

had to rebase the PR.

/retest

@neolit123

Copy link
Copy Markdown
Contributor Author

/retest

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: changing the scope of an exported resource causes cacher errors

4 participants