Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

Drop blockOwnerDeletion on proxy-runner owned Services

Aperta
  • Qualcosa non va?
#6,836 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
28/100
Tipo di issue
Refactoring
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
go, kubernetes
Ambito
authorization, infrastructure

Direzione di ricerca

Start in pkg/container/kubernetes/client.go, at ensureBackendServices and the service apply helper, where BlockOwnerDeletion is set, then check the two assertions in client_test.go. The RBAC change touches defaultRBACRules and its kubebuilder marker, followed by regenerating role.yaml. The issue asks a maintainer to decide whether to drop the flag or keep the finalizers grant, so confirm that choice in the thread before editing. Done means the tests pass and role.yaml matches the chosen approach.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

kubernetes needs-triage operator

Proposal

Stop setting blockOwnerDeletion: true on the owner reference the proxy runner puts on the headless and ClusterIP Services pointing at their StatefulSet. Once that's done, drop the apps/statefulsets/finalizers grant that #6756 adds.

Background

On OpenShift, OwnerReferencesPermissionEnforcement rejects a blockOwnerDeletion: true owner reference unless the caller has update on the owner's /finalizers (#6835). #6756 fixes that by granting statefulsets/finalizers to the proxy-runner Role and to the operator ClusterRole. The operator needs it too because Kubernetes won't let it grant a permission it doesn't hold.

That's the standard fix, but it's RBAC we may not need. The flag only does something when the owner is deleted with foreground propagation: the StatefulSet would wait until its Services are gone. We never delete that way:

  • RemoveWorkload deletes the StatefulSet with empty DeleteOptions (background) in pkg/container/kubernetes/client.go.
  • finalizeMCPServer deletes the StatefulSet and both Services by name.
  • Nothing in production code sets PropagationPolicy or DeletePropagationForeground.

GC still removes the Services once the StatefulSet is gone, with or without the flag.

Change

  • pkg/container/kubernetes/client.go, ensureBackendServices: drop BlockOwnerDeletion: ptr.To(true) from the StatefulSet owner reference.
  • pkg/container/kubernetes/client.go, the service apply helper: drop the hardcoded WithBlockOwnerDeletion(true). It ignores whatever the owner passed in says anyway.
  • pkg/container/kubernetes/client_test.go: update the two assertions that expect true.
  • Revert the apps/statefulsets/finalizers entry in defaultRBACRules and its +kubebuilder:rbac marker, then regenerate role.yaml.

Why it's safe

  • Admission rules: the plugin only asks for the finalizers permission when an owner reference is newly blocking or flipped to blocking (source). Removing the flag, or leaving it out, never triggers that check. Any change to owner refs also needs delete on the object itself, and the proxy-runner Role already has delete on services.
  • Existing Services on vanilla clusters: the next Force apply drops the field. That passes even with the plugin on.
  • OpenShift: nothing to migrate, because Service creation has failed there since #3992.
  • Recreated StatefulSet (new UID): under B this needs no extra permission. Under the current grant approach it needs the finalizers permission every time.
  • Coverage: this is the only owner reference the proxy runner sets. The runner creates the StatefulSet with no owner, and the operator later adopts it under the MCPServer.

What B buys over the RBAC grant alone

  • Least privilege: with the grant, every MCPServer's runner service account can set blocking owner references on any StatefulSet in its namespace.
  • Mixed versions: a newer runner works against an older operator chart that lacks statefulsets/finalizers.

Related

  • Enable OwnerReferencesPermissionEnforcement in test/e2e/thv-operator/kind-config.yaml (apiServer.extraArgs). The chainsaw job in operator-ci.yml creates its kind cluster without a config and would need one too. This would have caught #3992, and it would prove B in CI. Ideally it lands with or before B.

Open question

@ChrisJBurns, what do you think? Is it worth dropping the flag and shrinking the RBAC footprint, or should we stick with the conventional /finalizers grant from #6756?

Refs #6756, #6835

Lingua principale
Go
Stelle
2.3k
Fork
318
Merge medio
2g 3h
PR unite (30g)
104

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di stacklok/toolhive

Tutte le issue di stacklok/toolhive

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.