backend: k8cache: percent-encode '+' in cache key fields to avoid delimiter collision#6127
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sid200727 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @sid200727! |
…imiter collision GenerateKey joins apiGroup, kind, namespace, and contextID with '+' as a delimiter, and DeleteKeys later splits on '+' to strip the namespace via positional indexing. Kubeconfig context names are not restricted by Kubernetes naming rules and may legitimately contain '+', which produces a key with more than 4 segments and breaks the positional namespace stripping in DeleteKeys. This can cause cache invalidation to silently target the wrong key, leaving stale cached API responses served after a write. Escape literal '+' to '%2B' in each field before joining so the only '+' characters remaining in the key are the delimiters themselves. Related to kubernetes-sigs#6123 Signed-off-by: Siddhi Khandelwal <siddhi.200727@gmail.com>
64f322a to
6086264
Compare
Summary
This PR fixes a cache-key collision bug by percent-encoding any literal
+character in the fields used to build k8cache keys, so the+delimiter can no longer be ambiguous with field content.Related Issue
Fixes #6123
Changes
-> Updated
GenerateKeyinbackend/pkg/k8cache/cacheStore.goto escape literal+to%2BinapiGroup,kind,namespace, andcontextIDbefore joining them with+to form the cache key-> Added a regression test case to
TestGenerateKeyinbackend/pkg/k8cache/cacheStore_test.gocovering a context name containing+Steps to Test
+, e.g.prod+clusterGenerateKeywith a request URL and that context name+-delimited segments (e.g.apps+deployments+default+prod+cluster), which breaksDeleteKeys's positional namespace-stripping (keyPart[2] = "") andcan cause cache invalidation to target the wrong key+in the context name is escaped to%2B(apps+deployments+default+prod%2Bcluster), so the key always has exactly 4 segments andDeleteKeysparses it correctlygo test ./pkg/k8cache/...- all existing tests pass unchanged, plus the new regression testScreenshots (if applicable)
N/A (backend logic fix, no UI changes)
Notes for the Reviewer
->
DeleteKeysitself needs no changes, the fix is fully contained inGenerateKey, since it now guarantees the only+characters in a key are the delimiters-> No existing test fixtures use
+in any field, so this change is fully backward compatible->
gofmt,go vet, andnpm run backend:lintall pass clean