Repository navigation
Build impersonated client without the shared cache - #652
Closed
ArnobKumarSaha wants to merge 1 commit into
Closed
ArnobKumarSaha wants to merge 1 commit into
ArnobKumarSaha wants to merge 1 commit into
Conversation
DelegatingClient.Impersonate reused the base client's options, including the cache reader. The cache is populated with the base client's credentials, so any cachable typed Get/List through an impersonated client was served from that cache without the impersonation headers, bypassing the impersonated user's authorization. Drop the cache from the options passed to the impersonated client so it reads directly from the API, and treat a nil CacheReader as always-bypass in the delegating reader. Signed-off-by: Arnob Kumar Saha <arnob@appscode.com>
Member
Author
|
Superseding in the consumer (kubeops/ui-server) with a cache + per-object SubjectAccessReview approach, so Impersonate does not need to become cache-free for all consumers. Making impersonated clients unconditionally bypass the shared informer cache would regress consumers that do typed reads via Impersonate and rely on the cache. Closing in favor of authorizing cached reads per object at the call site. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
DelegatingClient.Impersonate(u)built the impersonated client from the base client's options, which carry the shared cache reader (optionsShallowCopy := d.options). That cache is populated with the base client's credentials.delegatingReader.shouldBypassCacheonly bypasses the cache for non-cachable GVKs, explicitly uncached GVKs, or unstructured objects (whencacheUnstructuredis false). So a cachable, typedGet/Listthrough an impersonated client was served from the base-credentialed cache, without the impersonation headers — silently bypassing the impersonated user's authorization.Unstructured reads already went direct, so callers that only read unstructured objects were unaffected; typed reads were the gap.
Fix
Impersonate: setoptionsShallowCopy.Cache = nilso the impersonated client is built without a cache and reads directly from the API (impersonation is already set on the copied rest config).delegatingReader.shouldBypassCache: returntruewhenCacheReader == nil, so a cache-free delegating client routes every read to the directClientReaderinstead of dereferencing a nil cache.Impact
Impersonated clients now always read as the impersonated user. Non-impersonated clients are unchanged. Downstream (e.g. kubeops/ui-server render/dashboard/graph endpoints) can rely on
Impersonatefor authorization without restricting itself to unstructured reads.