DynamicRESTMapper: add mapping for core k8s and kcp types #3630

gman0 · 2025-10-07T19:57:22Z

DynamicRESTMapper only watches for bound and regular CRDs, while CRDs from system:system-crds are not included at all. This PR adds the mappings for these too.

The mapper is now split into two controllers:

DynamicTypesController: manages dynamic, cluster-local types added through bindings or local CRDs,
BuiltinTypesController: manages core k8s and kcp types, common to all clusters.

Additionally, default group-versions are now populated automatically, making resource resolution where version is optional work.

The built-in mappings are now shared, no need to copy them to each cluster as it was before (see DynamicRESTMapper.builtin).

Summary

What Type of PR Is This?

/kind feature

Release Notes

NONE

mjudeikis

few nits

pkg/reconciler/dynamicrestmapper/defaultrestmapper_mutable.go

mjudeikis · 2025-10-08T09:16:01Z

pkg/reconciler/dynamicrestmapper/dynamicrestmapper_builtin_controller.go

+		return
+	}
+
+	key := fmt.Sprintf("%s.%s/%s", crd.Spec.Group, crd.Spec.Names.Plural, crd.Name)


This will be in foramt example.corp.com.machines/machine

Can you add comment that first part must be Group.Resource. Basically explain key structure as its custom key

Ohh that's a bug, thanks for spotting this! It indeed should be <Resource>.<Group>/<CRD name>. We don't have a test that covers deletion for built-in types so this went unchecked. I'll try to think of a way to test this.

Fixed in https://github.com/gman0/kcp/blob/drm-builtin-types/pkg/reconciler/dynamicrestmapper/dynamicrestmapper_builtin_controller.go#L125

Also I've added a test that checks for adding, updating and deleting CRDs in system:system-crds: https://github.com/gman0/kcp/blob/drm-builtin-types/test/integration/dynamicrestmapper/dynamicrestmapper_test.go#L163-L243

I confirm that without the fix above, this test wouldn't pass.

pkg/reconciler/dynamicrestmapper/dynamicrestmapper_builtin_controller.go

pkg/server/controllers.go

mjudeikis · 2025-10-08T09:21:00Z

/retest

gman0 · 2025-10-08T12:46:37Z

/retest

gman0 · 2025-10-08T13:39:04Z

/retest

mjudeikis · 2025-10-09T04:48:30Z

pkg/reconciler/dynamicrestmapper/dynamicrestmapper_builtin_controller.go

+func (c *BuiltinTypesController) process(ctx context.Context, key string) error {
+	logger := logging.WithQueueKey(klog.FromContext(ctx), key)
+
+	parts := strings.Split(key, "/")


Still want a comment with example key so reader does not need to read code to understand what type of custom key is this. same in the place where key is consturcted.
Simple:
// Key example <resource.group>/: machines.corp.example.com/machines

I ended up using the standard kcpcache.MetaClusterNamespaceKeyFunc() because CRD names in system-crds are validated to be <resource>.<group> anyway.

So in BuiltinTypesController.process() we can just extract the GR from the CRD name, and it's fine even if the CRD itself is deleted.

mjudeikis

Only thin comment i asked, but else ping to tag once added.

The mapper is now split into two controllers: * DynamicTypesController: manages dynamic, cluster-local types added through bindings or local CRDs * BuiltinTypesController: manages core k8s and kcp types, common to all clusters On-behalf-of: @SAP [email protected] Signed-off-by: Robert Vasek <[email protected]>

mjudeikis

/lgtm
/approve

kcp-ci-bot · 2025-10-09T08:00:09Z

LGTM label has been added.

Git tree hash: 9b55c3fbb62cc1fd4fe6208e9b2d23515adc4566

kcp-ci-bot · 2025-10-09T08:00:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mjudeikis]

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

gman0 · 2025-10-09T08:19:38Z

/retest

gman0 · 2025-10-09T08:49:16Z

