Fix forceLiveLookupCache key in namespace lifecycle admission (k8s 1.37.0 update)

Open
  • Is something wrong?
#4,522 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
55/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
go, kubernetes
Domain
backend, testing

Research direction

Read staging/src/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle/admission.go and the lifecycle plugin's unit tests. Check how the namespace delete stores its key and how object creation reads it; then add the requested regression coverage for a Pod in a terminating namespace and cluster scoping. Done means the lookup uses the object's namespace, the tests reject the create after the live lookup, and the kcp patch and 1.37.0 branch are updated.

Written by the indexing model from the issue text.

Description

What exists today

In the kcp Kubernetes fork update to 1.37.0 (https://github.com/kcp-dev/kubernetes/pull/198), the namespace lifecycle admission plugin builds one key and uses it for both the store and the lookup of forceLiveLookupCache:

staging/src/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle/admission.go (head 5ec79958933dfa6999e77eab9e90b691e26fbcd5)

  • line 93: namespaceKey := kcpcache.ToClusterAwareKey(clusterName.String(), "", a.GetName())
  • line 101: on a Namespace delete, l.forceLiveLookupCache.Add(namespaceKey, true, forceLiveLookupTTL). Here a.GetName() is the namespace name, so this is correct.
  • line 160: l.forceLiveLookupCache.Get(namespaceKey). On this path a is the namespaced object being created (e.g. a Pod), so a.GetName() is the object name, not the namespace name.

The lookup key is cluster|<objectname> but the stored key is cluster|<namespacename>, so the lookup effectively never matches. Upstream reads a.GetNamespace() here.

Impact

The "recently deleted namespace, force a live lookup" guard never fires. When the informer cache is stale (the namespace is still Active locally after a delete), creates into a terminating namespace can be admitted. A match is only possible by accident, when an object's name equals the deleted namespace's name.

What should replace it

Use a separate key for the lookup, built from the object's namespace:

liveKey := kcpcache.ToClusterAwareKey(clusterName.String(), "", a.GetNamespace())
if _, ok := l.forceLiveLookupCache.Get(liveKey); ok {

Keep namespaceKey (from a.GetName()) for the Namespace-delete store at line 101.

Checklist

  • Split the key: keep a.GetName() for the store at line 101 and use a.GetNamespace() for the read at line 160.
  • Add a unit test in the lifecycle plugin: delete namespace foo in a cluster, keep the lister showing foo as Active, then create a Pod named bar in foo. The create must go through the live lookup and be rejected once foo is Terminating.
  • Cover cluster scoping in that test: a delete in cluster A must not force a live lookup in cluster B.
  • Update the kcp patch in https://github.com/kcp-dev/kubernetes-patches/pull/2 and the 1.37.0 branch accordingly.

References

Dominant language
Go
Stars
2.9k
Forks
471
Avg merge
2d 13h
Merged PRs (30d)
77

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from kcp-dev/kcp

All issues in kcp-dev/kcp

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.