Skip to content

[1- De-Rooting] Add APIIdentity - #4488

Merged
kcp-ci-bot merged 9 commits into
kcp-dev:mainfrom
mjudeikis:identity.rework.part3
Oct 27, 2026
Merged

[1- De-Rooting] Add APIIdentity#4488
kcp-ci-bot merged 9 commits into
kcp-dev:mainfrom
mjudeikis:identity.rework.part3

Conversation

@mjudeikis

@mjudeikis mjudeikis commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This adds admin api for apiidentity tracking. This will allow centralized cache-server based apiexport indetity tracking.
We need this so we can decouple root shard from being Single point of failure.

k get apiidentities.admin.kcp.io                                                                                                                                                                                                                                                                                                               16:50:34
NAME                                                               OWNER                CLUSTER            AGE
05ac8e691ea654903fe6a26d82f1b072f847b2cf1404dcad3692d43b3bbc20fa   cache.kcp.io         root               6m11s
0f3cf893b1b37ec16cc6a86e317fbebfb9d7766a60c748688d5f84b949e00bf3   wildwest             2o5j3kntkazp0xg7   72s
1892ba382b4634c57c3364a7c197d89922d7c801f6adc70487198aca40ddbbd3   topology.kcp.io      root               6m11s
1da7d33cd17657d1f6316045a1a17c5b8a6e693d9a9d36fd15128b6d488197f8   migration.kcp.io     root               6m11s
3556a7d5b8b9e1713c650c222408915c6c44c6797775a6cfc9e63d377134a8c5   shards.core.kcp.io   root               6m11s
9c73e0fa62cc8204cba72e460a7a71f8bc0bebb3f053817212b2323d0ab88f64   tenancy.kcp.io       root               6m11s

For now no user facing changes so we should be able to be more lean how we approach this

What Type of PR Is This?

/kind feature

Related Issue(s)

Fixes #

Release Notes

Add APIIdentity tracking

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. dco-signoff: yes Indicates the PR's author has signed the DCO. 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 Oct 23, 2026
@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@mjudeikis-bot review this.

@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@mjudeikis-bot review this.

mjudeikis-bot
mjudeikis-bot reviewed Oct 26, 2026
View reviewed changes

@mjudeikis-bot mjudeikis-bot 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.

No issues found. Checked: the registry controller's owner-missing/succession/holder-drop logic, the APIExport claim and holder path in reconcileIdentity, the admin authorizer verb and group restrictions, and the unit tests for those packages (all pass).

@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@mjudeikis mjudeikis mentioned this pull request Oct 26, 2026
Merged
@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@mjudeikis mjudeikis changed the title Add APIIdentity [1- De-Rooting] Add APIIdentity Oct 26, 2026
@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

So this as structural problem due to kcp-dev/enhancements#19
I think I might try to move APIIdentity to be "virtual, compute on-the-fly" resource so its integrates nicer into this. Else we gonna have bit of linear replication issues

@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

@mjudeikis-bot review?

@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

flake #4473
/retest

@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

/retest

ntnn
ntnn reviewed Oct 26, 2026
View reviewed changes
Comment thread pkg/admission/apiexport/admission.go
Comment thread pkg/virtual/admin/builder/build.go
provider := &adminAPIDefinitionSetProvider{
config: mainConfig,
cacheDynamicClusterClient: cacheDynamicClusterClient,
identities: identities,

@ntnn ntnn Oct 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That makes me think - should the admin VW only report readiness once the relevant informers have synced?
Otherwise e.g. APIIdentty(<my-hash>) returns nothing during boot even though there may be APIExports with <my-hash>, the VW just doesn't report them yet since the informer hasn't synced. And IIRC most informers are started as part of the post start hook.

@mjudeikis mjudeikis Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread pkg/reconciler/apis/apiexport/apiexport_reconcile_identity.go Outdated
Comment thread staging/src/github.com/kcp-dev/sdk/apis/admin/v1alpha1/types_apiidentity.go Outdated
@mjudeikis

mjudeikis commented Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

/retest

ntnn
ntnn approved these changes Oct 26, 2026
View reviewed changes

@ntnn ntnn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve

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

kcp-ci-bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: f8ef5e48d614916b69f60884ff35bdb31f629d11

@xrstf

xrstf commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

/approve

@kcp-ci-bot

kcp-ci-bot commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ntnn, xrstf

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

The pull request process is described 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

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 27, 2026
@kcp-ci-bot
kcp-ci-bot merged commit 75333fb into kcp-dev:main Oct 27, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntnn ntnn ntnn approved these changes
+1 more reviewer
@mjudeikis-bot mjudeikis-bot mjudeikis-bot left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@ntnn ntnn

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mjudeikis @kcp-ci-bot @xrstf @ntnn @mjudeikis-bot