From e3662b54e06c1cb281ae8d4109d28083a502d3f9 Mon Sep 17 00:00:00 2001 From: Bryan Cox Date: Wed, 25 Feb 2026 12:05:07 -0500 Subject: [PATCH] OCPBUGS-XXXXX: Add cluster-scoped RBAC for machine-approver to read APIServer TLS profile The cluster-machine-approver binary now calls FetchAPIServerTLSProfile() at startup using its in-cluster (management cluster) client, which requires get/list/watch access to the cluster-scoped apiservers.config.openshift.io resource. This was introduced by openshift/cluster-machine-approver PR #286. The existing machine-approver RBAC in HyperShift is a namespace-scoped Role that only grants access to cluster.x-k8s.io/machines. Since apiservers is cluster-scoped, a ClusterRole and ClusterRoleBinding are needed. Following the existing pattern used by the PKI operator and KubeVirt CSI driver, the cluster-scoped RBAC is created by the hypershift-operator's HostedCluster controller, which has the necessary permissions. Changes: - Add reconcileMachineApproverClusterRBAC to the HostedCluster controller creating a ClusterRole and ClusterRoleBinding per HostedCluster - Gate RBAC creation on DisableMachineManagement annotation, matching the CPO's machine-approver component predicate - Add MachineApproverServiceAccount helper in cpomanifests for consistency with PKIOperatorServiceAccount and KubevirtCSIDriverInfraSA Co-Authored-By: Claude Opus 4.6 --- .../manifests/machineapprover.go | 15 ++++ .../hostedcluster/hostedcluster_controller.go | 73 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 control-plane-operator/controllers/hostedcontrolplane/manifests/machineapprover.go diff --git a/control-plane-operator/controllers/hostedcontrolplane/manifests/machineapprover.go b/control-plane-operator/controllers/hostedcontrolplane/manifests/machineapprover.go new file mode 100644 index 00000000000..0631785c657 --- /dev/null +++ b/control-plane-operator/controllers/hostedcontrolplane/manifests/machineapprover.go @@ -0,0 +1,15 @@ +package manifests + +import ( + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func MachineApproverServiceAccount(ns string) *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-approver", + Namespace: ns, + }, + } +} diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index e10d289f9e3..5cb692e9287 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -2028,6 +2028,14 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } } + // Reconcile machine-approver cluster-scoped RBAC - the CPO does not have rights to do this itself. + // Skip if machine management is disabled, matching the CPO's machine-approver component predicate. + if _, disabled := hcluster.Annotations[hyperv1.DisableMachineManagement]; !disabled { + if err = r.reconcileMachineApproverClusterRBAC(ctx, createOrUpdate, hcluster); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to reconcile machine-approver cluster RBAC: %w", err) + } + } + // Reconcile the network policies if err = r.reconcileNetworkPolicies(ctx, log, createOrUpdate, hcluster, hcp, releaseImageVersion, controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel); err != nil { return ctrl.Result{}, fmt.Errorf("failed to reconcile network policies: %w", err) @@ -2670,6 +2678,71 @@ func (r *HostedClusterReconciler) reconcileControlPlanePKIOperatorRBAC(ctx conte return nil } +func (r *HostedClusterReconciler) reconcileMachineApproverClusterRBAC(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster) error { + // We don't create this ServiceAccount, the CPO does via the machine-approver v2 component assets, + // but we can reference it in RBAC before it's created as the system is eventually consistent. + hcpns := manifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name) + serviceAccount := cpomanifests.MachineApproverServiceAccount(hcpns) + + // Reconcile machine-approver ClusterRole for reading APIServer TLS profile. + // The machine-approver binary calls FetchAPIServerTLSProfile() using its in-cluster + // (management cluster) client, which requires get access to the cluster-scoped + // apiservers.config.openshift.io resource. + machineApproverClusterRole := &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s-machine-approver", hcpns), + Labels: map[string]string{ + controlplanepkioperatormanifests.OwningHostedClusterNamespaceLabel: hcluster.Namespace, + controlplanepkioperatormanifests.OwningHostedClusterNameLabel: hcluster.Name, + }, + }, + } + _, err := createOrUpdate(ctx, r.Client, machineApproverClusterRole, func() error { + machineApproverClusterRole.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{"config.openshift.io"}, + Resources: []string{"apiservers"}, + Verbs: []string{"get", "list", "watch"}, + }, + } + return nil + }) + if err != nil { + return fmt.Errorf("failed to reconcile machine-approver cluster role: %w", err) + } + + // Reconcile machine-approver ClusterRoleBinding + machineApproverClusterRoleBinding := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: machineApproverClusterRole.Name, + Labels: map[string]string{ + controlplanepkioperatormanifests.OwningHostedClusterNamespaceLabel: hcluster.Namespace, + controlplanepkioperatormanifests.OwningHostedClusterNameLabel: hcluster.Name, + }, + }, + } + _, err = createOrUpdate(ctx, r.Client, machineApproverClusterRoleBinding, func() error { + machineApproverClusterRoleBinding.RoleRef = rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: machineApproverClusterRole.Name, + } + machineApproverClusterRoleBinding.Subjects = []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: serviceAccount.Name, + Namespace: serviceAccount.Namespace, + }, + } + return nil + }) + if err != nil { + return fmt.Errorf("failed to reconcile machine-approver cluster role binding: %w", err) + } + + return nil +} + func (r *HostedClusterReconciler) reconcileKubevirtCSIClusterRBAC(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster) error { // We don't create this ServiceAccount, it's part of the kubevirt CSI manifests, but we can reference it due to eventual consistency hcpns := manifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name)