/retest

Both pull-kcp-test-e2e-sharded and pull-kcp-test-e2e-multiple-runs failed at

=== Failed
=== FAIL: test/e2e/workspacetype TestWorkspaceTypes/create_a_workspace_without_an_explicit_type,_get_default_type (31.79s)
    controller_test.go:340: Created root:organization workspace root:e2e-workspace-npsrh as /clusters/36qnjtmawxj1u4b7 on shard "shard-1"
    controller_test.go:346: Create a workspace without explicit type
    controller_test.go:346: Saving test artifacts for test "TestWorkspaceTypes/create_a_workspace_without_an_explicit_type,_get_default_type" under "/logs/artifacts/TestWorkspaceTypes/create_a_workspace_without_an_explicit_type,_get_default_type/935858795".
    controller_test.go:346: 
        	Error Trace:	/home/prow/go/src/github.com/kcp-dev/kcp/test/e2e/workspacetype/controller_test.go:74
        	            				/home/prow/go/src/github.com/kcp-dev/kcp/test/e2e/workspacetype/controller_test.go:346
        	Error:      	Condition never satisfied
        	Test:       	TestWorkspaceTypes/create_a_workspace_without_an_explicit_type,_get_default_type
        	Messages:   	workspace should be ready

=== FAIL: test/e2e/workspacetype TestWorkspaceTypes (0.04s)
    controller_test.go:331: Shared kcp server will target configuration "/home/prow/go/src/github.com/kcp-dev/kcp/.kcp/admin.kubeconfig"
    controller_test.go:331: Waiting for readiness for server at https://127.0.0.1:6444

gman0 force-pushed the drm-builtin-types branch from 20f8e41 to c860a56 Compare October 7, 2025 22:23

kcp-ci-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 7, 2025

gman0 force-pushed the drm-builtin-types branch 2 times, most recently from c7eac4e to 1b72e35 Compare October 8, 2025 07:59

gman0 mentioned this pull request Oct 8, 2025

Add virtual resources #3620

Merged

mjudeikis reviewed Oct 8, 2025

View reviewed changes

gman0 force-pushed the drm-builtin-types branch 2 times, most recently from 82a799a to 85f588b Compare October 8, 2025 12:25

xrstf force-pushed the main branch from feaca28 to 002a578 Compare October 8, 2025 12:59

gman0 force-pushed the drm-builtin-types branch from 85f588b to 9007acc Compare October 8, 2025 22:17

mjudeikis reviewed Oct 9, 2025

View reviewed changes

gman0 force-pushed the drm-builtin-types branch from 9007acc to 9c9b316 Compare October 9, 2025 07:47

mjudeikis approved these changes Oct 9, 2025

View reviewed changes

kcp-ci-bot assigned mjudeikis Oct 9, 2025

kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2025

kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2025

kcp-ci-bot merged commit 8181da0 into kcp-dev:main Oct 9, 2025
14 checks passed

gman0 deleted the drm-builtin-types branch October 9, 2025 10:29

Uh oh!

DynamicRESTMapper: add mapping for core k8s and kcp types #3630

DynamicRESTMapper: add mapping for core k8s and kcp types #3630

Uh oh!

Conversation

gman0 commented Oct 7, 2025

Summary

What Type of PR Is This?

Release Notes

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mjudeikis Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

gman0 Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

gman0 Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mjudeikis commented Oct 8, 2025

Uh oh!

gman0 commented Oct 8, 2025

Uh oh!

gman0 commented Oct 8, 2025

Uh oh!

mjudeikis Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

gman0 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

gman0 Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

mjudeikis left a comment

Choose a reason for hiding this comment

Uh oh!

kcp-ci-bot commented Oct 9, 2025

Uh oh!

kcp-ci-bot commented Oct 9, 2025

Uh oh!

gman0 commented Oct 9, 2025

Uh oh!

gman0 commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gman0 commented Oct 9, 2025 •

edited

Loading