Skip to content

Scope the pod controller's ownedPod predicate to the operator namespace #477

Description

@ayuskauskas

Follow-up from the CodeRabbit review on #459 (Jobs migration). Not a live bug — hardening — so it was split out rather than folded into that PR.

What

ownedPod() in operator/internal/controller/pod_controller.go gates only on the presence of the <prefix>/name label:

func ownedPod() predicate.Predicate {
	return predicate.NewPredicateFuncs(func(o client.Object) bool {
		return labels.Set(o.GetLabels()).Has(fmt.Sprintf("%s/name", v1alpha1.METADATA_PREFIX))
	})
}

Its doc comment says "so unrelated pods in the namespace never enter the workqueue", but the Pod cache is deliberately cluster-wide (node drain has to see workload pods on any node). So the namespace assumption in the comment does not hold: a pod in any namespace that carries a copied <prefix>/name label passes the predicate, enters the workqueue, and can drive node-state writes.

Suggested change

  • Require both <prefix>/name and <prefix>/package rather than just name.
  • Pass options.Namespace into NewPodReconciler and have the predicate reject pods outside it.
  • Update the existing ownedPod() test call sites.
  • Fix the doc comment, which currently asserts a namespace guarantee the predicate does not provide.

Notes

  • Writer and reader agree on the label prefix today (job_builder.go stamps the same v1alpha1.METADATA_PREFIX keys), so this is purely about scope, not a label mismatch.
  • Deciding whether cross-namespace label spoofing is in scope for the threat model is part of this issue; if it is not, the minimal outcome is correcting the misleading comment.

Metadata

Metadata

Assignees

No one assigned

    Labels

    component/operatorSkyhook operator (controller-manager)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions