Skip to content

Add pkg/shardlookup#4063

Merged
kcp-ci-bot merged 5 commits into
kcp-dev:mainfrom
ntnn:pkg-shardlookup
Apr 24, 2026
Merged

Add pkg/shardlookup#4063
kcp-ci-bot merged 5 commits into
kcp-dev:mainfrom
ntnn:pkg-shardlookup

Conversation

@ntnn
Copy link
Copy Markdown
Member

@ntnn ntnn commented Apr 23, 2026

Summary

Refactors the cache implemented for cross-shard SA lookup into pkg/shardlookup to be reused.

Related: #4061

What Type of PR Is This?

/kind cleanup

Related Issue(s)

Fixes #

Release Notes

NONE

@kcp-ci-bot kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. dco-signoff: yes Indicates the PR's author has signed the DCO. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 23, 2026
@ntnn ntnn added this to tbd Apr 23, 2026
@ntnn ntnn moved this to In review in tbd Apr 23, 2026
@ntnn ntnn requested a review from Copilot April 24, 2026 07:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extracts the shard-local vs remote-shard lookup and request coalescing logic (previously embedded in the serviceaccount token getter cache) into a new reusable pkg/shardlookup package, and updates the serviceaccount cache to use the new primitives.

Changes:

  • Added pkg/shardlookup.TTLCache (TTL + singleflight request coalescing) and pkg/shardlookup.Lookup (local-or-fetch wrapper).
  • Migrated pkg/server/serviceaccount/cache.go to use the new shardlookup abstractions.
  • Removed the serviceaccount-specific coalescing_cache.go implementation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/shardlookup/ttl_cache.go Introduces a generic TTL + singleflight cache used for cross-shard lookups.
pkg/shardlookup/lookup.go Adds a reusable “local lister vs remote fetch” lookup wrapper keyed by cluster-aware keys.
pkg/shardlookup/doc.go Provides package-level guidance on when to use shardlookup vs the cache server.
pkg/server/serviceaccount/cache.go Switches SA/secret lookup to the new shardlookup cache + lookup wrappers.
pkg/server/serviceaccount/coalescing_cache.go Deletes the previous serviceaccount-specific coalescing cache implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/shardlookup/ttl_cache.go Outdated
Comment thread pkg/shardlookup/ttl_cache.go
@ntnn ntnn force-pushed the pkg-shardlookup branch from 9f8bc2c to 74a33fb Compare April 24, 2026 07:47
Copy link
Copy Markdown
Contributor

@mjudeikis mjudeikis left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Im not sure about naming of the packages but we can change this later. Or never :D

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 24, 2026
@kcp-ci-bot
Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: bfd0cd9aa6895135c412ce254f7c5b58d21587bc

@kcp-ci-bot
Copy link
Copy Markdown
Contributor

[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

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 Apr 24, 2026
@ntnn
Copy link
Copy Markdown
Member Author

ntnn commented Apr 24, 2026

Im not sure about naming of the packages

You and me both :D

@ntnn
Copy link
Copy Markdown
Member Author

ntnn commented Apr 24, 2026

/cherry-pick 0.31

@kcp-ci-bot
Copy link
Copy Markdown
Contributor

@ntnn: once the present PR merges, I will cherry-pick it on top of 0.31 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick 0.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@kcp-ci-bot kcp-ci-bot merged commit 31d5030 into kcp-dev:main Apr 24, 2026
14 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in tbd Apr 24, 2026
@ntnn ntnn deleted the pkg-shardlookup branch April 24, 2026 08:40
@kcp-ci-bot
Copy link
Copy Markdown
Contributor

@ntnn: cannot checkout 0.31: error checking out "0.31": exit status 1 error: pathspec '0.31' did not match any file(s) known to git

Details

In response to this:

/cherry-pick 0.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ntnn
Copy link
Copy Markdown
Member Author

ntnn commented Apr 24, 2026

/cherry-pick release-0.31

@kcp-ci-bot
Copy link
Copy Markdown
Contributor

@ntnn: new pull request created: #4065

Details

In response to this:

/cherry-pick release-0.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

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

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/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants