From 4e3c2c45740099362cf44125cba07f39b0486838 Mon Sep 17 00:00:00 2001 From: Divyam pateriya Date: Tue, 24 Mar 2026 18:44:37 +0530 Subject: [PATCH] fix: OCPBUGS-78575: create virt-launcher NetworkPolicy on external infra cluster When deploying HCP KubeVirt with external infrastructure (workers on a separate cluster), the virt-launcher NetworkPolicy was never created on the infrastructure cluster. The reconcileNetworkPolicies function explicitly skipped it when Credentials != nil, leaving guest VMs with unrestricted network access to all pods and services on the infra cluster. This patch adds an else branch that uses the existing infra cluster client (from KubevirtInfraClientMap) to create the virt-launcher NetworkPolicy in the infra namespace on the infrastructure cluster. A new reconcileVirtLauncherNetworkPolicyExternalInfra function builds the policy adapted for external infra: - Blocks infra cluster's clusterNetwork/serviceNetwork CIDRs - Allows inter-VM, DNS, and ingress controller traffic - Omits control-plane pod selectors (kube-apiserver, oauth, ignition-server-proxy) since those pods run on the management cluster and are reached via external IPs The Network config lookup is best-effort: if the infra kubeconfig lacks cluster-scoped get on networks.config.openshift.io, the policy is still created but without CIDR-based egress blocking. Also updates the documented minimum RBAC role for external infra to include networkpolicies (networking.k8s.io) and documents the optional ClusterRole for full network isolation. Made-with: Cursor --- .../v1beta1/hostedcluster_conditions.go | 12 + .../kubevirt/external-infrastructure.md | 53 ++- docs/content/reference/aggregated-docs.md | 62 +++- docs/content/reference/api.md | 9 + .../hostedcluster/hostedcluster_controller.go | 8 + .../hostedcluster/network_policies.go | 312 +++++++++++++----- support/conditions/conditions.go | 3 + test/e2e/util/util.go | 4 + .../v1beta1/hostedcluster_conditions.go | 12 + 9 files changed, 391 insertions(+), 84 deletions(-) diff --git a/api/hypershift/v1beta1/hostedcluster_conditions.go b/api/hypershift/v1beta1/hostedcluster_conditions.go index 59e13f4cba67..e9e01de0accc 100644 --- a/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -122,6 +122,15 @@ const ( // performance degradation due to fragmentation of the double encapsulation in ovn-kubernetes ValidKubeVirtInfraNetworkMTU ConditionType = "ValidKubeVirtInfraNetworkMTU" + // ValidKubeVirtInfraNetworkPolicyRBAC indicates whether the external infra + // kubeconfig has sufficient permissions to create/update the virt-launcher network policy + // on the infrastructure cluster. This covers both reading the + // cluster network configuration (networks.config.openshift.io) for CIDR- + // based egress blocking and creating/updating NetworkPolicy resources in + // the infra namespace. When false, tenant isolation may be weaker: the + // NetworkPolicy may be missing or lack CIDR-based egress restrictions. + ValidKubeVirtInfraNetworkPolicyRBAC ConditionType = "ValidKubeVirtInfraNetworkPolicyRBAC" + // KubeVirtNodesLiveMigratable indicates if all nodes (VirtualMachines) of the kubevirt // hosted cluster can be live migrated without experiencing a node restart KubeVirtNodesLiveMigratable ConditionType = "KubeVirtNodesLiveMigratable" @@ -279,6 +288,9 @@ const ( InvalidIdentityProvider = "InvalidIdentityProvider" PayloadArchNotFoundReason = "PayloadArchNotFound" + InfraClusterNetworkReadFailedReason = "InfraClusterNetworkReadFailed" + InfraClusterNetworkPolicyCreateFailedReason = "InfraClusterNetworkPolicyCreateFailed" + InvalidIAMRoleReason = "InvalidIAMRole" InvalidAzureCredentialsReason = "InvalidAzureCredentials" diff --git a/docs/content/how-to/kubevirt/external-infrastructure.md b/docs/content/how-to/kubevirt/external-infrastructure.md index b84b22b7ab85..bae9499747e1 100644 --- a/docs/content/how-to/kubevirt/external-infrastructure.md +++ b/docs/content/how-to/kubevirt/external-infrastructure.md @@ -63,13 +63,19 @@ The user or service account used in the provided kubeconfig should have full per * `endpointslices` * `endpointslices/restricted` * `routes` +* `networkpolicies` The user or service account used in the provided kubeconfig should also have get/create/delete permissions over the following resources: * `volumesnapshots` -As well as get permission for: +As well as get/create/update permission for: +* `events` +And get permission for: * `persistentvolumeclaims` All of these permissions are needed only on the target namespace on the infra cluster (passed through the `--infra-namespace` command-line argument). -This can be achieved by binding the following Role to the user used in the external infra kubeconfig: + +In addition, the HyperShift operator reads the infrastructure cluster's network configuration (`networks.config.openshift.io`) to build a virt-launcher NetworkPolicy that blocks egress to the infra cluster's internal pod/service networks. This resource is **cluster-scoped**, so it requires a separate ClusterRole and ClusterRoleBinding (see below). If this permission is not granted, the NetworkPolicy is still created but without CIDR-based egress blocking, and a `ValidKubeVirtInfraNetworkPolicyRBAC=False` condition is set on the HostedCluster along with a warning event in the infrastructure cluster namespace. + +This can be achieved by binding the following Role **and** ClusterRole to the user used in the external infra kubeconfig: ```yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -117,6 +123,20 @@ rules: - secrets verbs: - '*' + - apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - '*' + - apiGroups: + - '' + resources: + - events + verbs: + - get + - create + - update - apiGroups: - snapshot.storage.k8s.io resources: @@ -132,3 +152,32 @@ rules: verbs: - get ``` + +For full virt-launcher network isolation, also create a ClusterRole and ClusterRoleBinding +to allow reading the infrastructure cluster's network configuration: +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: kv-external-infra-network-reader +rules: + - apiGroups: + - config.openshift.io + resources: + - networks + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: kv-external-infra-network-reader-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: kv-external-infra-network-reader +subjects: + - kind: ServiceAccount + name: hcp-infra-sa + namespace: clusters-example +``` diff --git a/docs/content/reference/aggregated-docs.md b/docs/content/reference/aggregated-docs.md index 27b35ca75c3a..ebdd73f77b15 100644 --- a/docs/content/reference/aggregated-docs.md +++ b/docs/content/reference/aggregated-docs.md @@ -19364,13 +19364,19 @@ The user or service account used in the provided kubeconfig should have full per * `endpointslices` * `endpointslices/restricted` * `routes` +* `networkpolicies` The user or service account used in the provided kubeconfig should also have get/create/delete permissions over the following resources: * `volumesnapshots` -As well as get permission for: +As well as get/create/update permission for: +* `events` +And get permission for: * `persistentvolumeclaims` All of these permissions are needed only on the target namespace on the infra cluster (passed through the `--infra-namespace` command-line argument). -This can be achieved by binding the following Role to the user used in the external infra kubeconfig: + +In addition, the HyperShift operator reads the infrastructure cluster's network configuration (`networks.config.openshift.io`) to build a virt-launcher NetworkPolicy that blocks egress to the infra cluster's internal pod/service networks. This resource is **cluster-scoped**, so it requires a separate ClusterRole and ClusterRoleBinding (see below). If this permission is not granted, the NetworkPolicy is still created but without CIDR-based egress blocking, and a `ValidKubeVirtInfraNetworkPolicyRBAC=False` condition is set on the HostedCluster along with a warning event in the infrastructure cluster namespace. + +This can be achieved by binding the following Role **and** ClusterRole to the user used in the external infra kubeconfig: ```yaml apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -19418,6 +19424,20 @@ rules: - secrets verbs: - '*' + - apiGroups: + - networking.k8s.io + resources: + - networkpolicies + verbs: + - '*' + - apiGroups: + - '' + resources: + - events + verbs: + - get + - create + - update - apiGroups: - snapshot.storage.k8s.io resources: @@ -19434,6 +19454,35 @@ rules: - get ``` +For full virt-launcher network isolation, also create a ClusterRole and ClusterRoleBinding +to allow reading the infrastructure cluster's network configuration: +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: kv-external-infra-network-reader +rules: + - apiGroups: + - config.openshift.io + resources: + - networks + verbs: + - get +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: kv-external-infra-network-reader-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: kv-external-infra-network-reader +subjects: + - kind: ServiceAccount + name: hcp-infra-sa + namespace: clusters-example +``` + --- @@ -37019,6 +37068,15 @@ e.g. the user-provided IDP configuration provided is invalid or the IDP is not r hosting a guest cluster utilizing kubevirt platform is a sufficient value that will avoid performance degradation due to fragmentation of the double encapsulation in ovn-kubernetes

