Fix GC uid races and handling of conflicting ownerReferences

[liggitt]

Addresses issues with race conditions encountered when observing invalid ownerReferences

Fixes #65200

• address review comments
• add comments to each commit on the scenario being addresses
• add unit tests for GC controller and graph builder fixes
• ensure the coverage of the following testcases are represented:

---

• child in namespace A with owner reference to namespaced type in namespace B

  • should be deleted immediately
  • event should be logged in namespace A with involvedObject of bad-child indicating the error
  • test both orders:
    • real parent added to graph, then bad child added to graph
    • bad child added to graph, virtual parent added to graph, then real parent added to graph

• child that is cluster-scoped with owner reference to namespaced type in namespace B

  • should not be deleted
  • event should be logged in namespace kube-system with involvedObject of bad-child indicating the error
  • test both orders:
    • real parent added to graph, then bad child added to graph
    • bad child added to graph, virtual parent added to graph, then real parent added to graph

• child pointing at non-preferred still-served apiVersion of parent object (e.g. rbac/v1beta1)

  • should not be deleted prematurely
  • should not repeatedly poll attemptToDelete while waiting
  • should be deleted when the actual parent is deleted

• child pointing at no-longer-served apiVersion of still-existing parent object (e.g. extensions/v1beta1 deployment)

  • should not be deleted (this is indistinguishable from referencing an unknown kind/version)
  • should not repeatedly poll attemptToDelete once real parent is observed
  • TODO: follow up: should be fixed up by a controller adjusting known ownerReference API versions

• child pointing at no-longer-served apiVersion of no-longer-existing parent object (e.g. extensions/v1beta1 deployment)

  • should not be deleted (this is indistinguishable from referencing an unknown kind/version)
  • should repeatedly poll attemptToDelete
  • should not block deletion of legitimate children of missing deployment
  • TODO: follow up: should be fixed up by a controller adjusting known ownerReference API versions and once fixed up, should be deleted since parent does not exist

• child pointing at incorrect apiVersion/kind of still-existing parent object (e.g. core/v1 Secret with uid=123, where an apps/v1 Deployment with uid=123 exists)

  • should be deleted immediately
  • should not trigger deletion of legitimate children of parent

---

Follow-ups:

• add a controller/mechanism to migrate apiVersions of ownerReferences for known kubernetes types that have graduated or moved API groups (this is a pre-existing problem) - tracked in Add migration of ownerReferences that refer to deprecated/no-longer-served Kubernetes API versions #96650
• before 1.20 release - write a tool that scans items in the cluster and identifies invalid ownerReferences that would be impacted by this change, update the release note to describe running that tool on a 1.19 cluster to determine if there is existing bad data - https://github.com/kubernetes-sigs/kubectl-check-ownerreferences

/kind bug
/cc @jpbetz @deads2k
/sig api-machinery

Resolves non-deterministic behavior of the garbage collection controller when ownerReferences with incorrect data are encountered. Events with a reason of `OwnerRefInvalidNamespace` are recorded when namespace mismatches between child and owner objects are detected. The [kubectl-check-ownerreferences](https://github.com/kubernetes-sigs/kubectl-check-ownerreferences) tool can be run prior to upgrading to locate existing objects with invalid ownerReferences.
* A namespaced object with an ownerReference referencing a uid of a namespaced kind which does not exist in the same namespace is now consistently treated as though that owner does not exist, and the child object is deleted.
* A cluster-scoped object with an ownerReference referencing a uid of a namespaced kind is now consistently treated as though that owner is not resolvable, and the child object is ignored by the garbage collector.

[liggitt]

/retest

[liggitt]

/skip

[fedebongio]

/assign @jpbetz
/cc @caesarxuchao

[liggitt]

/retest

[mattfarina]

/retest pull-kubernetes-conformance-kind-ga-only-parallel

[mattfarina]

/test pull-kubernetes-conformance-kind-ga-only-parallel

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

[liggitt]

docs update for 1.20 at kubernetes/website#25091

[krmayankk]

curious why did we make the cross namespace reference an invalid case ?

[liggitt]

why did we make the cross namespace reference an invalid case ?

namespaces are intended to be independent of each other, so cross-namespace references have not been permitted in things like ownerReferences, secret/configmap volume references, etc.

additionally, granting permissions to namespace a is not generally intended to provide visibility or ability to interact with objects from namespace b (or cause system controllers to interact with objects from namespace b).

[krmayankk]

why did we make the cross namespace reference an invalid case ?

namespaces are intended to be independent of each other, so cross-namespace references have not been permitted in things like ownerReferences, secret/configmap volume references, etc.

additionally, granting permissions to namespace a is not generally intended to provide visibility or ability to interact with objects from namespace b (or cause system controllers to interact with objects from namespace b).

thanks

[little-guy-lxr]

could this fix merge to 1.19.x ?

[liggitt]

could this fix merge to 1.19.x ?

No, this is a significant enough rewrite of the garbagecontroller that it is limited to 1.20+

For clusters on previous versions, I would recommend running https://github.com/kubernetes-sigs/kubectl-check-ownerreferences tool is available to identify invalid ownerReferences that could trigger the bug this PR is fixing so they can be eliminated.