From 096f37bf376a451734415165347192e0f2bce6c0 Mon Sep 17 00:00:00 2001 From: Calvin Lee Date: Wed, 8 Jul 2026 12:35:07 -0400 Subject: [PATCH] fix(networkpolicy): allow DNS and KubeAPI egress for AllowIngressMetrics to enable secure metrics token validation --- .../log_file_metrics_exporter_types.go | 2 +- internal/factory/network_policy.go | 18 +++---- internal/factory/network_policy_test.go | 17 +++++-- .../metric_exporter_test.go | 21 ++++++-- internal/network/network_policy_test.go | 50 ++++++++++++------- 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/api/logging/v1alpha1/log_file_metrics_exporter_types.go b/api/logging/v1alpha1/log_file_metrics_exporter_types.go index da6ceb10a6..8d118c3431 100644 --- a/api/logging/v1alpha1/log_file_metrics_exporter_types.go +++ b/api/logging/v1alpha1/log_file_metrics_exporter_types.go @@ -46,7 +46,7 @@ type NetworkPolicy struct { type NetworkPolicyRuleSetType string const ( - // NetworkPolicyRuleSetTypeAllowIngressMetrics is the type of network policy rule set to use for allowing only ingress metrics + // NetworkPolicyRuleSetTypeAllowIngressMetrics allows ingress on the metrics port and restricts egress to DNS and the Kubernetes API server for secure metrics token validation NetworkPolicyRuleSetTypeAllowIngressMetrics NetworkPolicyRuleSetType = "AllowIngressMetrics" // NetworkPolicyRuleSetTypeAllowAllIngressEgress is the type of network policy rule set to use for allowing all ingress and egress traffic diff --git a/internal/factory/network_policy.go b/internal/factory/network_policy.go index 0c3420ff6f..810e13a750 100644 --- a/internal/factory/network_policy.go +++ b/internal/factory/network_policy.go @@ -9,7 +9,10 @@ import ( networkingv1 "k8s.io/api/networking/v1" ) -const DNSPortName = "dns" +const ( + DNSPortName = "dns" + KubeAPIPort int32 = 6443 +) // PortProtocol represents a port with its associated protocol type PortProtocol struct { @@ -35,7 +38,7 @@ func NewNetworkPolicyWithProtocolPorts(namespace, policyName, instanceName, comp // allow all ingress and egress traffic case string(loggingv1alpha1.NetworkPolicyRuleSetTypeAllowAllIngressEgress): NetworkPolicyTypeAllowAllIngressEgress(npBuilder) - // allow ingress on the metrics port only and deny all egress traffic + // allow ingress on the metrics port only; restrict egress to DNS and KubeAPI for secure metrics token validation case string(loggingv1alpha1.NetworkPolicyRuleSetTypeAllowIngressMetrics): NetworkPolicyTypeAllowIngressMetrics(npBuilder, metricsPort) @@ -56,13 +59,10 @@ func NetworkPolicyTypeAllowAllIngressEgress(npBuilder *runtime.NetworkPolicyBuil AllowAllEgress() } -// NetworkPolicyTypeAllowIngressMetrics configures the network policy to allow ingress on the metrics port only and deny all egress traffic. +// NetworkPolicyTypeAllowIngressMetrics configures the network policy to allow ingress on the metrics port only +// and restrict egress to only the Kubernetes API server and DNS (required for secure metrics token validation). func NetworkPolicyTypeAllowIngressMetrics(npBuilder *runtime.NetworkPolicyBuilder, port int32) *runtime.NetworkPolicyBuilder { - return npBuilder. - WithEgressPolicyType(). // Adding egress policy type without any rules to deny all egress traffic - NewIngressRule(). - OnPort(corev1.ProtocolTCP, port). - End() + return NetworkPolicyTypeRestrictIngressEgressWithProtocols(npBuilder, nil, nil, port) } // NetworkPolicyTypeRestrictIngressEgressWithProtocols configures the network policy to restrict ingress and egress traffic @@ -81,7 +81,7 @@ func NetworkPolicyTypeRestrictIngressEgressWithProtocols(npBuilder *runtime.Netw // Egress rules are allowed on all spec'd egress ports with their detected protocols egressRule := npBuilder.NewEgressRule(). OnNamedPort(corev1.ProtocolUDP, DNSPortName). // allow egress to openshift DNS service port - OnPort(corev1.ProtocolTCP, 6443) // allow egress to KubeAPI port + OnPort(corev1.ProtocolTCP, KubeAPIPort) // allow egress to KubeAPI port for _, portProtocol := range egressPorts { egressRule.OnPort(portProtocol.Protocol, portProtocol.Port) diff --git a/internal/factory/network_policy_test.go b/internal/factory/network_policy_test.go index 9836f5f0cc..eaa1db8ee8 100644 --- a/internal/factory/network_policy_test.go +++ b/internal/factory/network_policy_test.go @@ -136,14 +136,25 @@ var _ = Describe("#NewNetworkPolicy", func() { string(loggingv1alpha1.NetworkPolicyRuleSetTypeAllowIngressMetrics), nil, nil, - []networkingv1.PolicyType{networkingv1.PolicyTypeEgress, networkingv1.PolicyTypeIngress}, + []networkingv1.PolicyType{networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress}, []networkingv1.NetworkPolicyIngressRule{{ Ports: []networkingv1.NetworkPolicyPort{{ Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], Port: &[]intstr.IntOrString{{Type: intstr.Int, IntVal: constants.MetricsPort}}[0], }}, }}, - nil, // No egress rules + []networkingv1.NetworkPolicyEgressRule{{ + Ports: []networkingv1.NetworkPolicyPort{ + { + Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Port: &[]intstr.IntOrString{{Type: intstr.Int, IntVal: KubeAPIPort}}[0], + }, + { + Protocol: &[]corev1.Protocol{corev1.ProtocolUDP}[0], + Port: &[]intstr.IntOrString{{Type: intstr.String, StrVal: DNSPortName}}[0], + }, + }, + }}, ), Entry("with RestrictIngressEgress ruleset", string(obsv1.NetworkPolicyRuleSetTypeRestrictIngressEgress), @@ -170,7 +181,7 @@ var _ = Describe("#NewNetworkPolicy", func() { }, { Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], - Port: &[]intstr.IntOrString{{Type: intstr.Int, IntVal: 6443}}[0], + Port: &[]intstr.IntOrString{{Type: intstr.Int, IntVal: KubeAPIPort}}[0], }, { Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], diff --git a/internal/metrics/logfilemetricexporter/metric_exporter_test.go b/internal/metrics/logfilemetricexporter/metric_exporter_test.go index c6777d7d35..901f2c3d66 100644 --- a/internal/metrics/logfilemetricexporter/metric_exporter_test.go +++ b/internal/metrics/logfilemetricexporter/metric_exporter_test.go @@ -11,6 +11,7 @@ import ( securityv1 "github.com/openshift/api/security/v1" loggingv1alpha1 "github.com/openshift/cluster-logging-operator/api/logging/v1alpha1" "github.com/openshift/cluster-logging-operator/internal/constants" + "github.com/openshift/cluster-logging-operator/internal/factory" "github.com/openshift/cluster-logging-operator/internal/runtime" "github.com/openshift/cluster-logging-operator/internal/utils" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" @@ -217,9 +218,9 @@ var _ = Describe("Reconcile LogFileMetricExporter", func() { expectedPodSelector := runtime.Selectors(lfmeInstance.Name, constants.LogfilesmetricexporterName, constants.LogfilesmetricexporterName) Expect(networkPolicyInstance.Spec.PodSelector.MatchLabels).To(Equal(expectedPodSelector)) - // Verify policy types include Egress and Ingress (AllowIngressMetrics ruleset) - expectedPolicyTypes := []networkingv1.PolicyType{networkingv1.PolicyTypeEgress, networkingv1.PolicyTypeIngress} - Expect(networkPolicyInstance.Spec.PolicyTypes).To(Equal(expectedPolicyTypes)) + // Verify policy types include Ingress and Egress (AllowIngressMetrics ruleset) + expectedPolicyTypes := []networkingv1.PolicyType{networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress} + Expect(networkPolicyInstance.Spec.PolicyTypes).To(ConsistOf(expectedPolicyTypes)) // Verify ingress rules allow only the named metrics port Expect(networkPolicyInstance.Spec.Ingress).To(HaveLen(1)) @@ -231,8 +232,18 @@ var _ = Describe("Reconcile LogFileMetricExporter", func() { } Expect(networkPolicyInstance.Spec.Ingress[0].Ports[0]).To(Equal(expectedPort)) - // Verify egress rules are not present to deny all egress traffic - Expect(networkPolicyInstance.Spec.Egress).To(HaveLen(0)) + // Verify egress rules allow DNS and KubeAPI (required for secure metrics token validation) + Expect(networkPolicyInstance.Spec.Egress).To(HaveLen(1)) + Expect(networkPolicyInstance.Spec.Egress[0].Ports).To(ConsistOf( + networkingv1.NetworkPolicyPort{ + Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Port: &[]intstr.IntOrString{{Type: intstr.Int, IntVal: factory.KubeAPIPort}}[0], + }, + networkingv1.NetworkPolicyPort{ + Protocol: &[]corev1.Protocol{corev1.ProtocolUDP}[0], + Port: &[]intstr.IntOrString{{Type: intstr.String, StrVal: factory.DNSPortName}}[0], + }, + )) // Verify common labels are set expectedLabels := map[string]string{ diff --git a/internal/network/network_policy_test.go b/internal/network/network_policy_test.go index 4d4c308660..220fb337fc 100644 --- a/internal/network/network_policy_test.go +++ b/internal/network/network_policy_test.go @@ -8,6 +8,7 @@ import ( loggingv1alpha1 "github.com/openshift/cluster-logging-operator/api/logging/v1alpha1" obsv1 "github.com/openshift/cluster-logging-operator/api/observability/v1" "github.com/openshift/cluster-logging-operator/internal/constants" + "github.com/openshift/cluster-logging-operator/internal/factory" "github.com/openshift/cluster-logging-operator/internal/runtime" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -41,6 +42,7 @@ var _ = Describe("Reconcile NetworkPolicy", func() { ) instanceName = "test-instance" protocolTCP = corev1.ProtocolTCP + protocolUDP = corev1.ProtocolUDP ) Context("when the collector NetworkPolicy is reconciled", func() { @@ -93,7 +95,7 @@ var _ = Describe("Reconcile NetworkPolicy", func() { networkingv1.PolicyTypeIngress, networkingv1.PolicyTypeEgress, } - Expect(policyInstance.Spec.PolicyTypes).To(Equal(expectedPolicyTypes)) + Expect(policyInstance.Spec.PolicyTypes).To(ConsistOf(expectedPolicyTypes)) expectedIngressRules := []networkingv1.NetworkPolicyIngressRule{ {}, @@ -224,24 +226,24 @@ var _ = Describe("Reconcile NetworkPolicy", func() { expectedEgressPorts := []networkingv1.NetworkPolicyPort{ { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Protocol: &protocolTCP, Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 9200}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolUDP}[0], - Port: &intstr.IntOrString{Type: intstr.String, StrVal: "dns"}, + Protocol: &protocolUDP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: factory.DNSPortName}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Protocol: &protocolTCP, Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 8088}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Protocol: &protocolTCP, Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 3100}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], - Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 6443}, + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: factory.KubeAPIPort}, }, } @@ -329,20 +331,20 @@ var _ = Describe("Reconcile NetworkPolicy", func() { expectedEgressPorts := []networkingv1.NetworkPolicyPort{ { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Protocol: &protocolTCP, Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 9200}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], + Protocol: &protocolTCP, Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 9092}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0], - Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 6443}, + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: factory.KubeAPIPort}, }, { - Protocol: &[]corev1.Protocol{corev1.ProtocolUDP}[0], - Port: &intstr.IntOrString{Type: intstr.String, StrVal: "dns"}, + Protocol: &protocolUDP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: factory.DNSPortName}, }, } Expect(egressRule.Ports).To(ConsistOf(expectedEgressPorts)) @@ -391,12 +393,12 @@ var _ = Describe("Reconcile NetworkPolicy", func() { expectedPodSelector := runtime.Selectors(instanceName, componentName, constants.LogfilesmetricexporterName) Expect(policyInstance.Spec.PodSelector.MatchLabels).To(Equal(expectedPodSelector)) - // Verify policy types includes Egress and Ingress + // Verify policy types includes Ingress and Egress expectedPolicyTypes := []networkingv1.PolicyType{ - networkingv1.PolicyTypeEgress, networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, } - Expect(policyInstance.Spec.PolicyTypes).To(Equal(expectedPolicyTypes)) + Expect(policyInstance.Spec.PolicyTypes).To(ConsistOf(expectedPolicyTypes)) // Verify ingress rules allow only the named metrics port expectedIngressRules := []networkingv1.NetworkPolicyIngressRule{ @@ -411,8 +413,18 @@ var _ = Describe("Reconcile NetworkPolicy", func() { } Expect(policyInstance.Spec.Ingress).To(Equal(expectedIngressRules)) - // Verify egress rules are not present to deny all egress traffic - Expect(policyInstance.Spec.Egress).To(BeNil()) + // Verify egress rules allow DNS and KubeAPI (required for secure metrics token validation) + Expect(policyInstance.Spec.Egress).To(HaveLen(1)) + Expect(policyInstance.Spec.Egress[0].Ports).To(ConsistOf( + networkingv1.NetworkPolicyPort{ + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: factory.KubeAPIPort}, + }, + networkingv1.NetworkPolicyPort{ + Protocol: &protocolUDP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: factory.DNSPortName}, + }, + )) // Verify common labels are applied Expect(policyInstance.Labels).To(HaveKey(constants.LabelK8sName))