+

"ValidKubeVirtInfraNetworkPolicyRBAC"

+

ValidKubeVirtInfraNetworkPolicyRBAC indicates whether the external infra +kubeconfig has sufficient permissions to create/update the virt-launcher network policy +on the infrastructure cluster. This covers both reading the +cluster network configuration (networks.config.openshift.io) for CIDR- +based egress blocking and creating/updating NetworkPolicy resources in +the infra namespace. When false, tenant isolation may be weaker: the +NetworkPolicy may be missing or lack CIDR-based egress restrictions.

+

"ValidOIDCConfiguration"

ValidOIDCConfiguration indicates if an AWS cluster’s OIDC condition is detected as invalid. diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index b430388c9417..d6a506f42a3b 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -5998,6 +5998,15 @@ e.g. the user-provided IDP configuration provided is invalid or the IDP is not r hosting a guest cluster utilizing kubevirt platform is a sufficient value that will avoid performance degradation due to fragmentation of the double encapsulation in ovn-kubernetes

+

"ValidKubeVirtInfraNetworkPolicyRBAC"

+

ValidKubeVirtInfraNetworkPolicyRBAC indicates whether the external infra +kubeconfig has sufficient permissions to create/update the virt-launcher network policy +on the infrastructure cluster. This covers both reading the +cluster network configuration (networks.config.openshift.io) for CIDR- +based egress blocking and creating/updating NetworkPolicy resources in +the infra namespace. When false, tenant isolation may be weaker: the +NetworkPolicy may be missing or lack CIDR-based egress restrictions.

