From f5af5233ee0474e8ce4b28017303e4ddc6ba3ffc Mon Sep 17 00:00:00 2001 From: Andrew Pantuso Date: Fri, 5 Jun 2026 15:36:29 -0400 Subject: [PATCH] feat(operator): add --hcp-egress-block-cidrs flag to stabilize NetworkPolicy egress exceptions HCP namespace egress NetworkPolicies (private-router, management-kas) use /32 CIDRs derived from the management cluster's KAS endpoint IPs to block HCP pods from reaching the hosting KAS. These IPs rotate during rolling restarts, causing a burst of NetworkPolicy UPDATEs that can trigger OVN port-group reconciliation races and drop traffic to HCP router pods. Introduce --hcp-egress-block-cidrs, a repeatable flag that accepts stable CIDR blocks (e.g. the MC machine network) as a static replacement for the dynamic KAS endpoint /32s. When supplied, the egress exception list in both policies is built from the static CIDRs instead of the live Endpoints object, eliminating churn during KAS rollouts. Default behaviour (flag absent) is unchanged: KAS endpoint IPs continue to be discovered at reconcile time from the default/kubernetes Endpoints object. Signed-off-by: Andrew Pantuso --- cmd/install/assets/hypershift_operator.go | 4 + .../assets/hypershift_operator_test.go | 18 ++ cmd/install/install.go | 15 ++ .../hostedcluster/hostedcluster_controller.go | 6 + .../hostedcluster/network_policies.go | 57 ++--- .../hostedcluster/network_policies_test.go | 199 +++++++++++++++++- hypershift-operator/main.go | 11 + 7 files changed, 280 insertions(+), 30 deletions(-) diff --git a/cmd/install/assets/hypershift_operator.go b/cmd/install/assets/hypershift_operator.go index 3163fdcbaed1..2a2cd5704db2 100644 --- a/cmd/install/assets/hypershift_operator.go +++ b/cmd/install/assets/hypershift_operator.go @@ -527,6 +527,7 @@ type HyperShiftOperatorDeployment struct { ScaleFromZeroSecret *corev1.Secret ScaleFromZeroSecretKey string ScaleFromZeroProvider string + HCPEgressBlockCIDRs []string } func (o HyperShiftOperatorDeployment) Build() *appsv1.Deployment { @@ -756,6 +757,9 @@ func (o HyperShiftOperatorDeployment) buildArgs() []string { fmt.Sprintf("--enable-ci-debug-output=%t", o.EnableCIDebugOutput), fmt.Sprintf("--private-platform=%s", o.PrivatePlatform), } + for _, cidr := range o.HCPEgressBlockCIDRs { + args = append(args, fmt.Sprintf("--hcp-egress-block-cidrs=%s", cidr)) + } if o.RegistryOverrides != "" { args = append(args, fmt.Sprintf("--registry-overrides=%s", o.RegistryOverrides)) } diff --git a/cmd/install/assets/hypershift_operator_test.go b/cmd/install/assets/hypershift_operator_test.go index 7dcca5ccf9a2..14c19c9de251 100644 --- a/cmd/install/assets/hypershift_operator_test.go +++ b/cmd/install/assets/hypershift_operator_test.go @@ -847,6 +847,24 @@ func TestBuildArgs(t *testing.T) { fmt.Sprintf("--private-platform=%s", hyperv1.AWSPlatform), }, }, + { + name: "When HCPEgressBlockCIDRs is set, it should include one flag per CIDR", + deployment: HyperShiftOperatorDeployment{ + PrivatePlatform: string(hyperv1.NonePlatform), + HCPEgressBlockCIDRs: []string{"10.0.0.0/16", "10.1.0.0/16"}, + }, + expectContains: []string{ + "--hcp-egress-block-cidrs=10.0.0.0/16", + "--hcp-egress-block-cidrs=10.1.0.0/16", + }, + }, + { + name: "When HCPEgressBlockCIDRs is empty, it should not include the flag", + deployment: HyperShiftOperatorDeployment{ + PrivatePlatform: string(hyperv1.NonePlatform), + }, + expectNotContains: []string{"--hcp-egress-block-cidrs"}, + }, } for _, tc := range tests { diff --git a/cmd/install/install.go b/cmd/install/install.go index c13fd743ec1d..59d6ba039fd7 100644 --- a/cmd/install/install.go +++ b/cmd/install/install.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "io" + "net" "os" "strconv" "strings" @@ -156,6 +157,7 @@ type Options struct { ScaleFromZeroCredentialsSecret string ScaleFromZeroCredentialsSecretKey string RenderSensitive bool + HCPEgressBlockCIDRs []string } func (o *Options) Validate() error { @@ -173,10 +175,21 @@ func (o *Options) Validate() error { errs = append(errs, o.validateScaleFromZeroConfig()...) errs = append(errs, o.validateMonitoringConfig()...) errs = append(errs, o.validateMiscConfig()...) + errs = append(errs, o.validateHCPEgressBlockCIDRs()...) return errors.NewAggregate(errs) } +func (o *Options) validateHCPEgressBlockCIDRs() []error { + var errs []error + for _, cidr := range o.HCPEgressBlockCIDRs { + if _, _, err := net.ParseCIDR(cidr); err != nil { + errs = append(errs, fmt.Errorf("invalid --hcp-egress-block-cidrs value %q: %w", cidr, err)) + } + } + return errs +} + func (o *Options) validatePlatformConfig() []error { var errs []error switch hyperv1.PlatformType(o.PrivatePlatform) { @@ -439,6 +452,7 @@ func NewCommand() *cobra.Command { cmd.PersistentFlags().StringVar(&opts.ScaleFromZeroCreds, "scale-from-zero-creds", opts.ScaleFromZeroCreds, "Path to credentials file for scale-from-zero instance type queries") cmd.PersistentFlags().StringVar(&opts.ScaleFromZeroCredentialsSecret, "scale-from-zero-secret", opts.ScaleFromZeroCredentialsSecret, "Name of existing secret containing scale-from-zero credentials (alternative to --scale-from-zero-creds)") cmd.PersistentFlags().StringVar(&opts.ScaleFromZeroCredentialsSecretKey, "scale-from-zero-secret-key", opts.ScaleFromZeroCredentialsSecretKey, "Key within the scale-from-zero credentials secret (default: credentials)") + cmd.PersistentFlags().StringArrayVar(&opts.HCPEgressBlockCIDRs, "hcp-egress-block-cidrs", nil, "Static CIDRs to block in HCP namespace egress NetworkPolicies instead of dynamically-discovered hosting cluster KAS endpoint IPs. When specified, eliminates NetworkPolicy churn during hosting cluster KAS rolling restarts and avoids OVN port-group reconciliation races that can drop traffic to HCP routers. May be specified multiple times.") cmd.RunE = func(cmd *cobra.Command, args []string) error { return InstallHyperShiftOperator(cmd.Context(), cmd.OutOrStdout(), opts) @@ -1294,6 +1308,7 @@ func setupOperatorResources(opts Options, userCABundleCM *corev1.ConfigMap, trus ScaleFromZeroSecret: scaleFromZeroSecret, ScaleFromZeroSecretKey: opts.ScaleFromZeroCredentialsSecretKey, ScaleFromZeroProvider: opts.ScaleFromZeroProvider, + HCPEgressBlockCIDRs: opts.HCPEgressBlockCIDRs, }.Build() operatorService := assets.HyperShiftOperatorService{ Namespace: operatorNamespace, diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index f7c2deac532f..61c5c63bb16c 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -218,6 +218,12 @@ type HostedClusterReconciler struct { // via the shared ingress. Defaults to probeSharedIngressEndpoint. Override // in tests to avoid real network calls. ProbeSharedIngressEndpoint func(context context.Context, serviceIP string, servicePort int, kasHostname string) bool + // HCPEgressBlockCIDRs, when non-empty, provides a static list of CIDRs to + // block in HCP namespace egress NetworkPolicies. These replace the + // dynamically-discovered management cluster KAS endpoint IPs, eliminating + // NetworkPolicy churn during KAS rolling restarts that can trigger OVN + // port-group reconciliation races and cause traffic drops to HCP routers. + HCPEgressBlockCIDRs []string } // +kubebuilder:rbac:groups=hypershift.openshift.io,resources=hostedclusters,verbs=get;list;watch;create;update;patch;delete diff --git a/hypershift-operator/controllers/hostedcluster/network_policies.go b/hypershift-operator/controllers/hostedcluster/network_policies.go index 684fbbd710a4..364c5792d673 100644 --- a/hypershift-operator/controllers/hostedcluster/network_policies.go +++ b/hypershift-operator/controllers/hostedcluster/network_policies.go @@ -70,13 +70,19 @@ func (r *HostedClusterReconciler) reconcileNetworkPolicies(ctx context.Context, return fmt.Errorf("failed to reconcile kube-apiserver network policy: %w", err) } - //nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility - kubernetesEndpoint := &corev1.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "kubernetes", Namespace: "default"}} - if err := r.Get(ctx, client.ObjectKeyFromObject(kubernetesEndpoint), kubernetesEndpoint); err != nil { - return fmt.Errorf("failed to get management cluster network config: %w", err) + var kasBlock []string + if len(r.HCPEgressBlockCIDRs) > 0 { + kasBlock = append(kasBlock, r.HCPEgressBlockCIDRs...) + } else { + //nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility + kubernetesEndpoint := &corev1.Endpoints{ObjectMeta: metav1.ObjectMeta{Name: "kubernetes", Namespace: "default"}} + if err := r.Get(ctx, client.ObjectKeyFromObject(kubernetesEndpoint), kubernetesEndpoint); err != nil { + return fmt.Errorf("getting management cluster kubernetes endpoints: %w", err) + } + kasBlock = kasEndpointsToCIDRs(kubernetesEndpoint) } - if err := r.reconcileManagementKASPolicies(ctx, createOrUpdate, hcluster, hcp, controlPlaneNamespaceName, managementClusterNetwork, kubernetesEndpoint, controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel); err != nil { + if err := r.reconcileManagementKASPolicies(ctx, createOrUpdate, hcluster, hcp, controlPlaneNamespaceName, managementClusterNetwork, kasBlock, controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel); err != nil { return err } @@ -96,7 +102,7 @@ func (r *HostedClusterReconciler) reconcileNetworkPolicies(ctx context.Context, return fmt.Errorf("failed to reconcile monitoring network policy: %w", err) } - if err := r.reconcilePlatformNetworkPolicies(ctx, log, createOrUpdate, hcluster, kubernetesEndpoint, managementClusterNetwork, version, controlPlaneNamespaceName); err != nil { + if err := r.reconcilePlatformNetworkPolicies(ctx, log, createOrUpdate, hcluster, kasBlock, managementClusterNetwork, version, controlPlaneNamespaceName); err != nil { return err } @@ -114,15 +120,14 @@ func (r *HostedClusterReconciler) getManagementClusterNetwork(ctx context.Contex return managementClusterNetwork, nil } -//nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility -func (r *HostedClusterReconciler) reconcileManagementKASPolicies(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, controlPlaneNamespaceName string, managementClusterNetwork *configv1.Network, kubernetesEndpoint *corev1.Endpoints, controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel bool) error { +func (r *HostedClusterReconciler) reconcileManagementKASPolicies(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, controlPlaneNamespaceName string, managementClusterNetwork *configv1.Network, kasBlock []string, controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel bool) error { if !controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel || hcluster.Spec.Platform.Type != hyperv1.AWSPlatform { return nil } policy := networkpolicy.ManagementKASNetworkPolicy(controlPlaneNamespaceName) if _, err := createOrUpdate(ctx, r.Client, policy, func() error { - return reconcileManagementKASNetworkPolicy(policy, managementClusterNetwork, kubernetesEndpoint, r.ManagementClusterCapabilities.Has(capabilities.CapabilityDNS)) + return reconcileManagementKASNetworkPolicy(policy, managementClusterNetwork, kasBlock, r.ManagementClusterCapabilities.Has(capabilities.CapabilityDNS)) }); err != nil { return fmt.Errorf("failed to reconcile kube-apiserver network policy: %w", err) } @@ -139,14 +144,13 @@ func (r *HostedClusterReconciler) reconcileManagementKASPolicies(ctx context.Con return nil } -//nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility -func (r *HostedClusterReconciler) reconcilePlatformNetworkPolicies(ctx context.Context, log logr.Logger, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, kubernetesEndpoint *corev1.Endpoints, managementClusterNetwork *configv1.Network, version semver.Version, controlPlaneNamespaceName string) error { +func (r *HostedClusterReconciler) reconcilePlatformNetworkPolicies(ctx context.Context, log logr.Logger, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, kasBlock []string, managementClusterNetwork *configv1.Network, version semver.Version, controlPlaneNamespaceName string) error { switch hcluster.Spec.Platform.Type { case hyperv1.AWSPlatform, hyperv1.AzurePlatform, hyperv1.GCPPlatform: policy := networkpolicy.PrivateRouterNetworkPolicy(controlPlaneNamespaceName) ingressOnly := version.Major == 4 && version.Minor < 14 if _, err := createOrUpdate(ctx, r.Client, policy, func() error { - return reconcilePrivateRouterNetworkPolicy(policy, hcluster, kubernetesEndpoint, r.ManagementClusterCapabilities.Has(capabilities.CapabilityDNS), managementClusterNetwork, ingressOnly) + return reconcilePrivateRouterNetworkPolicy(policy, hcluster, kasBlock, r.ManagementClusterCapabilities.Has(capabilities.CapabilityDNS), managementClusterNetwork, ingressOnly) }); err != nil { return fmt.Errorf("failed to reconcile private router network policy: %w", err) } @@ -290,8 +294,7 @@ func reconcileKASNetworkPolicy(policy *networkingv1.NetworkPolicy, hcluster *hyp return nil } -//nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility -func reconcilePrivateRouterNetworkPolicy(policy *networkingv1.NetworkPolicy, _ *hyperv1.HostedCluster, kubernetesEndpoint *corev1.Endpoints, isOpenShiftDNS bool, managementClusterNetwork *configv1.Network, ingressOnly bool) error { +func reconcilePrivateRouterNetworkPolicy(policy *networkingv1.NetworkPolicy, _ *hyperv1.HostedCluster, kasBlockExceptions []string, isOpenShiftDNS bool, managementClusterNetwork *configv1.Network, ingressOnly bool) error { httpPort := intstr.FromInt(8080) httpsPort := intstr.FromInt(8443) protocol := corev1.ProtocolTCP @@ -330,11 +333,12 @@ func reconcilePrivateRouterNetworkPolicy(policy *networkingv1.NetworkPolicy, _ * } } - // Allow to any destination not on the management cluster service network - // i.e. block all inter-namespace egress not allowed by other rules. - // Also do not allow Kubernetes endpoint IPs explicitly - // i.e. block access to management cluster KAS. - exceptions := append(kasEndpointsToCIDRs(kubernetesEndpoint), clusterNetworks...) + // Allow to any destination not on the management cluster pod network and + // not on the KAS block CIDRs (either KAS endpoint /32s or the MC machine + // network CIDR depending on operator configuration). + exceptions := make([]string, 0, len(kasBlockExceptions)+len(clusterNetworks)) + exceptions = append(exceptions, kasBlockExceptions...) + exceptions = append(exceptions, clusterNetworks...) policy.Spec.Egress = []networkingv1.NetworkPolicyEgressRule{ { To: []networkingv1.NetworkPolicyPeer{ @@ -815,9 +819,7 @@ func reconcileSameNamespaceNetworkPolicy(policy *networkingv1.NetworkPolicy) err // reconcileManagementKASNetworkPolicy selects pods excluding the ones having NeedManagementKASAccessLabel and specific operands. // It denies egress traffic to the management cluster clusterNetwork and to the KAS endpoints. -// -//nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility -func reconcileManagementKASNetworkPolicy(policy *networkingv1.NetworkPolicy, managementClusterNetwork *configv1.Network, kubernetesEndpoint *corev1.Endpoints, isOpenShiftDNS bool) error { +func reconcileManagementKASNetworkPolicy(policy *networkingv1.NetworkPolicy, managementClusterNetwork *configv1.Network, kasBlockExceptions []string, isOpenShiftDNS bool) error { // Allow traffic to same namespace policy.Spec.Egress = []networkingv1.NetworkPolicyEgressRule{ { @@ -837,11 +839,12 @@ func reconcileManagementKASNetworkPolicy(policy *networkingv1.NetworkPolicy, man } } - // Allow to any destination not on the management cluster service network - // i.e. block all inter-namespace egress not allowed by other rules. - // Also do not allow Kubernetes endpoint IPs explicitly - // i.e. block access to management cluster KAS. - exceptions := append(kasEndpointsToCIDRs(kubernetesEndpoint), clusterNetworks...) + // Allow to any destination not on the management cluster pod network and + // not on the KAS block CIDRs (either KAS endpoint /32s or the MC machine + // network CIDR depending on operator configuration). + exceptions := make([]string, 0, len(kasBlockExceptions)+len(clusterNetworks)) + exceptions = append(exceptions, kasBlockExceptions...) + exceptions = append(exceptions, clusterNetworks...) policy.Spec.Egress = append(policy.Spec.Egress, networkingv1.NetworkPolicyEgressRule{ To: []networkingv1.NetworkPolicyPeer{ diff --git a/hypershift-operator/controllers/hostedcluster/network_policies_test.go b/hypershift-operator/controllers/hostedcluster/network_policies_test.go index 8690cba7f7eb..bb87d1f6fad7 100644 --- a/hypershift-operator/controllers/hostedcluster/network_policies_test.go +++ b/hypershift-operator/controllers/hostedcluster/network_policies_test.go @@ -232,7 +232,7 @@ func TestGCPPrivateRouterNetworkPolicy_IngressOnly(t *testing.T) { policy := networkpolicy.PrivateRouterNetworkPolicy("test-namespace") // Test with ingressOnly = true - err := reconcilePrivateRouterNetworkPolicy(policy, hcluster, kubernetesEndpoint, false, nil, true) + err := reconcilePrivateRouterNetworkPolicy(policy, hcluster, kasEndpointsToCIDRs(kubernetesEndpoint), false, nil, true) if err != nil { t.Fatalf("reconcilePrivateRouterNetworkPolicy with ingressOnly=true failed: %v", err) } @@ -672,7 +672,7 @@ func TestReconcileManagementKASPolicies(t *testing.T) { return controllerutil.OperationResultCreated, nil }) - err := reconciler.reconcileManagementKASPolicies(t.Context(), createOrUpdate, hcluster, hcp, controlPlaneNamespaceName, managementClusterNetwork, kubernetesEndpoint, tc.controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel) + err := reconciler.reconcileManagementKASPolicies(t.Context(), createOrUpdate, hcluster, hcp, controlPlaneNamespaceName, managementClusterNetwork, kasEndpointsToCIDRs(kubernetesEndpoint), tc.controlPlaneOperatorAppliesManagementKASNetworkPolicyLabel) g.Expect(err).ToNot(HaveOccurred()) _, hasManagementKAS := createdPolicies["management-kas"] @@ -805,7 +805,7 @@ func TestReconcilePlatformNetworkPolicies(t *testing.T) { log := ctrl.Log.WithName("test") version := semver.MustParse(tc.version) - err := reconciler.reconcilePlatformNetworkPolicies(t.Context(), log, createOrUpdate, hcluster, kubernetesEndpoint, managementClusterNetwork, version, controlPlaneNamespaceName) + err := reconciler.reconcilePlatformNetworkPolicies(t.Context(), log, createOrUpdate, hcluster, kasEndpointsToCIDRs(kubernetesEndpoint), managementClusterNetwork, version, controlPlaneNamespaceName) g.Expect(err).ToNot(HaveOccurred()) _, hasPrivateRouter := createdPolicies["private-router"] @@ -1470,3 +1470,196 @@ func findConditionByType(conditions []metav1.Condition, condType string) *metav1 } return nil } + +func TestKasEndpointsToCIDRs(t *testing.T) { + tests := []struct { + name string + addresses []string + expectedCIDRs []string + }{ + { + name: "When the endpoint has a single address it should return a single /32 CIDR", + addresses: []string{"10.0.0.1"}, + expectedCIDRs: []string{"10.0.0.1/32"}, + }, + { + name: "When the endpoint has multiple addresses it should return a /32 CIDR for each", + addresses: []string{"10.0.0.1", "10.0.0.2"}, + expectedCIDRs: []string{"10.0.0.1/32", "10.0.0.2/32"}, + }, + { + name: "When the endpoint has no addresses it should return an empty slice", + addresses: []string{}, + expectedCIDRs: []string{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + addresses := make([]corev1.EndpointAddress, 0, len(tc.addresses)) + for _, ip := range tc.addresses { + addresses = append(addresses, corev1.EndpointAddress{IP: ip}) + } + //nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility + kubernetesEndpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{Name: "kubernetes", Namespace: "default"}, + //nolint:staticcheck // SA1019: corev1.EndpointSubset is intentionally used for backward compatibility + Subsets: []corev1.EndpointSubset{ + {Addresses: addresses}, + }, + } + + result := kasEndpointsToCIDRs(kubernetesEndpoint) + + g.Expect(result).To(ConsistOf(tc.expectedCIDRs)) + }) + } +} + +func TestReconcileNetworkPolicies_HCPEgressBlockCIDRs(t *testing.T) { + // Build a minimal AWS hosted cluster that triggers both the private-router + // and management-kas NetworkPolicies. + newAWSCluster := func() (*hyperv1.HostedCluster, *hyperv1.HostedControlPlane) { + hcluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "test-ns"}, + Spec: hyperv1.HostedClusterSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{EndpointAccess: hyperv1.Private}, + }, + }, + } + hcp := &hyperv1.HostedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: manifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name), + }, + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: hyperv1.PlatformSpec{ + Type: hyperv1.AWSPlatform, + AWS: &hyperv1.AWSPlatformSpec{EndpointAccess: hyperv1.Private}, + }, + }, + } + return hcluster, hcp + } + + kasEndpointIP := "10.0.0.1" + machineNetworkCIDR := "10.100.0.0/16" + + tests := []struct { + name string + hcpEgressBlockCIDRs []string + expectedExceptCIDRs []string + expectCIDRNotPresent []string + }{ + { + name: "When HCPEgressBlockCIDRs is empty it should use KAS endpoint /32 CIDRs in private-router egress exceptions", + expectedExceptCIDRs: []string{kasEndpointIP + "/32"}, + expectCIDRNotPresent: []string{machineNetworkCIDR}, + }, + { + name: "When HCPEgressBlockCIDRs is set it should use static CIDRs in private-router egress exceptions", + hcpEgressBlockCIDRs: []string{machineNetworkCIDR}, + expectedExceptCIDRs: []string{machineNetworkCIDR}, + expectCIDRNotPresent: []string{kasEndpointIP + "/32"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + scheme := runtime.NewScheme() + g.Expect(hyperv1.AddToScheme(scheme)).To(Succeed()) + g.Expect(corev1.AddToScheme(scheme)).To(Succeed()) + g.Expect(configv1.AddToScheme(scheme)).To(Succeed()) + g.Expect(networkingv1.AddToScheme(scheme)).To(Succeed()) + + //nolint:staticcheck // SA1019: corev1.Endpoints is intentionally used for backward compatibility + kubernetesEndpoint := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{Name: "kubernetes", Namespace: "default"}, + //nolint:staticcheck // SA1019: corev1.EndpointSubset is intentionally used for backward compatibility + Subsets: []corev1.EndpointSubset{ + {Addresses: []corev1.EndpointAddress{{IP: kasEndpointIP}}}, + }, + } + managementClusterNetwork := &configv1.Network{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.NetworkSpec{ + ClusterNetwork: []configv1.ClusterNetworkEntry{{CIDR: "10.128.0.0/14"}}, + ServiceNetwork: []string{"172.30.0.0/16"}, + }, + } + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(kubernetesEndpoint, managementClusterNetwork).Build() + reconciler := &HostedClusterReconciler{ + Client: fakeClient, + ManagementClusterCapabilities: fakecapabilities.NewSupportAllExcept(), + HCPEgressBlockCIDRs: tc.hcpEgressBlockCIDRs, + } + + createdNetworkPolicies := make(map[string]*networkingv1.NetworkPolicy) + createOrUpdate := upsert.CreateOrUpdateFN(func(_ context.Context, _ client.Client, obj client.Object, f controllerutil.MutateFn) (controllerutil.OperationResult, error) { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + if err := f(); err != nil { + return controllerutil.OperationResultNone, err + } + createdNetworkPolicies[np.Name] = np + } + return controllerutil.OperationResultCreated, nil + }) + + hcluster, hcp := newAWSCluster() + ctx := context.Background() + log := ctrl.Log.WithName("test") + version := semver.MustParse("4.15.0") + + err := reconciler.reconcileNetworkPolicies(ctx, log, createOrUpdate, hcluster, hcp, version, true) + g.Expect(err).ToNot(HaveOccurred()) + + // Verify the private-router policy egress exceptions. + privateRouterPolicy, exists := createdNetworkPolicies["private-router"] + g.Expect(exists).To(BeTrue(), "private-router NetworkPolicy should be created") + + exceptCIDRs := collectExceptCIDRs(privateRouterPolicy) + g.Expect(exceptCIDRs).To(ContainElement("10.128.0.0/14"), "private-router Except list should always retain management cluster pod CIDR") + for _, cidr := range tc.expectedExceptCIDRs { + g.Expect(exceptCIDRs).To(ContainElement(cidr), "private-router Except list should contain %s", cidr) + } + for _, cidr := range tc.expectCIDRNotPresent { + g.Expect(exceptCIDRs).NotTo(ContainElement(cidr), "private-router Except list should NOT contain %s", cidr) + } + + // Verify the management-kas policy egress exceptions use the same kasBlock. + managementKASPolicy, exists := createdNetworkPolicies["management-kas"] + g.Expect(exists).To(BeTrue(), "management-kas NetworkPolicy should be created") + + managementKASExceptCIDRs := collectExceptCIDRs(managementKASPolicy) + g.Expect(managementKASExceptCIDRs).To(ContainElement("10.128.0.0/14"), "management-kas Except list should always retain management cluster pod CIDR") + for _, cidr := range tc.expectedExceptCIDRs { + g.Expect(managementKASExceptCIDRs).To(ContainElement(cidr), "management-kas Except list should contain %s", cidr) + } + for _, cidr := range tc.expectCIDRNotPresent { + g.Expect(managementKASExceptCIDRs).NotTo(ContainElement(cidr), "management-kas Except list should NOT contain %s", cidr) + } + }) + } +} + +// collectExceptCIDRs returns all CIDRs in IPBlock.Except entries across all egress rules. +func collectExceptCIDRs(policy *networkingv1.NetworkPolicy) []string { + var cidrs []string + for _, rule := range policy.Spec.Egress { + for _, peer := range rule.To { + if peer.IPBlock != nil { + cidrs = append(cidrs, peer.IPBlock.Except...) + } + } + } + return cidrs +} diff --git a/hypershift-operator/main.go b/hypershift-operator/main.go index a663242bf0f8..9e4794eda6da 100644 --- a/hypershift-operator/main.go +++ b/hypershift-operator/main.go @@ -18,6 +18,7 @@ import ( "context" "crypto/tls" "fmt" + "net" "os" "strings" "time" @@ -156,6 +157,7 @@ type StartOptions struct { ScaleFromZeroProvider string ScaleFromZeroCreds string EtcdBackupMaxCount int + HCPEgressBlockCIDRs []string } func NewStartCommand() *cobra.Command { @@ -196,6 +198,7 @@ func NewStartCommand() *cobra.Command { cmd.Flags().StringVar(&opts.ScaleFromZeroProvider, "scale-from-zero-provider", opts.ScaleFromZeroProvider, "Platform type for scale-from-zero autoscaling (aws)") cmd.Flags().StringVar(&opts.ScaleFromZeroCreds, "scale-from-zero-creds", opts.ScaleFromZeroCreds, "Path to credentials file for scale-from-zero instance type queries") cmd.Flags().IntVar(&opts.EtcdBackupMaxCount, "etcd-backup-max-count", 5, "Maximum number of completed HCPEtcdBackup CRs to retain per HostedControlPlane") + cmd.Flags().StringArrayVar(&opts.HCPEgressBlockCIDRs, "hcp-egress-block-cidrs", nil, "Static CIDRs to block in HCP namespace egress NetworkPolicies instead of dynamically-discovered hosting cluster KAS endpoint IPs. When specified, eliminates NetworkPolicy churn during hosting cluster KAS rolling restarts and avoids OVN port-group reconciliation races that can drop traffic to HCP routers. May be specified multiple times (e.g. --hcp-egress-block-cidrs=10.0.0.0/16 --hcp-egress-block-cidrs=10.1.0.0/16).") // Attempt to determine featureset prior to adding featuregate flags. // It is safe to get the empty string from this as the empty string is the default featureset. @@ -220,6 +223,13 @@ func NewStartCommand() *cobra.Command { os.Exit(1) } + for _, cidr := range opts.HCPEgressBlockCIDRs { + if _, _, err := net.ParseCIDR(cidr); err != nil { + fmt.Fprintf(os.Stderr, "invalid --hcp-egress-block-cidrs value %q: %v\n", cidr, err) + os.Exit(1) + } + } + if err := run(ctx, &opts, ctrl.Log.WithName("setup")); err != nil { fmt.Println(err) os.Exit(1) @@ -528,6 +538,7 @@ func setupHostedClusterController(ctx context.Context, mgr ctrl.Manager, opts *S EnableEtcdRecovery: enableEtcdRecovery, FeatureSet: featuregate.FeatureSet(), OpenShiftTrustedCAFilePath: "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", + HCPEgressBlockCIDRs: opts.HCPEgressBlockCIDRs, } if opts.OIDCStorageProviderS3BucketName != "" { awsSession := awsutil.NewSession(ctx, "hypershift-operator-oidc-bucket", opts.OIDCStorageProviderS3Credentials, "", "", opts.OIDCStorageProviderS3Region)