From d4e61a2392688146ad2132078dfe1ad145486e4d Mon Sep 17 00:00:00 2001 From: Joel Smith Date: Tue, 16 Jun 2026 23:11:54 -0600 Subject: [PATCH] UPSTREAM: : Add NodeSelectorAdjuster admission plugin for HCP clusters Add a new kube-apiserver admission plugin that removes the node-role.kubernetes.io/master node selector to qualifying control-plane-adjacent Day 2 operator pods at admission time. On hosted-control plane clusters, there are no master nodes so Day 2 operators which expect to run on a master node are unable to run without manual intervention. On standalone OpenShift clusters, operators like the VPA operator must run on master nodes for security reasons (e.g., limiting blast radius of privilege escalation from a worker-node compromise). HCP clusters have a different security posture (using the cluster rather than the node as a security boundary) and no master nodes exist in the guest cluster, so the plugin provides a means of running the VPA on HCP without manual intervention. The plugin detects standalone clusters by checking POD_NAMESPACE=openshift-kube-apiserver at start-up. Currently the VPA Operator pod is the only registered workload. The design is extensible to other control-plane-adjacent operators by adding label matchers to requiresNodeSelectorAdjustment(). Co-Authored-By: Claude Opus 4.6 --- .../admission/admissionenablement/register.go | 48 +-- .../nodeselectoradjuster/admission.go | 108 +++++++ .../nodeselectoradjuster/admission_test.go | 280 ++++++++++++++++++ 3 files changed, 417 insertions(+), 19 deletions(-) create mode 100644 openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go create mode 100644 openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go diff --git a/openshift-kube-apiserver/admission/admissionenablement/register.go b/openshift-kube-apiserver/admission/admissionenablement/register.go index eb822bd996542..c0fbd8f7d4ec4 100644 --- a/openshift-kube-apiserver/admission/admissionenablement/register.go +++ b/openshift-kube-apiserver/admission/admissionenablement/register.go @@ -22,6 +22,7 @@ import ( ingressadmission "k8s.io/kubernetes/openshift-kube-apiserver/admission/route" "k8s.io/kubernetes/openshift-kube-apiserver/admission/route/hostassignment" projectnodeenv "k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/nodeenv" + "k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster" schedulerpodnodeconstraints "k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/podnodeconstraints" "k8s.io/kubernetes/openshift-kube-apiserver/admission/storage/csiinlinevolumesecurity" "k8s.io/kubernetes/openshift-kube-apiserver/admission/storage/performantsecuritypolicy" @@ -46,6 +47,9 @@ func RegisterOpenshiftKubeAdmissionPlugins(plugins *admission.Plugins) { restrictedendpoints.RegisterRestrictedEndpoints(plugins) csiinlinevolumesecurity.Register(plugins) performantsecuritypolicy.Register(plugins) + if !nodeselectoradjuster.IsStandalone() { + nodeselectoradjuster.Register(plugins) + } } var ( @@ -62,25 +66,31 @@ var ( ) // openshiftAdmissionPluginsForKubeBeforeMutating are the admission plugins to add after kube admission, before mutating webhooks - openshiftAdmissionPluginsForKubeBeforeMutating = []string{ - "autoscaling.openshift.io/ClusterResourceOverride", - managementcpusoverride.PluginName, // "autoscaling.openshift.io/ManagementCPUsOverride" - "authorization.openshift.io/RestrictSubjectBindings", - "autoscaling.openshift.io/RunOnceDuration", - "scheduling.openshift.io/PodNodeConstraints", - "scheduling.openshift.io/OriginPodNodeEnvironment", - "network.openshift.io/ExternalIPRanger", - "network.openshift.io/RestrictedEndpointsAdmission", - imagepolicyapiv1.PluginName, // "image.openshift.io/ImagePolicy" - "security.openshift.io/SecurityContextConstraint", - "security.openshift.io/SCCExecRestrictions", - "route.openshift.io/IngressAdmission", - hostassignment.PluginName, // "route.openshift.io/RouteHostAssignment" - csiinlinevolumesecurity.PluginName, // "storage.openshift.io/CSIInlineVolumeSecurity" - managednode.PluginName, // "autoscaling.openshift.io/ManagedNode" - mixedcpus.PluginName, // "autoscaling.openshift.io/MixedCPUs" - performantsecuritypolicy.PluginName, // "storage.openshift.io/PerformantSecurityPolicy" - } + openshiftAdmissionPluginsForKubeBeforeMutating = func() []string { + plugins := []string{ + "autoscaling.openshift.io/ClusterResourceOverride", + managementcpusoverride.PluginName, // "autoscaling.openshift.io/ManagementCPUsOverride" + "authorization.openshift.io/RestrictSubjectBindings", + "autoscaling.openshift.io/RunOnceDuration", + "scheduling.openshift.io/PodNodeConstraints", + "scheduling.openshift.io/OriginPodNodeEnvironment", + "network.openshift.io/ExternalIPRanger", + "network.openshift.io/RestrictedEndpointsAdmission", + imagepolicyapiv1.PluginName, // "image.openshift.io/ImagePolicy" + "security.openshift.io/SecurityContextConstraint", + "security.openshift.io/SCCExecRestrictions", + "route.openshift.io/IngressAdmission", + hostassignment.PluginName, // "route.openshift.io/RouteHostAssignment" + csiinlinevolumesecurity.PluginName, // "storage.openshift.io/CSIInlineVolumeSecurity" + managednode.PluginName, // "autoscaling.openshift.io/ManagedNode" + mixedcpus.PluginName, // "autoscaling.openshift.io/MixedCPUs" + performantsecuritypolicy.PluginName, // "storage.openshift.io/PerformantSecurityPolicy" + } + if !nodeselectoradjuster.IsStandalone() { + plugins = append(plugins, nodeselectoradjuster.PluginName) // "scheduling.openshift.io/NodeSelectorAdjuster" + } + return plugins + }() // openshiftAdmissionPluginsForKubeAfterResourceQuota are the plugins to add after ResourceQuota plugin openshiftAdmissionPluginsForKubeAfterResourceQuota = []string{ diff --git a/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go new file mode 100644 index 0000000000000..009349b6fbae8 --- /dev/null +++ b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go @@ -0,0 +1,108 @@ +package nodeselectoradjuster + +// The NodeSelectorAdjuster admission plugin removes the +// node-role.kubernetes.io/master node selector from qualifying pods. It only +// activates on HCP (hosted control plane) clusters, detected by +// POD_NAMESPACE != openshift-kube-apiserver. On standalone OpenShift clusters +// the plugin does not register itself and takes no action, leaving the master +// node selector in place so that qualifying pods run on master nodes as +// intended. + +import ( + "context" + "fmt" + "io" + "os" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apiserver/pkg/admission" + coreapi "k8s.io/kubernetes/pkg/apis/core" +) + +const ( + // PluginName is the name used to identify this plugin in the admission chain. + PluginName = "scheduling.openshift.io/NodeSelectorAdjuster" + + // masterRoleKey is the node role label used as a node selector key in + // 4.x VPA operator manifests. + masterRoleKey = "node-role.kubernetes.io/master" + + // vpaOperatorLabelKey / vpaOperatorLabelValue identify the VPA operator pod. + vpaOperatorLabelKey = "k8s-app" + vpaOperatorLabelValue = "vertical-pod-autoscaler-operator" + // vpaOperatorNamespace is the namespace the VPA operator is expected to run in. + vpaOperatorNamespace = "openshift-vertical-pod-autoscaler" + + // standaloneEnvVar is the environment variable checked at start-up. + // It is injected by the downward API and reflects the namespace the + // kube-apiserver pod runs in. + standaloneEnvVar = "POD_NAMESPACE" + // standaloneEnvValue is the namespace used by the kube-apiserver on a + // standalone OpenShift cluster. + standaloneEnvValue = "openshift-kube-apiserver" +) + +// IsStandalone reports whether the current process is running inside a standalone +// OpenShift cluster. It is checked once at start-up to decide whether the plugin +// should register itself. +func IsStandalone() bool { + return os.Getenv(standaloneEnvVar) == standaloneEnvValue +} + +// Register adds the plugin to the admission plugin registry. It must only be +// called when IsStandalone() returns false (i.e. on HCP clusters). +func Register(plugins *admission.Plugins) { + plugins.Register(PluginName, func(_ io.Reader) (admission.Interface, error) { + return &nodeSelectorAdjuster{ + Handler: admission.NewHandler(admission.Create), + }, nil + }) +} + +// nodeSelectorAdjuster implements admission.MutationInterface. +type nodeSelectorAdjuster struct { + *admission.Handler +} + +var _ admission.MutationInterface = &nodeSelectorAdjuster{} + +// Admit examines newly-created Pod objects and, for qualifying pods, removes the +// master node selector so that they can run on worker nodes in HCP clusters. +func (p *nodeSelectorAdjuster) Admit(_ context.Context, attr admission.Attributes, _ admission.ObjectInterfaces) error { + if attr.GetResource().GroupResource() != corev1.Resource("pods") || attr.GetSubresource() != "" { + return nil + } + + pod, ok := attr.GetObject().(*coreapi.Pod) + if !ok { + return admission.NewForbidden(attr, fmt.Errorf("unexpected object type: %T", attr.GetObject())) + } + + if !requiresNodeSelectorAdjustment(pod) { + return nil + } + + delete(pod.Spec.NodeSelector, masterRoleKey) + return nil +} + +// ValidateInitialization satisfies admission.InitializationValidator. The plugin +// has no external dependencies to validate. +func (p *nodeSelectorAdjuster) ValidateInitialization() error { + return nil +} + +// requiresNodeSelectorAdjustment returns true when the pod carries a label that +// opts it in to node placement and lives in a namespace where that +// label is expected. Control-plane-adjacent Day 2 operators can be added here. +func requiresNodeSelectorAdjustment(pod *coreapi.Pod) bool { + // For VPA, we only want to update if the sole node selector is the + // master role key, which is the default from the 4.x VPA operator manifests. + if pod.Labels[vpaOperatorLabelKey] == vpaOperatorLabelValue && + pod.Namespace == vpaOperatorNamespace && len(pod.Spec.NodeSelector) == 1 { + if _, found := pod.Spec.NodeSelector[masterRoleKey]; found { + return true + } + } + return false +} diff --git a/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go new file mode 100644 index 0000000000000..9c78248b1701a --- /dev/null +++ b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go @@ -0,0 +1,280 @@ +package nodeselectoradjuster + +import ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authentication/user" + coreapi "k8s.io/kubernetes/pkg/apis/core" +) + +func TestAdmit(t *testing.T) { + tests := []struct { + name string + pod *coreapi.Pod + resource schema.GroupVersionResource + subresource string + expectedNodeSelector map[string]string + }{ + { + name: "VPA operator pod with master node selector: selector is removed", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + withNodeSelector(map[string]string{masterRoleKey: ""}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: map[string]string{}, + }, + { + name: "VPA operator pod with no node selector: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "VPA operator pod with non-default node selector: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + withNodeSelector(map[string]string{"topology.kubernetes.io/zone": "us-east-1a"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: map[string]string{"topology.kubernetes.io/zone": "us-east-1a"}, + }, + { + name: "VPA operator pod with extra node selectors including master: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + withNodeSelector(map[string]string{masterRoleKey: "", "topology.kubernetes.io/zone": "us-east-1a"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: map[string]string{ + masterRoleKey: "", + "topology.kubernetes.io/zone": "us-east-1a", + }, + }, + { + name: "non-qualifying pod: not modified", + pod: makePod( + withLabels(map[string]string{"app": "some-other-app"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "pod with no labels: not modified", + pod: makePod(), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "VPA operator label with wrong value: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: "some-other-operator"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "VPA operator label in wrong namespace: not modified", + pod: makePod( + withNamespace("other-namespace"), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "non-pod resource: request is ignored", + pod: makePod( + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("nodes").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "pod subresource: request is ignored", + pod: makePod( + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + subresource: "exec", + expectedNodeSelector: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + plugin := &nodeSelectorAdjuster{ + Handler: admission.NewHandler(admission.Create), + } + + attrs := admission.NewAttributesRecord( + tc.pod, + nil, + schema.GroupVersionKind{}, + tc.pod.Namespace, + tc.pod.Name, + tc.resource, + tc.subresource, + admission.Create, + nil, + false, + fakeUser(), + ) + + if err := plugin.Admit(context.TODO(), attrs, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify node selector. + gotSelector := tc.pod.Spec.NodeSelector + if len(gotSelector) != len(tc.expectedNodeSelector) { + t.Errorf("node selector: expected %v, got %v", tc.expectedNodeSelector, gotSelector) + } else { + for k, v := range tc.expectedNodeSelector { + if gotSelector[k] != v { + t.Errorf("node selector key %q: expected %q, got %q", k, v, gotSelector[k]) + } + } + } + }) + } +} + +func TestRequiresNodeSelectorAdjustment(t *testing.T) { + tests := []struct { + name string + pod *coreapi.Pod + expected bool + }{ + { + name: "VPA operator with master node selector: match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{masterRoleKey: ""})), + expected: true, + }, + { + name: "VPA operator with no node selector: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue})), + expected: false, + }, + { + name: "VPA operator with non-default node selector: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{"topology.kubernetes.io/zone": "us-east-1a"})), + expected: false, + }, + { + name: "VPA operator with extra node selectors: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{masterRoleKey: "", "extra": "value"})), + expected: false, + }, + { + name: "VPA operator in wrong namespace: no match", + pod: makePod(withNamespace("other-namespace"), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{masterRoleKey: ""})), + expected: false, + }, + { + name: "no labels: no match", + pod: makePod(), + expected: false, + }, + { + name: "unrelated labels: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{"app": "foo", "version": "v1"})), + expected: false, + }, + { + name: "VPA operator label with wrong value: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: "some-other-operator"})), + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := requiresNodeSelectorAdjustment(tc.pod) + if got != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, got) + } + }) + } +} + +func TestIsStandalone(t *testing.T) { + tests := []struct { + name string + envValue string + expected bool + }{ + { + name: "env var set to 'openshift-kube-apiserver': IsStandalone returns true", + envValue: "openshift-kube-apiserver", + expected: true, + }, + { + name: "env var set to empty string: IsStandalone returns false", + envValue: "", + expected: false, + }, + { + name: "env var set to another namespace: IsStandalone returns false", + envValue: "clusters-my-cluster", + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(standaloneEnvVar, tc.envValue) + got := IsStandalone() + if got != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, got) + } + }) + } +} + +// makePod constructs a coreapi.Pod, applying each option in order. +func makePod(opts ...func(*coreapi.Pod)) *coreapi.Pod { + p := &coreapi.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + for _, opt := range opts { + opt(p) + } + return p +} + +func withNamespace(ns string) func(*coreapi.Pod) { + return func(p *coreapi.Pod) { + p.Namespace = ns + } +} + +func withLabels(labels map[string]string) func(*coreapi.Pod) { + return func(p *coreapi.Pod) { + p.Labels = labels + } +} + +func withNodeSelector(selector map[string]string) func(*coreapi.Pod) { + return func(p *coreapi.Pod) { + p.Spec.NodeSelector = selector + } +} + +func fakeUser() user.Info { + return &user.DefaultInfo{Name: "testuser"} +}