From 0210ab72f121d70a9cc0398364f9a6ecb198768d Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 22 Jun 2026 14:14:27 +0200 Subject: [PATCH 1/8] fix: narrow down rbacs and pin to resource names Reduce the permission range by pinning RBAC rules to the object names we actually work with. AI-attribution: AIA PAI Ce Hin R claude-4.6-opus-1M v1.0 Signed-off-by: Francesco Romani --- ...ources-operator.clusterserviceversion.yaml | 169 +++++++++++++++++- config/rbac/role.yaml | 169 +++++++++++++++++- .../controller/kubeletconfig_controller.go | 2 +- .../numaresourcesoperator_controller.go | 29 +-- .../numaresourcesscheduler_controller.go | 19 +- 5 files changed, 352 insertions(+), 36 deletions(-) diff --git a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml index a62cb30d5c..512dff87cc 100644 --- a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml +++ b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml @@ -382,9 +382,13 @@ spec: - "" resources: - configmaps - - serviceaccounts verbs: - - '*' + - create + - delete + - get + - list + - update + - watch - apiGroups: - "" resources: @@ -407,19 +411,77 @@ spec: - get - list - watch + - apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - list + - watch + - apiGroups: + - "" + resourceNames: + - rte + resources: + - serviceaccounts + verbs: + - get + - update + - apiGroups: + - "" + resourceNames: + - secondary-scheduler + resources: + - serviceaccounts + verbs: + - get + - update - apiGroups: - apiextensions.k8s.io resources: - customresourcedefinitions verbs: - - '*' + - create + - list + - watch + - apiGroups: + - apiextensions.k8s.io + resourceNames: + - noderesourcetopologies.topology.node.k8s.io + resources: + - customresourcedefinitions + verbs: + - get + - update - apiGroups: - apps resources: - daemonsets + verbs: + - create + - delete + - get + - list + - update + - watch + - apiGroups: + - apps + resources: - deployments verbs: - - '*' + - create + - list + - watch + - apiGroups: + - apps + resourceNames: + - secondary-scheduler + resources: + - deployments + verbs: + - get + - update - apiGroups: - config.openshift.io resources: @@ -461,7 +523,12 @@ spec: resources: - machineconfigs verbs: - - '*' + - create + - delete + - get + - list + - update + - watch - apiGroups: - nodetopology.openshift.io resources: @@ -487,6 +554,16 @@ spec: - get - patch - update + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - secondary-scheduler + - topology-aware-scheduler + resources: + - clusterrolebindings + verbs: + - get + - update - apiGroups: - rbac.authorization.k8s.io resources: @@ -495,13 +572,58 @@ spec: - rolebindings - roles verbs: - - '*' + - create + - list + - watch + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - clusterrolebindings + - clusterroles + - rolebindings + - roles + verbs: + - get + - update + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler + resources: + - clusterroles + verbs: + - get + - update + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler-leader-elect + resources: + - rolebindings + - roles + verbs: + - get + - update + - apiGroups: + - security.openshift.io + resources: + - securitycontextconstraints + verbs: + - create + - list + - watch - apiGroups: - security.openshift.io + resourceNames: + - resource-topology-exporter + - resource-topology-exporter-v2 resources: - securitycontextconstraints verbs: - - '*' + - get + - update - apiGroups: - topology.node.k8s.io resources: @@ -651,13 +773,42 @@ spec: resources: - services verbs: - - '*' + - create + - apiGroups: + - "" + resourceNames: + - numaresources-rte-metrics-service + resources: + - services + verbs: + - get + - update + - apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - create + - apiGroups: + - networking.k8s.io + resourceNames: + - rte-default-deny-all + - rte-egress-to-api-server + resources: + - networkpolicies + verbs: + - get + - update - apiGroups: - networking.k8s.io + resourceNames: + - scheduler-default-deny-all + - scheduler-egress-to-api-server resources: - networkpolicies verbs: - - '*' + - get + - update serviceAccountName: numaresources-controller-manager strategy: deployment installModes: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index b18c4080c2..5b4dd8d758 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -8,9 +8,13 @@ rules: - "" resources: - configmaps - - serviceaccounts verbs: - - '*' + - create + - delete + - get + - list + - update + - watch - apiGroups: - "" resources: @@ -33,19 +37,77 @@ rules: - get - list - watch +- apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - list + - watch +- apiGroups: + - "" + resourceNames: + - rte + resources: + - serviceaccounts + verbs: + - get + - update +- apiGroups: + - "" + resourceNames: + - secondary-scheduler + resources: + - serviceaccounts + verbs: + - get + - update - apiGroups: - apiextensions.k8s.io resources: - customresourcedefinitions verbs: - - '*' + - create + - list + - watch +- apiGroups: + - apiextensions.k8s.io + resourceNames: + - noderesourcetopologies.topology.node.k8s.io + resources: + - customresourcedefinitions + verbs: + - get + - update - apiGroups: - apps resources: - daemonsets + verbs: + - create + - delete + - get + - list + - update + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - list + - watch +- apiGroups: + - apps + resourceNames: + - secondary-scheduler + resources: - deployments verbs: - - '*' + - get + - update - apiGroups: - config.openshift.io resources: @@ -87,7 +149,12 @@ rules: resources: - machineconfigs verbs: - - '*' + - create + - delete + - get + - list + - update + - watch - apiGroups: - nodetopology.openshift.io resources: @@ -113,6 +180,16 @@ rules: - get - patch - update +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - secondary-scheduler + - topology-aware-scheduler + resources: + - clusterrolebindings + verbs: + - get + - update - apiGroups: - rbac.authorization.k8s.io resources: @@ -121,13 +198,58 @@ rules: - rolebindings - roles verbs: - - '*' + - create + - list + - watch +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - clusterrolebindings + - clusterroles + - rolebindings + - roles + verbs: + - get + - update +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler + resources: + - clusterroles + verbs: + - get + - update +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler-leader-elect + resources: + - rolebindings + - roles + verbs: + - get + - update - apiGroups: - security.openshift.io resources: - securitycontextconstraints verbs: - - '*' + - create + - list + - watch +- apiGroups: + - security.openshift.io + resourceNames: + - resource-topology-exporter + - resource-topology-exporter-v2 + resources: + - securitycontextconstraints + verbs: + - get + - update - apiGroups: - topology.node.k8s.io resources: @@ -149,10 +271,39 @@ rules: resources: - services verbs: - - '*' + - create +- apiGroups: + - "" + resourceNames: + - numaresources-rte-metrics-service + resources: + - services + verbs: + - get + - update +- apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - create +- apiGroups: + - networking.k8s.io + resourceNames: + - rte-default-deny-all + - rte-egress-to-api-server + resources: + - networkpolicies + verbs: + - get + - update - apiGroups: - networking.k8s.io + resourceNames: + - scheduler-default-deny-all + - scheduler-egress-to-api-server resources: - networkpolicies verbs: - - '*' + - get + - update diff --git a/internal/controller/kubeletconfig_controller.go b/internal/controller/kubeletconfig_controller.go index d97c2e8860..c459521aeb 100644 --- a/internal/controller/kubeletconfig_controller.go +++ b/internal/controller/kubeletconfig_controller.go @@ -90,7 +90,7 @@ type kubeletConfigHandler struct { // Namespace Scoped // Cluster Scoped -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=* +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;get;list;update;watch //+kubebuilder:rbac:groups="",resources=events,verbs=create;patch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs/finalizers,verbs=update diff --git a/internal/controller/numaresourcesoperator_controller.go b/internal/controller/numaresourcesoperator_controller.go index ce4463d7d4..0d31e0bb24 100644 --- a/internal/controller/numaresourcesoperator_controller.go +++ b/internal/controller/numaresourcesoperator_controller.go @@ -109,24 +109,31 @@ type NUMAResourcesOperatorReconciler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups="",resources=services,verbs=*,namespace="numaresources" -//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=*,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=services,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=services,resourceNames=numaresources-rte-metrics-service,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,resourceNames=rte-default-deny-all;rte-egress-to-api-server,verbs=get;update,namespace="numaresources" // Cluster Scoped //+kubebuilder:rbac:groups=config.openshift.io,resources=apiservers,verbs=get;list;watch //+kubebuilder:rbac:groups=config.openshift.io,resources=clusterversions,verbs=list //+kubebuilder:rbac:groups=config.openshift.io,resources=clusteroperators,verbs=get //+kubebuilder:rbac:groups=config.openshift.io,resources=infrastructures,verbs=get -//+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=machineconfigs,verbs=* +//+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=machineconfigs,verbs=create;delete;get;list;update;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=machineconfigpools,verbs=get;list;watch -//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=* -//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=* -//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=* -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=* +//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=create;list;watch +//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=resource-topology-exporter;resource-topology-exporter-v2,verbs=get;update +//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=create;list;watch +//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,resourceNames=noderesourcetopologies.topology.node.k8s.io,verbs=get;update +//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;update;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create;list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=rte,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=rte,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=rte,verbs=get;update +//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch +//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=rte,verbs=get;update //+kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=nodes,verbs=list //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesoperators,verbs=get;list;watch diff --git a/internal/controller/numaresourcesscheduler_controller.go b/internal/controller/numaresourcesscheduler_controller.go index 8b6588fe15..55f98a790f 100644 --- a/internal/controller/numaresourcesscheduler_controller.go +++ b/internal/controller/numaresourcesscheduler_controller.go @@ -78,14 +78,21 @@ type NUMAResourcesSchedulerReconciler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=*,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,resourceNames=scheduler-default-deny-all;scheduler-egress-to-api-server,verbs=get;update,namespace="numaresources" // Cluster Scoped -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=* -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=* -//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=* -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=* -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=* +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create;list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=topology-aware-scheduler,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create;list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=secondary-scheduler;topology-aware-scheduler,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=topology-aware-scheduler-leader-elect,verbs=get;update +//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;list;watch +//+kubebuilder:rbac:groups=apps,resources=deployments,resourceNames=secondary-scheduler,verbs=get;update +//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch +//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=secondary-scheduler,verbs=get;update +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;get;list;update;watch //+kubebuilder:rbac:groups="",resources=nodes,verbs=list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/status,verbs=get;update;patch From 790548d482f76b04cdc67d765bf3df0251c9c732 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 22 Jun 2026 14:54:57 +0200 Subject: [PATCH 2/8] rbac: narrow down permissions clusterscope -> namespace scope quite some permissions we need are actually namespaced scope; narrow down accordingly AI-attribution: AIA PAI Ce Hin R claude-4.6-opus-1M v1.0 Signed-off-by: Francesco Romani --- ...ources-operator.clusterserviceversion.yaml | 184 ++++++++++-------- cmd/main.go | 14 +- config/rbac/role.yaml | 184 ++++++++++-------- .../controller/kubeletconfig_controller.go | 4 +- .../numaresourcesoperator_controller.go | 14 +- .../numaresourcesscheduler_controller.go | 16 +- 6 files changed, 222 insertions(+), 194 deletions(-) diff --git a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml index 512dff87cc..119e0eab67 100644 --- a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml +++ b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml @@ -378,24 +378,6 @@ spec: spec: clusterPermissions: - rules: - - apiGroups: - - "" - resources: - - configmaps - verbs: - - create - - delete - - get - - list - - update - - watch - - apiGroups: - - "" - resources: - - events - verbs: - - create - - patch - apiGroups: - "" resources: @@ -411,32 +393,6 @@ spec: - get - list - watch - - apiGroups: - - "" - resources: - - serviceaccounts - verbs: - - create - - list - - watch - - apiGroups: - - "" - resourceNames: - - rte - resources: - - serviceaccounts - verbs: - - get - - update - - apiGroups: - - "" - resourceNames: - - secondary-scheduler - resources: - - serviceaccounts - verbs: - - get - - update - apiGroups: - apiextensions.k8s.io resources: @@ -454,34 +410,6 @@ spec: verbs: - get - update - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - create - - delete - - get - - list - - update - - watch - - apiGroups: - - apps - resources: - - deployments - verbs: - - create - - list - - watch - - apiGroups: - - apps - resourceNames: - - secondary-scheduler - resources: - - deployments - verbs: - - get - - update - apiGroups: - config.openshift.io resources: @@ -569,8 +497,6 @@ spec: resources: - clusterrolebindings - clusterroles - - rolebindings - - roles verbs: - create - list @@ -582,8 +508,6 @@ spec: resources: - clusterrolebindings - clusterroles - - rolebindings - - roles verbs: - get - update @@ -596,16 +520,6 @@ spec: verbs: - get - update - - apiGroups: - - rbac.authorization.k8s.io - resourceNames: - - topology-aware-scheduler-leader-elect - resources: - - rolebindings - - roles - verbs: - - get - - update - apiGroups: - security.openshift.io resources: @@ -771,9 +685,48 @@ spec: - apiGroups: - "" resources: + - configmaps + verbs: + - create + - delete + - get + - list + - update + - watch + - apiGroups: + - "" + resources: + - events + verbs: + - create + - patch + - apiGroups: + - "" + resourceNames: + - rte + resources: + - serviceaccounts + verbs: + - get + - update + - apiGroups: + - "" + resourceNames: + - secondary-scheduler + resources: + - serviceaccounts + verbs: + - get + - update + - apiGroups: + - "" + resources: + - serviceaccounts - services verbs: - create + - list + - watch - apiGroups: - "" resourceNames: @@ -783,12 +736,42 @@ spec: verbs: - get - update + - apiGroups: + - apps + resources: + - daemonsets + verbs: + - create + - delete + - get + - list + - update + - watch + - apiGroups: + - apps + resources: + - deployments + verbs: + - create + - list + - watch + - apiGroups: + - apps + resourceNames: + - secondary-scheduler + resources: + - deployments + verbs: + - get + - update - apiGroups: - networking.k8s.io resources: - networkpolicies verbs: - create + - list + - watch - apiGroups: - networking.k8s.io resourceNames: @@ -809,6 +792,35 @@ spec: verbs: - get - update + - apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - list + - watch + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - rolebindings + - roles + verbs: + - get + - update + - apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler-leader-elect + resources: + - rolebindings + - roles + verbs: + - get + - update serviceAccountName: numaresources-controller-manager strategy: deployment installModes: diff --git a/cmd/main.go b/cmd/main.go index 10aa384cf6..da89b962c2 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -38,7 +38,6 @@ import ( securityv1 "github.com/openshift/api/security/v1" ctrltls "github.com/openshift/controller-runtime-common/pkg/tls" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -289,6 +288,7 @@ func main() { if !ok { namespace = defaultNamespace } + klog.InfoS("namespace detected", "namespace", namespace) rteManifests, err := rtemanifests.GetManifests(discoveredCluster.Platform, discoveredCluster.ShortVersion, namespace, false, true) if err != nil { @@ -340,12 +340,16 @@ func main() { webhookTLSOpts := append(webhookTLSOpts(params.enableHTTP2), tlsConfig) + cacheNamespaces := map[string]cache.Config{ + namespace: {}, + } + if discoveredCluster.Platform == platform.HyperShift { + cacheNamespaces[controller.HyperShiftKubeletConfigConfigMapNamespace] = cache.Config{} + } + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Cache: cache.Options{ - DefaultNamespaces: map[string]cache.Config{ - namespace: {}, - metav1.NamespaceNone: {}, - }, + DefaultNamespaces: cacheNamespaces, }, Scheme: scheme, Metrics: metricsserver.Options{ diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 5b4dd8d758..d95022a834 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -4,24 +4,6 @@ kind: ClusterRole metadata: name: manager-role rules: -- apiGroups: - - "" - resources: - - configmaps - verbs: - - create - - delete - - get - - list - - update - - watch -- apiGroups: - - "" - resources: - - events - verbs: - - create - - patch - apiGroups: - "" resources: @@ -37,32 +19,6 @@ rules: - get - list - watch -- apiGroups: - - "" - resources: - - serviceaccounts - verbs: - - create - - list - - watch -- apiGroups: - - "" - resourceNames: - - rte - resources: - - serviceaccounts - verbs: - - get - - update -- apiGroups: - - "" - resourceNames: - - secondary-scheduler - resources: - - serviceaccounts - verbs: - - get - - update - apiGroups: - apiextensions.k8s.io resources: @@ -80,34 +36,6 @@ rules: verbs: - get - update -- apiGroups: - - apps - resources: - - daemonsets - verbs: - - create - - delete - - get - - list - - update - - watch -- apiGroups: - - apps - resources: - - deployments - verbs: - - create - - list - - watch -- apiGroups: - - apps - resourceNames: - - secondary-scheduler - resources: - - deployments - verbs: - - get - - update - apiGroups: - config.openshift.io resources: @@ -195,8 +123,6 @@ rules: resources: - clusterrolebindings - clusterroles - - rolebindings - - roles verbs: - create - list @@ -208,8 +134,6 @@ rules: resources: - clusterrolebindings - clusterroles - - rolebindings - - roles verbs: - get - update @@ -222,16 +146,6 @@ rules: verbs: - get - update -- apiGroups: - - rbac.authorization.k8s.io - resourceNames: - - topology-aware-scheduler-leader-elect - resources: - - rolebindings - - roles - verbs: - - get - - update - apiGroups: - security.openshift.io resources: @@ -269,9 +183,48 @@ rules: - apiGroups: - "" resources: + - configmaps + verbs: + - create + - delete + - get + - list + - update + - watch +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +- apiGroups: + - "" + resourceNames: + - rte + resources: + - serviceaccounts + verbs: + - get + - update +- apiGroups: + - "" + resourceNames: + - secondary-scheduler + resources: + - serviceaccounts + verbs: + - get + - update +- apiGroups: + - "" + resources: + - serviceaccounts - services verbs: - create + - list + - watch - apiGroups: - "" resourceNames: @@ -281,12 +234,42 @@ rules: verbs: - get - update +- apiGroups: + - apps + resources: + - daemonsets + verbs: + - create + - delete + - get + - list + - update + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - list + - watch +- apiGroups: + - apps + resourceNames: + - secondary-scheduler + resources: + - deployments + verbs: + - get + - update - apiGroups: - networking.k8s.io resources: - networkpolicies verbs: - create + - list + - watch - apiGroups: - networking.k8s.io resourceNames: @@ -307,3 +290,32 @@ rules: verbs: - get - update +- apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - list + - watch +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - rte + resources: + - rolebindings + - roles + verbs: + - get + - update +- apiGroups: + - rbac.authorization.k8s.io + resourceNames: + - topology-aware-scheduler-leader-elect + resources: + - rolebindings + - roles + verbs: + - get + - update diff --git a/internal/controller/kubeletconfig_controller.go b/internal/controller/kubeletconfig_controller.go index c459521aeb..1284e0f014 100644 --- a/internal/controller/kubeletconfig_controller.go +++ b/internal/controller/kubeletconfig_controller.go @@ -88,10 +88,10 @@ type kubeletConfigHandler struct { } // Namespace Scoped +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;get;list;update;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch,namespace="numaresources" // Cluster Scoped -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;get;list;update;watch -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs/finalizers,verbs=update diff --git a/internal/controller/numaresourcesoperator_controller.go b/internal/controller/numaresourcesoperator_controller.go index 0d31e0bb24..4dd207d6f6 100644 --- a/internal/controller/numaresourcesoperator_controller.go +++ b/internal/controller/numaresourcesoperator_controller.go @@ -109,10 +109,15 @@ type NUMAResourcesOperatorReconciler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups="",resources=services,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=services,verbs=create;list;watch,namespace="numaresources" //+kubebuilder:rbac:groups="",resources=services,resourceNames=numaresources-rte-metrics-service,verbs=get;update,namespace="numaresources" -//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create;list;watch,namespace="numaresources" //+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,resourceNames=rte-default-deny-all;rte-egress-to-api-server,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;update;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=rte,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=rte,verbs=get;update,namespace="numaresources" // Cluster Scoped //+kubebuilder:rbac:groups=config.openshift.io,resources=apiservers,verbs=get;list;watch @@ -125,15 +130,10 @@ type NUMAResourcesOperatorReconciler struct { //+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=resource-topology-exporter;resource-topology-exporter-v2,verbs=get;update //+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=create;list;watch //+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,resourceNames=noderesourcetopologies.topology.node.k8s.io,verbs=get;update -//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=create;delete;get;list;update;watch -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create;list;watch -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=rte,verbs=get;update //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=rte,verbs=get;update //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=rte,verbs=get;update -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch -//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=rte,verbs=get;update //+kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=nodes,verbs=list //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesoperators,verbs=get;list;watch diff --git a/internal/controller/numaresourcesscheduler_controller.go b/internal/controller/numaresourcesscheduler_controller.go index 55f98a790f..e4e1004a7d 100644 --- a/internal/controller/numaresourcesscheduler_controller.go +++ b/internal/controller/numaresourcesscheduler_controller.go @@ -78,21 +78,21 @@ type NUMAResourcesSchedulerReconciler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=create;list;watch,namespace="numaresources" //+kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,resourceNames=scheduler-default-deny-all;scheduler-egress-to-api-server,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=topology-aware-scheduler-leader-elect,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups=apps,resources=deployments,resourceNames=secondary-scheduler,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=secondary-scheduler,verbs=get;update,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;get;list;update;watch,namespace="numaresources" // Cluster Scoped //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create;list;watch //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=topology-aware-scheduler,verbs=get;update //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create;list;watch //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=secondary-scheduler;topology-aware-scheduler,verbs=get;update -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=create -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,resourceNames=topology-aware-scheduler-leader-elect,verbs=get;update -//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=create;list;watch -//+kubebuilder:rbac:groups=apps,resources=deployments,resourceNames=secondary-scheduler,verbs=get;update -//+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch -//+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=secondary-scheduler,verbs=get;update -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;get;list;update;watch //+kubebuilder:rbac:groups="",resources=nodes,verbs=list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/status,verbs=get;update;patch From a6332e2aa0926fb97f4f5a300ddf24a33324dde5 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 22 Jun 2026 14:54:57 +0200 Subject: [PATCH 3/8] controller: nrop: kubelet: remove events At the beginning of this project, we added events attempting to improve UX. But events are actually useful only if you either interactive watch them while the operator is doing stuff or if you store somewhere durable. So the usefulness of the events - especially how we use them, was already questionable. In addition, we need permissions on the `default` namespace, as byproduct of the machinery we use to send events. We can fix the latter with new faciities provided by the controller-runtime, but given our usage of events were already questionable in the first place, we take the chance to just remove them, reducing both the code and the RBAC footprint. AI-attribution: AIA PAI Ce Hin R claude-4.6-opus-1M v1.0 Signed-off-by: Francesco Romani --- ...ources-operator.clusterserviceversion.yaml | 7 --- cmd/main.go | 2 - config/rbac/role.yaml | 7 --- .../controller/kubeletconfig_controller.go | 10 +--- .../kubeletconfig_controller_test.go | 56 +------------------ .../numaresourcesoperator_controller.go | 16 +----- .../numaresourcesoperator_controller_test.go | 4 -- 7 files changed, 5 insertions(+), 97 deletions(-) diff --git a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml index 119e0eab67..f72a16258b 100644 --- a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml +++ b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml @@ -693,13 +693,6 @@ spec: - list - update - watch - - apiGroups: - - "" - resources: - - events - verbs: - - create - - patch - apiGroups: - "" resourceNames: diff --git a/cmd/main.go b/cmd/main.go index da89b962c2..57566d2d47 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -397,7 +397,6 @@ func main() { if err = (&controller.NUMAResourcesOperatorReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("numaresources-controller"), APIManifests: apiManifests, RTEManifests: rtestate.Manifests{ Core: rteManifestsRendered, @@ -417,7 +416,6 @@ func main() { if err = (&controller.KubeletConfigReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("kubeletconfig-controller"), Namespace: namespace, Platform: discoveredCluster.Platform, }).SetupWithManager(mgr); err != nil { diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index d95022a834..85875b088d 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -191,13 +191,6 @@ rules: - list - update - watch -- apiGroups: - - "" - resources: - - events - verbs: - - create - - patch - apiGroups: - "" resourceNames: diff --git a/internal/controller/kubeletconfig_controller.go b/internal/controller/kubeletconfig_controller.go index 1284e0f014..018197c1e3 100644 --- a/internal/controller/kubeletconfig_controller.go +++ b/internal/controller/kubeletconfig_controller.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" @@ -71,7 +70,6 @@ const ( type KubeletConfigReconciler struct { client.Client Scheme *runtime.Scheme - Recorder record.EventRecorder Namespace string Platform platform.Platform // garbageCollectionFor is a list of objects @@ -89,8 +87,6 @@ type kubeletConfigHandler struct { // Namespace Scoped //+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;get;list;update;watch,namespace="numaresources" -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch,namespace="numaresources" - // Cluster Scoped //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs/finalizers,verbs=update @@ -120,17 +116,15 @@ func (r *KubeletConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { var klErr *InvalidKubeletConfig if errors.As(err, &klErr) { - r.Recorder.Event(instance, "Normal", "ProcessSkip", "ignored kubelet config "+klErr.ObjectName) + klog.InfoS("ignored kubelet config", "object", klErr.ObjectName) return ctrl.Result{}, nil } klog.ErrorS(err, "failed to reconcile configmap", "controller", "kubeletconfig") - - r.Recorder.Event(instance, "Warning", "ProcessFailed", "Failed to update RTE config from kubelet config "+req.NamespacedName.String()) return ctrl.Result{}, err } - r.Recorder.Event(instance, "Normal", "ProcessOK", fmt.Sprintf("Updated RTE config %s/%s from kubelet config %s", cm.Namespace, cm.Name, req.NamespacedName.String())) + klog.InfoS("updated RTE config from kubelet config", "configmap", klog.KRef(cm.Namespace, cm.Name), "kubeletconfig", req.NamespacedName.String()) return ctrl.Result{}, nil } diff --git a/internal/controller/kubeletconfig_controller_test.go b/internal/controller/kubeletconfig_controller_test.go index fae8a52f72..2e682e11ff 100644 --- a/internal/controller/kubeletconfig_controller_test.go +++ b/internal/controller/kubeletconfig_controller_test.go @@ -23,7 +23,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -45,17 +44,12 @@ import ( type reconcilerBuilderFunc func(...runtime.Object) (*KubeletConfigReconciler, error) -const ( - bufferSize = 1024 -) - func NewFakeKubeletConfigReconciler(initObjects ...runtime.Object) (*KubeletConfigReconciler, error) { fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(initObjects...).Build() return &KubeletConfigReconciler{ Client: fakeClient, Scheme: scheme.Scheme, Namespace: testNamespace, - Recorder: record.NewFakeRecorder(bufferSize), Platform: platform.OpenShift, }, nil } @@ -66,7 +60,6 @@ func NewFakeKubeletConfigReconcilerForHyperShift(initObjects ...runtime.Object) Client: fakeClient, Scheme: scheme.Scheme, Namespace: testNamespace, - Recorder: record.NewFakeRecorder(bufferSize), Platform: platform.HyperShift, }, nil } @@ -146,22 +139,7 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelOperatorName, nro.Name)) Expect(cm.Labels).To(HaveKeyWithValue(rteconfig.LabelNodeGroupName+"/"+rteconfig.LabelNodeGroupKindMachineConfigPool, poolName)) }) - It("should send events when NRO present and operation successful", func() { - reconciler, err := newFakeReconciler(nro, mcp1, mcoKc1, cmKc1) - Expect(err).ToNot(HaveOccurred()) - - result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) - Expect(err).ToNot(HaveOccurred()) - Expect(result).To(Equal(reconcile.Result{})) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessOK")) - }) - - It("should send events when NRO present and operation failure", func() { + It("should fail when NRO present and operation failure", func() { brokenMcoKc := testobjs.NewKubeletConfigWithData("broken", label1, mcp1.Spec.MachineConfigSelector, []byte("")) // on HyperShift we can mimic this behavior by not having a ConfigMap with a KubeletConfig // present on the cluster at all @@ -171,12 +149,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { key := client.ObjectKeyFromObject(brokenMcoKc) _, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).To(HaveOccurred()) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessFailed")) }) It("should skip invalid kubeletconfig", func() { @@ -189,13 +161,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { key := client.ObjectKeyFromObject(invalidMcoKc) _, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessSkip")) - Expect(event).To(ContainSubstring(invalidMcoKc.Name)) }) It("should ignore non-matching kubeketconfigs", func() { @@ -209,22 +174,12 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) - - // verify creation event - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessSkip")) - Expect(event).To(ContainSubstring(ctrlPlaneKc.Name)) }) It("should process matching kubeletconfig, then ignore non-matching kubeketconfig", func() { reconciler, err := newFakeReconciler(nro, mcp1) Expect(err).ToNot(HaveOccurred()) - fakeRecorder, ok := reconciler.Recorder.(*record.FakeRecorder) - Expect(ok).To(BeTrue()) - var reconciledObj client.Object reconciledObj = mcoKc1 if clusterPlatform == platform.HyperShift { @@ -243,10 +198,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { Name: objectnames.GetComponentName(nro.Name, poolName), } Expect(reconciler.Client.Get(context.TODO(), key, cm)).ToNot(HaveOccurred()) - // verify creation event - event := <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessOK")) - Expect(event).To(ContainSubstring(reconciledObj.GetName())) ctrlPlaneKc := testobjs.NewKubeletConfigAutoresizeControlPlane() err = reconciler.Client.Create(context.TODO(), ctrlPlaneKc) @@ -261,11 +212,6 @@ var _ = Describe("Test KubeletConfig Reconcile", func() { result, err = reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) Expect(err).ToNot(HaveOccurred()) Expect(result).To(Equal(reconcile.Result{})) - - // verify creation event - event = <-fakeRecorder.Events - Expect(event).To(ContainSubstring("ProcessSkip")) - Expect(event).To(ContainSubstring(ctrlPlaneKc.Name)) }) }) }, diff --git a/internal/controller/numaresourcesoperator_controller.go b/internal/controller/numaresourcesoperator_controller.go index 4dd207d6f6..d842b7b8bf 100644 --- a/internal/controller/numaresourcesoperator_controller.go +++ b/internal/controller/numaresourcesoperator_controller.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/client-go/tools/record" "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" @@ -100,7 +99,6 @@ type NUMAResourcesOperatorReconciler struct { Namespace string Images images.Data ImagePullPolicy corev1.PullPolicy - Recorder record.EventRecorder ForwardMCPConds bool OverridableRTEImage bool // RTEMetricsTLS is derived from apiserver.config.openshift.io/cluster TLS profile at operator @@ -239,19 +237,14 @@ func (r *NUMAResourcesOperatorReconciler) degradeStatus(ctx context.Context, ini func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) intreconcile.Step { if annotations.IsNRTAPIDefinitionCluster(instance.Annotations) { - // should never happen, so let's be vocal. Very vocal. - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "SkipCRDInstall", "Skipped to install Node Resource Topology CRD: caused by annotation") + klog.InfoS("Skipped NRT CRD install: caused by annotation") return intreconcile.StepSuccess() } - applied, err := r.syncNodeResourceTopologyAPI(ctx) + _, err := r.syncNodeResourceTopologyAPI(ctx) if err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedCRDInstall", "Failed to install Node Resource Topology CRD: %v", err) err = fmt.Errorf("FailedAPISync: %w", err) return intreconcile.StepFailed(err) } - if applied { - r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulCRDInstall", "Node Resource Topology CRD installed") - } return intreconcile.StepSuccess() } @@ -260,11 +253,9 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx con // before checking additional components for updates waitByPool, pausedMCPNames, err := r.syncMachineConfigs(ctx, instance, existing, trees) if err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedMCSync", "Failed to set up machine configuration for worker nodes: %v", err) err = fmt.Errorf("failed to sync machine configs: %w", err) return intreconcile.StepFailed(err) } - r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulMCSync", "Enabled machine configuration for worker nodes") // MCO needs to update the SELinux context removal and other stuff, and need to trigger a reboot. // It can take a while. @@ -284,7 +275,6 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx con func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context.Context, instance *nropv1.NUMAResourcesOperator, existing *rtestate.ExistingManifests, trees []nodegroupv1.Tree) ([]poolDaemonSet, intreconcile.Step) { daemonSetsInfoPerPool, err := r.syncNUMAResourcesOperatorResources(ctx, instance, existing, trees) if err != nil { - r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedRTECreate", "Failed to create Resource-Topology-Exporter DaemonSets: %v", err) err = fmt.Errorf("FailedRTESync: %w", err) return nil, intreconcile.StepFailed(err) } @@ -293,8 +283,6 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context return nil, intreconcile.StepSuccess() } - r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulRTECreate", "Created Resource-Topology-Exporter DaemonSets") - dssWithReadyStatus, dsNamePending, err := r.syncDaemonSetsStatuses(ctx, r.Client, daemonSetsInfoPerPool) instance.Status.DaemonSets = dssWithReadyStatus instance.Status.RelatedObjects = relatedobjects.ResourceTopologyExporter(r.Namespace, dssWithReadyStatus) diff --git a/internal/controller/numaresourcesoperator_controller_test.go b/internal/controller/numaresourcesoperator_controller_test.go index 02598fcac6..a958138f4f 100644 --- a/internal/controller/numaresourcesoperator_controller_test.go +++ b/internal/controller/numaresourcesoperator_controller_test.go @@ -37,7 +37,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -88,8 +87,6 @@ func NewFakeNUMAResourcesOperatorReconciler(plat platform.Platform, platVersion if err != nil { return nil, err } - recorder := record.NewFakeRecorder(bufferSize) - return &NUMAResourcesOperatorReconciler{ Client: fakeClient, Scheme: scheme.Scheme, @@ -103,7 +100,6 @@ func NewFakeNUMAResourcesOperatorReconciler(plat platform.Platform, platVersion Images: images.Data{ Builtin: testImageSpec, }, - Recorder: recorder, }, nil } From 268f25e82fd3c557fbf9faf567717f1f8cf91516 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Wed, 24 Jun 2026 17:11:40 +0200 Subject: [PATCH 4/8] sched: rbac: consolidate the ClusterRole Up until now, the scheduler was bound to 2 cluster roles: 1. the standard `system:kube-scheduler` granting baseline permissions for scheduler-compatible operation 2. the custom `topology-aware-scheduler` enabling the numa-aware scheduler plugin NodeTopologyMatch. While this worked, it created unnecessary large permissions requirements _and_ coupling to the main scheduler RBAC rules. With this change, we 1. merge the kube-scheduler clusterrole permissions into our custom clusterrole permissions. The scheduler is now independent and easier to audit 2. narrow down as much as possible the clusterrole permissions, matching only the objects which the profile we support requires. 3. make sure the old clusterrole binding to kube-scheduler is deleted This enables us to have a less privileged scheduler with a much smaller RBAC footprint. It must be noted that to work at all the scheduler still needs a quite large set of RBAC write permissions, but this is unavoidable and driven by how secondary scheduler work in general, e.g. is not numa-aware specific. AI-attribution: AIA PAI Ce Hin R claude-4.6-opus-1M v1.0 Signed-off-by: Francesco Romani --- cmd/main.go | 15 ++ config/rbac/role.yaml | 140 ++++++++++++++- .../numaresourcesscheduler_controller.go | 35 +++- .../manifests/manifests.go | 4 - .../manifests/manifests_test.go | 3 - .../manifests/sched/sched.go | 8 - .../manifests/yaml/clusterrole.nrt.yaml | 163 +++++++++++++++++- .../manifests/yaml/clusterrolebinding.yaml | 12 -- .../objectstate/sched/sched.go | 31 ++-- .../objectstate/sched/sched_test.go | 50 ++++++ pkg/objectupdate/sched/sched.go | 2 - 11 files changed, 406 insertions(+), 57 deletions(-) delete mode 100644 pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml diff --git a/cmd/main.go b/cmd/main.go index 57566d2d47..bdd2bdff1a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -47,6 +47,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/healthz" + "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -67,6 +68,7 @@ import ( "github.com/openshift-kni/numaresources-operator/pkg/images" rtemetricsmanifests "github.com/openshift-kni/numaresources-operator/pkg/metrics/manifests/monitor" schedmanifests "github.com/openshift-kni/numaresources-operator/pkg/numaresourcesscheduler/manifests/sched" + schedstate "github.com/openshift-kni/numaresources-operator/pkg/numaresourcesscheduler/objectstate/sched" rtestate "github.com/openshift-kni/numaresources-operator/pkg/objectstate/rte" rteupdate "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/rte" schedupdate "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/sched" @@ -423,6 +425,19 @@ func main() { exitWithCancel(cancel, 1) } + // intentionally done unconditionally to catch the unlikely case someone managed + // to disable the scheduler pre-upgrade (very hard task anyway, but still) to + // ensure no dangerous leftovers + if err = mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { + if err := schedstate.Cleanup(ctx, mgr.GetClient()); err != nil { + klog.ErrorS(err, "best-effort scheduler cleanup failed") + } + return nil + })); err != nil { + klog.ErrorS(err, "unable to add scheduler cleanup runnable") + exitWithCancel(cancel, 1) + } + if params.enableScheduler { schedMf, err := schedmanifests.GetManifests(namespace) if err != nil { diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 85875b088d..cdc39ecb34 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -7,8 +7,21 @@ rules: - apiGroups: - "" resources: + - bindings + - pods/binding + verbs: + - create +- apiGroups: + - "" + resources: + - namespaces - nodes + - persistentvolumeclaims + - persistentvolumes + - replicationcontrollers + - services verbs: + - get - list - watch - apiGroups: @@ -16,9 +29,26 @@ rules: resources: - pods verbs: + - delete - get - list - watch +- apiGroups: + - "" + resources: + - pods/status + verbs: + - patch + - update +- apiGroups: + - "" + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update - apiGroups: - apiextensions.k8s.io resources: @@ -36,6 +66,35 @@ rules: verbs: - get - update +- apiGroups: + - apps + resources: + - statefulsets + verbs: + - get + - list + - watch +- apiGroups: + - apps + - extensions + resources: + - replicasets + verbs: + - get + - list + - watch +- apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create +- apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create - apiGroups: - config.openshift.io resources: @@ -57,6 +116,36 @@ rules: - clusterversions verbs: - list +- apiGroups: + - coordination.k8s.io + resources: + - leasecandidates + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create +- apiGroups: + - coordination.k8s.io + resourceNames: + - numa-scheduler-leader + resources: + - leases + verbs: + - get + - list + - update + - watch - apiGroups: - machineconfiguration.openshift.io resources: @@ -108,16 +197,22 @@ rules: - get - patch - update +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - get + - list + - watch - apiGroups: - rbac.authorization.k8s.io resourceNames: - secondary-scheduler - - topology-aware-scheduler resources: - clusterrolebindings verbs: - - get - - update + - delete - apiGroups: - rbac.authorization.k8s.io resources: @@ -142,10 +237,21 @@ rules: resourceNames: - topology-aware-scheduler resources: + - clusterrolebindings - clusterroles verbs: - get - update +- apiGroups: + - resource.k8s.io + resources: + - deviceclasses + - resourceclaims + - resourceslices + verbs: + - get + - list + - watch - apiGroups: - security.openshift.io resources: @@ -164,6 +270,18 @@ rules: verbs: - get - update +- apiGroups: + - storage.k8s.io + resources: + - csidrivers + - csinodes + - csistoragecapacities + - storageclasses + - volumeattachments + verbs: + - get + - list + - watch - apiGroups: - topology.node.k8s.io resources: @@ -173,6 +291,7 @@ rules: - get - list - update + - watch --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -191,6 +310,21 @@ rules: - list - update - watch +- apiGroups: + - "" + resources: + - endpoints + verbs: + - create +- apiGroups: + - "" + resourceNames: + - numa-scheduler-leader + resources: + - endpoints + verbs: + - get + - update - apiGroups: - "" resourceNames: diff --git a/internal/controller/numaresourcesscheduler_controller.go b/internal/controller/numaresourcesscheduler_controller.go index e4e1004a7d..a7822c2ff6 100644 --- a/internal/controller/numaresourcesscheduler_controller.go +++ b/internal/controller/numaresourcesscheduler_controller.go @@ -87,13 +87,34 @@ type NUMAResourcesSchedulerReconciler struct { //+kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=create;list;watch,namespace="numaresources" //+kubebuilder:rbac:groups="",resources=serviceaccounts,resourceNames=secondary-scheduler,verbs=get;update,namespace="numaresources" //+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;get;list;update;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=endpoints,verbs=create,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=endpoints,resourceNames=numa-scheduler-leader,verbs=get;update,namespace="numaresources" // Cluster Scoped //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=create;list;watch //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,resourceNames=topology-aware-scheduler,verbs=get;update //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create;list;watch -//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=secondary-scheduler;topology-aware-scheduler,verbs=get;update -//+kubebuilder:rbac:groups="",resources=nodes,verbs=list;watch +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=topology-aware-scheduler,verbs=get;update +//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=secondary-scheduler,verbs=delete +//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=pods,verbs=delete +//+kubebuilder:rbac:groups="",resources=pods/status,verbs=patch;update +//+kubebuilder:rbac:groups="",resources=pods/binding;bindings,verbs=create +//+kubebuilder:rbac:groups="",resources=replicationcontrollers;services;namespaces,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=persistentvolumeclaims;persistentvolumes,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch;update +//+kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch;update +//+kubebuilder:rbac:groups=apps;extensions,resources=replicasets,verbs=get;list;watch +//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch +//+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch +//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create +//+kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=create +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,resourceNames=numa-scheduler-leader,verbs=get;list;update;watch +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leasecandidates,verbs=create;delete;deletecollection;get;list;patch;update;watch +//+kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses;csinodes;csidrivers;csistoragecapacities;volumeattachments,verbs=get;list;watch +//+kubebuilder:rbac:groups=resource.k8s.io,resources=deviceclasses;resourceclaims;resourceslices,verbs=get;list;watch +//+kubebuilder:rbac:groups=topology.node.k8s.io,resources=noderesourcetopologies,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/finalizers,verbs=update @@ -380,12 +401,14 @@ func (r *NUMAResourcesSchedulerReconciler) syncNUMASchedulerResources(ctx contex existing := schedstate.FromClient(ctx, r.Client, r.SchedulerManifests) for _, objState := range existing.State(r.SchedulerManifests) { - if err := controllerutil.SetControllerReference(instance, objState.Desired, r.Scheme); err != nil { - return schedStatus, fmt.Errorf("failed to set controller reference to %s %s: %w", objState.Desired.GetNamespace(), objState.Desired.GetName(), err) + if objState.IsCreateOrUpdate() { + if err := controllerutil.SetControllerReference(instance, objState.Desired, r.Scheme); err != nil { + return schedStatus, fmt.Errorf("failed to set controller reference to %s %s: %w", objState.Desired.GetNamespace(), objState.Desired.GetName(), err) + } } - obj, _, err := apply.ApplyObject(ctx, r.Client, objState) + obj, _, err := apply.ApplyState(ctx, r.Client, objState) if err != nil { - return schedStatus, fmt.Errorf("could not apply (%s) %s/%s: %w", objState.Desired.GetObjectKind().GroupVersionKind(), objState.Desired.GetNamespace(), objState.Desired.GetName(), err) + return schedStatus, fmt.Errorf("could not apply scheduler object: %w", err) } if nname, ok := schedstate.DeploymentNamespacedNameFromObject(obj); ok { diff --git a/pkg/numaresourcesscheduler/manifests/manifests.go b/pkg/numaresourcesscheduler/manifests/manifests.go index 051474b877..561eb7083d 100644 --- a/pkg/numaresourcesscheduler/manifests/manifests.go +++ b/pkg/numaresourcesscheduler/manifests/manifests.go @@ -62,10 +62,6 @@ func ClusterRole() (*rbacv1.ClusterRole, error) { return cr, nil } -func ClusterRoleBindingK8S(namespace string) (*rbacv1.ClusterRoleBinding, error) { - return loadClusterRoleBinding("clusterrolebinding.yaml", namespace) -} - func ClusterRoleBindingNRT(namespace string) (*rbacv1.ClusterRoleBinding, error) { return loadClusterRoleBinding("clusterrolebinding.nrt.yaml", namespace) } diff --git a/pkg/numaresourcesscheduler/manifests/manifests_test.go b/pkg/numaresourcesscheduler/manifests/manifests_test.go index ff97e49c26..c074461382 100644 --- a/pkg/numaresourcesscheduler/manifests/manifests_test.go +++ b/pkg/numaresourcesscheduler/manifests/manifests_test.go @@ -27,9 +27,6 @@ func TestLoad(t *testing.T) { if obj, err := ClusterRole(); obj == nil || err != nil { t.Errorf("ClusterRole() failed: err=%v", err) } - if obj, err := ClusterRoleBindingK8S(""); obj == nil || err != nil { - t.Errorf("ClusterRoleBindingK8S() failed: err=%v", err) - } if obj, err := ClusterRoleBindingNRT(""); obj == nil || err != nil { t.Errorf("ClusterRoleBindingNRT() failed: err=%v", err) } diff --git a/pkg/numaresourcesscheduler/manifests/sched/sched.go b/pkg/numaresourcesscheduler/manifests/sched/sched.go index c3dd8587f8..f926c054c5 100644 --- a/pkg/numaresourcesscheduler/manifests/sched/sched.go +++ b/pkg/numaresourcesscheduler/manifests/sched/sched.go @@ -33,7 +33,6 @@ type Manifests struct { ServiceAccount *corev1.ServiceAccount ConfigMap *corev1.ConfigMap ClusterRole *rbacv1.ClusterRole - ClusterRoleBindingK8S *rbacv1.ClusterRoleBinding ClusterRoleBindingNRT *rbacv1.ClusterRoleBinding Role *rbacv1.Role RoleBinding *rbacv1.RoleBinding @@ -47,7 +46,6 @@ func (mf Manifests) ToObjects() []client.Object { mf.ServiceAccount, mf.ConfigMap, mf.ClusterRole, - mf.ClusterRoleBindingK8S, mf.ClusterRoleBindingNRT, mf.Role, mf.RoleBinding, @@ -62,7 +60,6 @@ func (mf Manifests) Clone() Manifests { ServiceAccount: mf.ServiceAccount.DeepCopy(), ConfigMap: mf.ConfigMap.DeepCopy(), ClusterRole: mf.ClusterRole.DeepCopy(), - ClusterRoleBindingK8S: mf.ClusterRoleBindingK8S.DeepCopy(), ClusterRoleBindingNRT: mf.ClusterRoleBindingNRT.DeepCopy(), Role: mf.Role.DeepCopy(), RoleBinding: mf.RoleBinding.DeepCopy(), @@ -91,11 +88,6 @@ func GetManifests(namespace string) (Manifests, error) { return mf, err } - mf.ClusterRoleBindingK8S, err = manifests.ClusterRoleBindingK8S(namespace) - if err != nil { - return mf, err - } - mf.ClusterRoleBindingNRT, err = manifests.ClusterRoleBindingNRT(namespace) if err != nil { return mf, err diff --git a/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml b/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml index a714ce87c6..fb4049bcf2 100644 --- a/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml +++ b/pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml @@ -3,10 +3,159 @@ kind: ClusterRole metadata: name: topology-aware-scheduler rules: - - apiGroups: ["topology.node.k8s.io"] - resources: ["noderesourcetopologies"] - verbs: ["get", "list", "watch"] - - apiGroups: ["storage.k8s.io"] - resources: ["storageclasses"] - verbs: ["get", "list", "watch"] - +- apiGroups: + - "" + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update +- apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - delete + - get + - list + - watch +- apiGroups: + - "" + resources: + - bindings + - pods/binding + verbs: + - create +- apiGroups: + - "" + resources: + - pods/status + verbs: + - patch + - update +- apiGroups: + - "" + resources: + - replicationcontrollers + - services + - namespaces + verbs: + - get + - list + - watch +- apiGroups: + - apps + - extensions + resources: + - replicasets + verbs: + - get + - list + - watch +- apiGroups: + - apps + resources: + - statefulsets + verbs: + - get + - list + - watch +- apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - persistentvolumeclaims + - persistentvolumes + verbs: + - get + - list + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create +- apiGroups: + - coordination.k8s.io + resourceNames: + - numa-scheduler-leader + resources: + - leases + verbs: + - get + - list + - update + - watch +- apiGroups: + - coordination.k8s.io + resources: + - leasecandidates + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch +- apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create +- apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create +- apiGroups: + - storage.k8s.io + resources: + - csinodes + - storageclasses + - csidrivers + - csistoragecapacities + - volumeattachments + verbs: + - get + - list + - watch +- apiGroups: + - resource.k8s.io + resources: + - deviceclasses + - resourceclaims + - resourceslices + verbs: + - get + - list + - watch +- apiGroups: + - topology.node.k8s.io + resources: + - noderesourcetopologies + verbs: + - get + - list + - watch diff --git a/pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml b/pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml deleted file mode 100644 index fedb062ae2..0000000000 --- a/pkg/numaresourcesscheduler/manifests/yaml/clusterrolebinding.yaml +++ /dev/null @@ -1,12 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: secondary-scheduler -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: system:kube-scheduler -subjects: - - kind: ServiceAccount - name: secondary-scheduler - namespace: placeholder diff --git a/pkg/numaresourcesscheduler/objectstate/sched/sched.go b/pkg/numaresourcesscheduler/objectstate/sched/sched.go index 73204b5c00..e05496df2e 100644 --- a/pkg/numaresourcesscheduler/objectstate/sched/sched.go +++ b/pkg/numaresourcesscheduler/objectstate/sched/sched.go @@ -18,11 +18,13 @@ package sched import ( "context" + "fmt" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -41,14 +43,30 @@ const ( SchedulerConfigFileName = "config.yaml" SchedulerConfigMapVolumeName = "etckubernetes" SchedulerPluginName = "NodeResourceTopologyMatch" + + legacyClusterRoleBindingK8SName = "secondary-scheduler" ) +// Cleanup removes leftover resources from previous operator versions. +func Cleanup(ctx context.Context, cli client.Client) error { + crb := &rbacv1.ClusterRoleBinding{} + crb.Name = legacyClusterRoleBindingK8SName + err := cli.Delete(ctx, crb) + if err != nil { + if apierrors.IsNotFound(err) { + return nil // expected steady state. Stay silent + } + return fmt.Errorf("failed to delete legacy ClusterRoleBinding %q: %w", legacyClusterRoleBindingK8SName, err) + } + klog.Infof("deleted legacy ClusterRoleBinding %q", legacyClusterRoleBindingK8SName) + return nil +} + type ExistingManifests struct { Existing schedmanifests.Manifests serviceAccountError error configMapError error clusterRoleError error - clusterRoleBindingK8SError error clusterRoleBindingNRTError error roleError error roleBindingError error @@ -80,13 +98,6 @@ func (em ExistingManifests) State(mf schedmanifests.Manifests) []objectstate.Obj Compare: compare.Object, Merge: merge.MetadataForUpdate, }, - { - Existing: em.Existing.ClusterRoleBindingK8S, - Error: em.clusterRoleBindingK8SError, - Desired: mf.ClusterRoleBindingK8S.DeepCopy(), - Compare: compare.Object, - Merge: merge.MetadataForUpdate, - }, { Existing: em.Existing.ClusterRoleBindingNRT, Error: em.clusterRoleBindingNRTError, @@ -152,10 +163,6 @@ func FromClient(ctx context.Context, cli client.Client, mf schedmanifests.Manife ret.Existing.ClusterRole = cro } - crbK8S := &rbacv1.ClusterRoleBinding{} - if ret.clusterRoleBindingK8SError = cli.Get(ctx, client.ObjectKeyFromObject(mf.ClusterRoleBindingK8S), crbK8S); ret.clusterRoleBindingK8SError == nil { - ret.Existing.ClusterRoleBindingK8S = crbK8S - } crbNRT := &rbacv1.ClusterRoleBinding{} if ret.clusterRoleBindingNRTError = cli.Get(ctx, client.ObjectKeyFromObject(mf.ClusterRoleBindingNRT), crbNRT); ret.clusterRoleBindingNRTError == nil { ret.Existing.ClusterRoleBindingNRT = crbNRT diff --git a/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go b/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go index e9ef6c64e5..dfb7672092 100644 --- a/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go +++ b/pkg/numaresourcesscheduler/objectstate/sched/sched_test.go @@ -17,14 +17,18 @@ limitations under the License. package sched import ( + "context" "reflect" "testing" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" nropv1 "github.com/openshift-kni/numaresources-operator/api/v1" ) @@ -88,6 +92,52 @@ profiles: kubeconfigpath: "" # needs to be empty string` ) +func TestCleanup(t *testing.T) { + type testCase struct { + name string + objects []client.Object + } + + testCases := []testCase{ + { + name: "CRB exists and is deleted", + objects: []client.Object{ + &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secondary-scheduler", + }, + }, + }, + }, + { + name: "CRB does not exist", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + builder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + for _, obj := range tc.objects { + builder = builder.WithObjects(obj) + } + cli := builder.Build() + + ctx := context.Background() + + cleanErr := Cleanup(ctx, cli) + if cleanErr != nil { + t.Errorf("unexpected error: %v", cleanErr) + } + crb := &rbacv1.ClusterRoleBinding{} + crb.Name = "secondary-scheduler" + getErr := cli.Get(ctx, client.ObjectKeyFromObject(crb), crb) + if getErr == nil { + t.Errorf("ClusterRoleBinding should not exist after Cleanup") + } + }) + } +} + func TestSchedulerNameFromObject(t *testing.T) { type testCase struct { name string diff --git a/pkg/objectupdate/sched/sched.go b/pkg/objectupdate/sched/sched.go index 388e168739..5086b34468 100644 --- a/pkg/objectupdate/sched/sched.go +++ b/pkg/objectupdate/sched/sched.go @@ -39,8 +39,6 @@ import ( objtls "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/tls" ) -// these should be provided by a deployer API - const ( MainContainerName = "secondary-scheduler" ) From c5ca2871ef8f2e5e758bc5c864d42f0e3cbe1388 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 26 Jun 2026 10:38:23 +0200 Subject: [PATCH 5/8] rbac: regenerate bundle run: `make generate bundle manifests` Signed-off-by: Francesco Romani --- ...ources-operator.clusterserviceversion.yaml | 142 +++++++++++++++++- 1 file changed, 138 insertions(+), 4 deletions(-) diff --git a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml index f72a16258b..9788bb6f48 100644 --- a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml +++ b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml @@ -66,7 +66,7 @@ metadata: } ] capabilities: Basic Install - createdAt: "2026-07-06T08:55:09Z" + createdAt: "2026-07-14T07:55:36Z" features.operators.openshift.io/cnf: "true" features.operators.openshift.io/cni: "false" features.operators.openshift.io/csi: "false" @@ -381,8 +381,21 @@ spec: - apiGroups: - "" resources: + - bindings + - pods/binding + verbs: + - create + - apiGroups: + - "" + resources: + - namespaces - nodes + - persistentvolumeclaims + - persistentvolumes + - replicationcontrollers + - services verbs: + - get - list - watch - apiGroups: @@ -390,9 +403,26 @@ spec: resources: - pods verbs: + - delete - get - list - watch + - apiGroups: + - "" + resources: + - pods/status + verbs: + - patch + - update + - apiGroups: + - "" + - events.k8s.io + resources: + - events + verbs: + - create + - patch + - update - apiGroups: - apiextensions.k8s.io resources: @@ -410,6 +440,35 @@ spec: verbs: - get - update + - apiGroups: + - apps + resources: + - statefulsets + verbs: + - get + - list + - watch + - apiGroups: + - apps + - extensions + resources: + - replicasets + verbs: + - get + - list + - watch + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create - apiGroups: - config.openshift.io resources: @@ -431,6 +490,36 @@ spec: - clusterversions verbs: - list + - apiGroups: + - coordination.k8s.io + resources: + - leasecandidates + verbs: + - create + - delete + - deletecollection + - get + - list + - patch + - update + - watch + - apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - apiGroups: + - coordination.k8s.io + resourceNames: + - numa-scheduler-leader + resources: + - leases + verbs: + - get + - list + - update + - watch - apiGroups: - machineconfiguration.openshift.io resources: @@ -482,16 +571,22 @@ spec: - get - patch - update + - apiGroups: + - policy + resources: + - poddisruptionbudgets + verbs: + - get + - list + - watch - apiGroups: - rbac.authorization.k8s.io resourceNames: - secondary-scheduler - - topology-aware-scheduler resources: - clusterrolebindings verbs: - - get - - update + - delete - apiGroups: - rbac.authorization.k8s.io resources: @@ -516,10 +611,21 @@ spec: resourceNames: - topology-aware-scheduler resources: + - clusterrolebindings - clusterroles verbs: - get - update + - apiGroups: + - resource.k8s.io + resources: + - deviceclasses + - resourceclaims + - resourceslices + verbs: + - get + - list + - watch - apiGroups: - security.openshift.io resources: @@ -538,6 +644,18 @@ spec: verbs: - get - update + - apiGroups: + - storage.k8s.io + resources: + - csidrivers + - csinodes + - csistoragecapacities + - storageclasses + - volumeattachments + verbs: + - get + - list + - watch - apiGroups: - topology.node.k8s.io resources: @@ -547,6 +665,7 @@ spec: - get - list - update + - watch serviceAccountName: numaresources-controller-manager deployments: - label: @@ -693,6 +812,21 @@ spec: - list - update - watch + - apiGroups: + - "" + resources: + - endpoints + verbs: + - create + - apiGroups: + - "" + resourceNames: + - numa-scheduler-leader + resources: + - endpoints + verbs: + - get + - update - apiGroups: - "" resourceNames: From bb91236c452023a63c123cac1cd5924ac4e16ecf Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 26 Jun 2026 14:51:01 +0200 Subject: [PATCH 6/8] controller: rbac: move operand RBAC annotations to separate files The operator needs to hold permissions actually required by its operand in order to be able to grant them. But we can at very least split them in separate files to clearly distinguish the RBAC (through the kubebuilder annotations) the operator truly needs from what it needs just to pass through the operands. AI-attribution: AIA PAI Ce Hin R claude-4.6-opus-1M v1.0 Signed-off-by: Francesco Romani --- .../numaresourcesscheduler_controller.go | 19 ------- internal/controller/operand_rbac_rte.go | 38 +++++++++++++ internal/controller/operand_rbac_sched.go | 53 +++++++++++++++++++ 3 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 internal/controller/operand_rbac_rte.go create mode 100644 internal/controller/operand_rbac_sched.go diff --git a/internal/controller/numaresourcesscheduler_controller.go b/internal/controller/numaresourcesscheduler_controller.go index a7822c2ff6..138b167abd 100644 --- a/internal/controller/numaresourcesscheduler_controller.go +++ b/internal/controller/numaresourcesscheduler_controller.go @@ -96,25 +96,6 @@ type NUMAResourcesSchedulerReconciler struct { //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=create;list;watch //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=topology-aware-scheduler,verbs=get;update //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,resourceNames=secondary-scheduler,verbs=delete -//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch -//+kubebuilder:rbac:groups="",resources=pods,verbs=delete -//+kubebuilder:rbac:groups="",resources=pods/status,verbs=patch;update -//+kubebuilder:rbac:groups="",resources=pods/binding;bindings,verbs=create -//+kubebuilder:rbac:groups="",resources=replicationcontrollers;services;namespaces,verbs=get;list;watch -//+kubebuilder:rbac:groups="",resources=persistentvolumeclaims;persistentvolumes,verbs=get;list;watch -//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch;update -//+kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch;update -//+kubebuilder:rbac:groups=apps;extensions,resources=replicasets,verbs=get;list;watch -//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch -//+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch -//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create -//+kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create -//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=create -//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,resourceNames=numa-scheduler-leader,verbs=get;list;update;watch -//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leasecandidates,verbs=create;delete;deletecollection;get;list;patch;update;watch -//+kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses;csinodes;csidrivers;csistoragecapacities;volumeattachments,verbs=get;list;watch -//+kubebuilder:rbac:groups=resource.k8s.io,resources=deviceclasses;resourceclaims;resourceslices,verbs=get;list;watch -//+kubebuilder:rbac:groups=topology.node.k8s.io,resources=noderesourcetopologies,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers,verbs=get;list;watch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=nodetopology.openshift.io,resources=numaresourcesschedulers/finalizers,verbs=update diff --git a/internal/controller/operand_rbac_rte.go b/internal/controller/operand_rbac_rte.go new file mode 100644 index 0000000000..0153fcb196 --- /dev/null +++ b/internal/controller/operand_rbac_rte.go @@ -0,0 +1,38 @@ +/* + * Copyright 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// RTE operand RBAC escalation permissions. +// +// The operator creates and updates the "rte" ClusterRole (sourced from +// vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/yaml/rte/clusterrole.yaml). +// Kubernetes RBAC prevents a subject from granting permissions it does not +// already hold, so the operator's own ClusterRole must include every +// permission present in the RTE operand's ClusterRole. +// +// These annotations exist solely to satisfy that escalation requirement. +// The operator itself never exercises these permissions (some overlap +// with the operator's own needs, such as nodes list, but they are listed +// here explicitly so the escalation rationale is self-documenting). +// +// Future improvement: ship the operand ClusterRoles as static manifests +// in the OLM bundle (pre-deploy) instead of creating them in the reconciler. +// The operator would then only create ClusterRoleBindings, which does not +// require escalation privileges, and all annotations below can be removed. + +package controller + +//+kubebuilder:rbac:groups=topology.node.k8s.io,resources=noderesourcetopologies,verbs=get;list;create;update +//+kubebuilder:rbac:groups="",resources=nodes,verbs=watch;list diff --git a/internal/controller/operand_rbac_sched.go b/internal/controller/operand_rbac_sched.go new file mode 100644 index 0000000000..d4f75cdf4c --- /dev/null +++ b/internal/controller/operand_rbac_sched.go @@ -0,0 +1,53 @@ +/* + * Copyright 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Scheduler operand RBAC escalation permissions. +// +// The operator creates and updates the "topology-aware-scheduler" ClusterRole +// (see pkg/numaresourcesscheduler/manifests/yaml/clusterrole.nrt.yaml). +// Kubernetes RBAC prevents a subject from granting permissions it does not +// already hold, so the operator's own ClusterRole must include every +// permission present in the scheduler operand's ClusterRole. +// +// These annotations exist solely to satisfy that escalation requirement. +// The operator itself never exercises these permissions. +// +// Future improvement: ship the operand ClusterRoles as static manifests +// in the OLM bundle (pre-deploy) instead of creating them in the reconciler. +// The operator would then only create ClusterRoleBindings, which does not +// require escalation privileges, and all annotations below can be removed. + +package controller + +//+kubebuilder:rbac:groups="",resources=nodes,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=pods,verbs=delete +//+kubebuilder:rbac:groups="",resources=pods/status,verbs=patch;update +//+kubebuilder:rbac:groups="",resources=pods/binding;bindings,verbs=create +//+kubebuilder:rbac:groups="",resources=replicationcontrollers;services;namespaces,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=persistentvolumeclaims;persistentvolumes,verbs=get;list;watch +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch;update +//+kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patch;update +//+kubebuilder:rbac:groups=apps;extensions,resources=replicasets,verbs=get;list;watch +//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch +//+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch +//+kubebuilder:rbac:groups=authentication.k8s.io,resources=tokenreviews,verbs=create +//+kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=create +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,resourceNames=numa-scheduler-leader,verbs=get;list;update;watch +//+kubebuilder:rbac:groups=coordination.k8s.io,resources=leasecandidates,verbs=create;delete;deletecollection;get;list;patch;update;watch +//+kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses;csinodes;csidrivers;csistoragecapacities;volumeattachments,verbs=get;list;watch +//+kubebuilder:rbac:groups=resource.k8s.io,resources=deviceclasses;resourceclaims;resourceslices,verbs=get;list;watch +//+kubebuilder:rbac:groups=topology.node.k8s.io,resources=noderesourcetopologies,verbs=get;list;watch From 8406f55e63e06515d1329b126b16a743cf2bfef7 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Fri, 26 Jun 2026 15:00:19 +0200 Subject: [PATCH 7/8] chore: verify-generated to capture the exit value we need to capture and bubble up the actual exit value of git diff vs rely on the output or lack thereof. Signed-off-by: Francesco Romani --- hack/verify-generated.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/verify-generated.sh b/hack/verify-generated.sh index 92e93ffc4f..fab2c7956a 100755 --- a/hack/verify-generated.sh +++ b/hack/verify-generated.sh @@ -2,7 +2,7 @@ # compare the CSV file against the latest revision # but ignore the createdAt field which is always changing. -if [[ "$(git diff --quiet -I'^ createdAt: ' bundle)" -eq 0 ]]; then +if git diff --quiet -I'^ createdAt: ' bundle; then git checkout --quiet bundle fi From 8bdd0a78053ac2adabf40a94f690518bd39a6de3 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 14 Jul 2026 09:53:55 +0200 Subject: [PATCH 8/8] controller: hypershift: relax configmap rbac rules On HyperShift, we need to read configmaps from different namespaces, namely from the `openshift-config-managed` namespace. The only viable option is to move back the ConfigMap read permission as cluster role, while keeping the write/mutation permission as namespaced role. Ideally, we would have injected a specific role+rolebinding in the manifest bundle, but turns out that OLM always overwrites the namespace in the namespaced manifests (and that would perfectly make sense) so we cna't add permissions about a different namespace. AI-attribution: AIA PAI Ce Hin R claude-4.6-opus-1M v1.0 Signed-off-by: Francesco Romani --- .../numaresources-operator.clusterserviceversion.yaml | 3 ++- config/rbac/role.yaml | 1 + internal/controller/kubeletconfig_controller.go | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml index 9788bb6f48..182cecffe7 100644 --- a/bundle/manifests/numaresources-operator.clusterserviceversion.yaml +++ b/bundle/manifests/numaresources-operator.clusterserviceversion.yaml @@ -66,7 +66,7 @@ metadata: } ] capabilities: Basic Install - createdAt: "2026-07-14T07:55:36Z" + createdAt: "2026-07-14T08:24:59Z" features.operators.openshift.io/cnf: "true" features.operators.openshift.io/cni: "false" features.operators.openshift.io/csi: "false" @@ -388,6 +388,7 @@ spec: - apiGroups: - "" resources: + - configmaps - namespaces - nodes - persistentvolumeclaims diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index cdc39ecb34..a10fb3cd95 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -14,6 +14,7 @@ rules: - apiGroups: - "" resources: + - configmaps - namespaces - nodes - persistentvolumeclaims diff --git a/internal/controller/kubeletconfig_controller.go b/internal/controller/kubeletconfig_controller.go index 018197c1e3..3d9b669854 100644 --- a/internal/controller/kubeletconfig_controller.go +++ b/internal/controller/kubeletconfig_controller.go @@ -86,8 +86,9 @@ type kubeletConfigHandler struct { } // Namespace Scoped -//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;get;list;update;watch,namespace="numaresources" +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;update,namespace="numaresources" // Cluster Scoped +//+kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs,verbs=get;list;watch //+kubebuilder:rbac:groups=machineconfiguration.openshift.io,resources=kubeletconfigs/finalizers,verbs=update