+

"ValidOIDCConfiguration"

ValidOIDCConfiguration indicates if an AWS cluster’s OIDC condition is detected as invalid. diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index de465192a5de..f19fcbcbb3a8 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -2039,6 +2039,14 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques // Reconcile platform specific items switch hcluster.Spec.Platform.Type { case hyperv1.KubevirtPlatform: + if hcluster.Spec.Platform.Kubevirt != nil && hcluster.Spec.Platform.Kubevirt.Credentials != nil { + if err := r.Client.Status().Update(ctx, hcluster); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to update status after network policy RBAC check: %w", err) + } + } err = r.reconcileKubevirtCSIClusterRBAC(ctx, createOrUpdate, hcluster) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to reconcile kubevirt CSI cluster wide RBAC: %w", err) diff --git a/hypershift-operator/controllers/hostedcluster/network_policies.go b/hypershift-operator/controllers/hostedcluster/network_policies.go index cacebd25db80..54c53809fa04 100644 --- a/hypershift-operator/controllers/hostedcluster/network_policies.go +++ b/hypershift-operator/controllers/hostedcluster/network_policies.go @@ -6,6 +6,7 @@ import ( "net" "net/netip" "os" + "time" hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" "github.com/openshift/hypershift/hypershift-operator/controllers/manifests" @@ -22,6 +23,8 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" utilsnet "k8s.io/utils/net" @@ -141,13 +144,90 @@ func (r *HostedClusterReconciler) reconcileNetworkPolicies(ctx context.Context, } case hyperv1.KubevirtPlatform: if hcluster.Spec.Platform.Kubevirt.Credentials == nil { - // network policy is being set on centralized infra only, not on external infra + // Centralized infra: policy targets the control plane namespace on the management cluster policy = networkpolicy.VirtLauncherNetworkPolicy(controlPlaneNamespaceName) if _, err := createOrUpdate(ctx, r.Client, policy, func() error { return reconcileVirtLauncherNetworkPolicy(log, policy, hcluster, managementClusterNetwork) }); err != nil { return fmt.Errorf("failed to reconcile virt launcher policy: %w", err) } + } else { + // External infra: policy targets the infra namespace on the infrastructure cluster + kvInfraClient, err := r.KubevirtInfraClients.DiscoverKubevirtClusterClient(ctx, + r.Client, + hcluster.Spec.InfraID, + hcluster.Spec.Platform.Kubevirt.Credentials, + hcluster.Namespace, + hcluster.Namespace) + if err != nil { + return fmt.Errorf("failed to get kubevirt infra client for network policy: %w", err) + } + + infraClient := kvInfraClient.GetInfraClient() + infraNamespace := kvInfraClient.GetInfraNamespace() + + // networks.config.openshift.io is cluster-scoped, so the infra + // kubeconfig needs a ClusterRole with get permission on that + // resource. When the permission is missing we still create the + // NetworkPolicy but without CIDR-based egress blocking, and + // surface the RBAC gap as a condition on the HostedCluster. + var infraClusterNetwork *configv1.Network + networkObj := &configv1.Network{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + if err := infraClient.Get(ctx, client.ObjectKeyFromObject(networkObj), networkObj); err != nil { + if apierrors.IsForbidden(err) || apierrors.IsNotFound(err) || meta.IsNoMatchError(err) { + rbacMsg := fmt.Sprintf( + "The external infrastructure kubeconfig lacks permission to read "+ + "networks.config.openshift.io/cluster. The virt-launcher NetworkPolicy "+ + "has been created without CIDR-based egress restrictions, resulting in "+ + "weaker tenant isolation. Grant a ClusterRole with get on "+ + "networks.config.openshift.io to the infra service account for full isolation. "+ + "Error: %v", err) + log.Info(rbacMsg) + meta.SetStatusCondition(&hcluster.Status.Conditions, metav1.Condition{ + Type: string(hyperv1.ValidKubeVirtInfraNetworkPolicyRBAC), + Status: metav1.ConditionFalse, + Reason: hyperv1.InfraClusterNetworkReadFailedReason, + ObservedGeneration: hcluster.Generation, + Message: rbacMsg, + }) + emitInfraClusterWarningEvent(ctx, infraClient, infraNamespace, hcluster.Spec.InfraID, rbacMsg, log) + } else { + return fmt.Errorf("failed to get infrastructure cluster network config: %w", err) + } + } else { + infraClusterNetwork = networkObj + meta.SetStatusCondition(&hcluster.Status.Conditions, metav1.Condition{ + Type: string(hyperv1.ValidKubeVirtInfraNetworkPolicyRBAC), + Status: metav1.ConditionTrue, + Reason: hyperv1.AsExpectedReason, + ObservedGeneration: hcluster.Generation, + Message: "Infrastructure cluster network configuration is readable; CIDR-based egress restrictions are active.", + }) + } + + policy = networkpolicy.VirtLauncherNetworkPolicy(infraNamespace) + if _, err := createOrUpdate(ctx, infraClient, policy, func() error { + return reconcileVirtLauncherNetworkPolicyExternalInfra(log, policy, hcluster, infraClusterNetwork) + }); err != nil { + if apierrors.IsForbidden(err) { + rbacMsg := fmt.Sprintf( + "Unable to create/update virt-launcher NetworkPolicy on the infrastructure cluster: "+ + "the external infra kubeconfig lacks networking.k8s.io/networkpolicies permissions. "+ + "Grant create/update/get/list/watch on networkpolicies in the infra namespace. "+ + "Error: %v", err) + log.Info(rbacMsg) + meta.SetStatusCondition(&hcluster.Status.Conditions, metav1.Condition{ + Type: string(hyperv1.ValidKubeVirtInfraNetworkPolicyRBAC), + Status: metav1.ConditionFalse, + Reason: hyperv1.InfraClusterNetworkPolicyCreateFailedReason, + ObservedGeneration: hcluster.Generation, + Message: rbacMsg, + }) + emitInfraClusterWarningEvent(ctx, infraClient, infraNamespace, hcluster.Spec.InfraID, rbacMsg, log) + } else { + return fmt.Errorf("failed to reconcile virt launcher policy on external infra: %w", err) + } + } } } @@ -567,20 +647,76 @@ func addToBlockedNetworks(network string, blockedIPv4Networks []string, blockedI } func reconcileVirtLauncherNetworkPolicy(log logr.Logger, policy *networkingv1.NetworkPolicy, hcluster *hyperv1.HostedCluster, managementClusterNetwork *configv1.Network) error { - protocolTCP := corev1.ProtocolTCP - protocolUDP := corev1.ProtocolUDP - protocolSCTP := corev1.ProtocolSCTP - blockedIPv4Networks := []string{} blockedIPv6Networks := []string{} for _, network := range managementClusterNetwork.Spec.ClusterNetwork { blockedIPv4Networks, blockedIPv6Networks = addToBlockedNetworks(network.CIDR, blockedIPv4Networks, blockedIPv6Networks) } - for _, network := range managementClusterNetwork.Spec.ServiceNetwork { blockedIPv4Networks, blockedIPv6Networks = addToBlockedNetworks(network, blockedIPv4Networks, blockedIPv6Networks) } + controlPlanePeers := []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "hypershift.openshift.io/control-plane-component": "kube-apiserver", + }, + }, + }, + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "hypershift.openshift.io/control-plane-component": "oauth-openshift", + }, + }, + }, + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "ignition-server-proxy", + }, + }, + }, + } + + return buildVirtLauncherNetworkPolicyBase(log, policy, hcluster, blockedIPv4Networks, blockedIPv6Networks, controlPlanePeers) +} + +// reconcileVirtLauncherNetworkPolicyExternalInfra builds the virt-launcher +// NetworkPolicy for deployments where the KubeVirt VMs run on a separate +// infrastructure cluster. Unlike the centralized variant, this omits egress +// rules for control-plane pods (kube-apiserver, oauth, ignition-server-proxy) +// because those pods reside on the management cluster and are reached via +// external IPs already permitted by the broad 0.0.0.0/0 allow rule. +// +// infraClusterNetwork may be nil when the infra kubeconfig lacks cluster- +// scoped read access to networks.config.openshift.io. In that case the +// policy is still created but without CIDR-based egress blocking. +func reconcileVirtLauncherNetworkPolicyExternalInfra(log logr.Logger, policy *networkingv1.NetworkPolicy, hcluster *hyperv1.HostedCluster, infraClusterNetwork *configv1.Network) error { + blockedIPv4Networks := []string{} + blockedIPv6Networks := []string{} + if infraClusterNetwork != nil { + for _, network := range infraClusterNetwork.Spec.ClusterNetwork { + blockedIPv4Networks, blockedIPv6Networks = addToBlockedNetworks(network.CIDR, blockedIPv4Networks, blockedIPv6Networks) + } + for _, network := range infraClusterNetwork.Spec.ServiceNetwork { + blockedIPv4Networks, blockedIPv6Networks = addToBlockedNetworks(network, blockedIPv4Networks, blockedIPv6Networks) + } + } + + return buildVirtLauncherNetworkPolicyBase(log, policy, hcluster, blockedIPv4Networks, blockedIPv6Networks, nil) +} + +// buildVirtLauncherNetworkPolicyBase constructs the common virt-launcher +// NetworkPolicy structure shared by both centralized and external infra +// deployments. extraEgressPeers are appended to the primary egress rule +// (e.g. control-plane pod selectors for centralized infra). +func buildVirtLauncherNetworkPolicyBase(log logr.Logger, policy *networkingv1.NetworkPolicy, hcluster *hyperv1.HostedCluster, blockedIPv4Networks, blockedIPv6Networks []string, extraEgressPeers []networkingv1.NetworkPolicyPeer) error { + protocolTCP := corev1.ProtocolTCP + protocolUDP := corev1.ProtocolUDP + protocolSCTP := corev1.ProtocolSCTP + policy.Spec.PolicyTypes = []networkingv1.PolicyType{networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress} policy.Spec.PodSelector = metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -597,95 +733,66 @@ func reconcileVirtLauncherNetworkPolicy(log logr.Logger, policy *networkingv1.Ne }, }, } - policy.Spec.Egress = []networkingv1.NetworkPolicyEgressRule{ + + egressPeers := []networkingv1.NetworkPolicyPeer{ { - To: []networkingv1.NetworkPolicyPeer{ - { - // Allow access towards outside the cluster for the virtual machines (guest nodes), - // But deny access to other cluster's pods and services with the exceptions below - // IPv4 - IPBlock: &networkingv1.IPBlock{ - CIDR: netip.PrefixFrom(netip.IPv4Unspecified(), 0).String(), // 0.0.0.0/0 - Except: blockedIPv4Networks, - }, - }, - { - // IPv6 - IPBlock: &networkingv1.IPBlock{ - CIDR: netip.PrefixFrom(netip.IPv6Unspecified(), 0).String(), // ::/0 - Except: blockedIPv6Networks, - }, - }, - { - // Allow the guest nodes to communicate between each other - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - hyperv1.InfraIDLabel: hcluster.Spec.InfraID, - "kubevirt.io": "virt-launcher", - }, - }, - }, - { - // Allow access to the cluster's DNS server for name resolution - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "dns.operator.openshift.io/daemonset-dns": "default", - }, - }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "kubernetes.io/metadata.name": "openshift-dns", - }, - }, + IPBlock: &networkingv1.IPBlock{ + CIDR: netip.PrefixFrom(netip.IPv4Unspecified(), 0).String(), + Except: blockedIPv4Networks, + }, + }, + { + IPBlock: &networkingv1.IPBlock{ + CIDR: netip.PrefixFrom(netip.IPv6Unspecified(), 0).String(), + Except: blockedIPv6Networks, + }, + }, + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + hyperv1.InfraIDLabel: hcluster.Spec.InfraID, + "kubevirt.io": "virt-launcher", }, - { - // Allow access to the guest cluster API server - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "hypershift.openshift.io/control-plane-component": "kube-apiserver", - }, - }, + }, + }, + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "dns.operator.openshift.io/daemonset-dns": "default", }, - { - // Allow access to the oauth server (from the console pod on the virtual machine) - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "hypershift.openshift.io/control-plane-component": "oauth-openshift", - }, - }, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "kubernetes.io/metadata.name": "openshift-dns", }, - { - // Allow access to the ignition server - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "ignition-server-proxy", - }, - }, + }, + }, + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "ingresscontroller.operator.openshift.io/deployment-ingresscontroller": "default", }, - { - // Allow access to the management cluster ingress (for ignition server) - PodSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "ingresscontroller.operator.openshift.io/deployment-ingresscontroller": "default", - }, - }, - NamespaceSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "name": "openshift-ingress", - }, - }, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "name": "openshift-ingress", }, }, }, } + egressPeers = append(egressPeers, extraEgressPeers...) + + policy.Spec.Egress = []networkingv1.NetworkPolicyEgressRule{ + {To: egressPeers}, + } + nodeAddressesMap := make(map[string]bool) for _, hcService := range hcluster.Spec.Services { if hcService.Type != hyperv1.NodePort { continue } nodeAddress := hcService.NodePort.Address - _, exists := nodeAddressesMap[nodeAddress] - if exists { + if nodeAddressesMap[nodeAddress] { continue } nodeAddressesMap[nodeAddress] = true @@ -956,3 +1063,48 @@ func reconcileMetricsServerNetworkPolicy(policy *networkingv1.NetworkPolicy, hcp return nil } + +// emitInfraClusterWarningEvent creates or updates a warning Event in the +// infrastructure cluster namespace so that operators monitoring the infra +// cluster can see the RBAC gap without access to the management cluster. +func emitInfraClusterWarningEvent(ctx context.Context, infraClient client.Client, namespace, infraID, message string, log logr.Logger) { + now := metav1.NewTime(time.Now()) + eventName := fmt.Sprintf("virt-launcher-netpol-rbac-%s", infraID) + + event := &corev1.Event{ + ObjectMeta: metav1.ObjectMeta{ + Name: eventName, + Namespace: namespace, + }, + } + + existing := &corev1.Event{} + if err := infraClient.Get(ctx, client.ObjectKeyFromObject(event), existing); err == nil { + existing.Count++ + existing.LastTimestamp = now + existing.Message = message + if updateErr := infraClient.Update(ctx, existing); updateErr != nil { + log.Info("unable to update warning event on infrastructure cluster", "error", updateErr) + } + return + } + + event.InvolvedObject = corev1.ObjectReference{ + APIVersion: "v1", + Kind: "Namespace", + Name: namespace, + } + event.Reason = "InsufficientNetworkPolicyRBAC" + event.Message = message + event.Type = corev1.EventTypeWarning + event.Source = corev1.EventSource{ + Component: "hypershift-operator", + } + event.FirstTimestamp = now + event.LastTimestamp = now + event.Count = 1 + + if createErr := infraClient.Create(ctx, event); createErr != nil { + log.Info("unable to create warning event on infrastructure cluster", "error", createErr) + } +} diff --git a/support/conditions/conditions.go b/support/conditions/conditions.go index 4378888fcb94..2d13f01bed8f 100644 --- a/support/conditions/conditions.go +++ b/support/conditions/conditions.go @@ -88,6 +88,9 @@ func ExpectedHCConditions(hostedCluster *hyperv1.HostedCluster) map[hyperv1.Cond // thus the PVC is RWO and VMs are expected to be non-live-migratable conditions[hyperv1.KubeVirtNodesLiveMigratable] = metav1.ConditionFalse } + if hostedCluster.Spec.Platform.Kubevirt != nil && hostedCluster.Spec.Platform.Kubevirt.Credentials != nil { + conditions[hyperv1.ValidKubeVirtInfraNetworkPolicyRBAC] = metav1.ConditionTrue + } } if hostedCluster.Spec.Etcd.ManagementType == hyperv1.Unmanaged { diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index f538a9bb0bb4..0a9b9961162d 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -2983,6 +2983,7 @@ func ValidateHostedClusterConditions(t *testing.T, ctx context.Context, client c expectedConditions[hyperv1.ClusterVersionSucceeding] = metav1.ConditionFalse expectedConditions[hyperv1.ClusterVersionProgressing] = metav1.ConditionTrue delete(expectedConditions, hyperv1.ValidKubeVirtInfraNetworkMTU) + delete(expectedConditions, hyperv1.ValidKubeVirtInfraNetworkPolicyRBAC) expectedConditions[hyperv1.DataPlaneConnectionAvailable] = metav1.ConditionUnknown expectedConditions[hyperv1.ControlPlaneConnectionAvailable] = metav1.ConditionUnknown } @@ -2990,6 +2991,9 @@ func ValidateHostedClusterConditions(t *testing.T, ctx context.Context, client c // ValidKubeVirtInfraNetworkMTU condition is not present in versions < 4.15 delete(expectedConditions, hyperv1.ValidKubeVirtInfraNetworkMTU) } + if IsLessThan(Version422) { + delete(expectedConditions, hyperv1.ValidKubeVirtInfraNetworkPolicyRBAC) + } if IsLessThan(Version421) { delete(expectedConditions, hyperv1.DataPlaneConnectionAvailable) diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go index 59e13f4cba67..e9e01de0accc 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -122,6 +122,15 @@ const ( // performance degradation due to fragmentation of the double encapsulation in ovn-kubernetes ValidKubeVirtInfraNetworkMTU ConditionType = "ValidKubeVirtInfraNetworkMTU" + // ValidKubeVirtInfraNetworkPolicyRBAC indicates whether the external infra + // kubeconfig has sufficient permissions to create/update the virt-launcher network policy + // on the infrastructure cluster. This covers both reading the + // cluster network configuration (networks.config.openshift.io) for CIDR- + // based egress blocking and creating/updating NetworkPolicy resources in + // the infra namespace. When false, tenant isolation may be weaker: the + // NetworkPolicy may be missing or lack CIDR-based egress restrictions. + ValidKubeVirtInfraNetworkPolicyRBAC ConditionType = "ValidKubeVirtInfraNetworkPolicyRBAC" + // KubeVirtNodesLiveMigratable indicates if all nodes (VirtualMachines) of the kubevirt // hosted cluster can be live migrated without experiencing a node restart KubeVirtNodesLiveMigratable ConditionType = "KubeVirtNodesLiveMigratable" @@ -279,6 +288,9 @@ const ( InvalidIdentityProvider = "InvalidIdentityProvider" PayloadArchNotFoundReason = "PayloadArchNotFound" + InfraClusterNetworkReadFailedReason = "InfraClusterNetworkReadFailed" + InfraClusterNetworkPolicyCreateFailedReason = "InfraClusterNetworkPolicyCreateFailed" + InvalidIAMRoleReason = "InvalidIAMRole" InvalidAzureCredentialsReason = "InvalidAzureCredentials"