fix(networkpolicy): allow DNS and KubeAPI egress for AllowIngressMetrics to enable secure metrics token validation

[Clee2691]

Description

This PR adds DNS and KubeAPI server ports to the egress allow list for the AllowIngressMetrics network policy ruleset, enabling secure metrics token validation

/cc @vparfonov
/assign @jcantrill

• GitHub issue:
• JIRA: https://redhat.atlassian.net/browse/LOG-9642

Summary by CodeRabbit

• Bug Fixes
  • Network policy handling for metrics-related traffic now allows the expected DNS and Kubernetes API server access instead of leaving egress empty.
  • Policy expectations were updated to reflect the correct rule ordering and port references, improving consistency across generated network policies.
  • Documentation comments were clarified to better describe the metrics network policy behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR changes how the AllowIngressMetrics network policy rule set handles egress: instead of denying all egress traffic, it now restricts egress to DNS and Kubernetes API server communication. A new KubeAPIPort constant replaces a hardcoded literal, and associated tests across three files are updated to reflect the new egress expectations.

Changes

AllowIngressMetrics egress behavior change

Layer / File(s)	Summary
Doc comment update api/logging/v1alpha1/log_file_metrics_exporter_types.go	Updates the doc comment describing restricted egress behavior for metrics token validation.
KubeAPIPort constant and policy builder change internal/factory/network_policy.go	Adds KubeAPIPort constant, rewrites NetworkPolicyTypeAllowIngressMetrics to delegate to the restrict-ingress-egress builder, and replaces literal 6443 with the new constant.
Factory-level test updates internal/factory/network_policy_test.go	Updates expected PolicyTypes ordering and egress rules for AllowIngressMetrics, and uses KubeAPIPort in RestrictIngressEgress expectations.
LogFileMetricExporter reconciliation tests internal/metrics/logfilemetricexporter/metric_exporter_test.go	Adds internal/factory import and updates PolicyTypes/egress assertions to expect TCP/UDP ports via factory constants.
Network package reconciliation tests internal/network/network_policy_test.go	Adds internal/factory import, relaxes PolicyTypes assertions to order-insensitive matching, and reworks egress port expectations across multiple scenarios using factory constants.

Estimated code review effort: 2 (Simple) | ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main network policy change and its purpose.
Description check	✅ Passed	The description covers the change, includes /cc and /assign, and references the JIRA issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-for-rh-openshift]

PR Summary by Qodo

Allow DNS and KubeAPI egress for AllowIngressMetrics NetworkPolicy

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Allow DNS and KubeAPI egress for the AllowIngressMetrics network policy ruleset.
• Reuse the restrict-ingress/egress helper to generate consistent policy rules.
• Update unit tests to assert the new egress ports and policy type ordering.

Diagram

graph TD
  A(["LogFileMetricExporter / NetworkPolicy reconcile"]) --> B["NetworkPolicy factory"] --> C["NetworkPolicy (AllowIngressMetrics)"]
  C --> D["DNS egress (UDP named port)"]
  C --> E["KubeAPI egress (TCP 6443)"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Constrain egress by destination selectors (not just ports)

• ➕ Reduces blast radius vs allowing TCP/6443 and DNS to any destination
• ➕ More accurately targets kube-apiserver and DNS endpoints
• ➖ Requires builder/support for to peers (namespace/label selectors or ipBlocks)
• ➖ May be harder to keep portable across OpenShift/Kubernetes variations

2. Make allowed egress for AllowIngressMetrics configurable

• ➕ Gives operators a supported knob for strict environments
• ➕ Avoids future code changes for additional validation endpoints
• ➖ Adds API/configuration surface area
• ➖ Risk of misconfiguration causing hard-to-diagnose outages

Recommendation: The PR’s approach (adding minimal required egress for DNS + KubeAPI) is the right functional fix for enabling secure metrics token validation. If security posture requires tighter scoping, consider a follow-up to restrict egress by destination selectors (kube-apiserver and DNS) rather than by port alone.

Files changed (5) +71 / -37

Bug fix (1) +9 / -9

network_policy.goAllow DNS + KubeAPI egress for AllowIngressMetrics via shared helper +9/-9

Allow DNS + KubeAPI egress for AllowIngressMetrics via shared helper

• Introduces a named constant for the Kube API port and updates AllowIngressMetrics to reuse the restrict-ingress/egress helper so the policy includes DNS (named UDP) and KubeAPI (TCP/6443) egress instead of denying all egress.

internal/factory/network_policy.go

Tests (3) +61 / -27

network_policy_test.goUpdate factory tests for AllowIngressMetrics egress ports +14/-3

Update factory tests for AllowIngressMetrics egress ports

• Adjusts the AllowIngressMetrics expectations to include a single egress rule allowing TCP/6443 and UDP DNS, and updates hard-coded 6443 usage to the new constant.

internal/factory/network_policy_test.go

metric_exporter_test.goAssert metrics exporter NetworkPolicy allows DNS + KubeAPI egress +16/-5

Assert metrics exporter NetworkPolicy allows DNS + KubeAPI egress

• Updates reconciliation tests to validate the presence of the new egress ports and uses order-insensitive assertions for PolicyTypes/ports.

internal/metrics/logfilemetricexporter/metric_exporter_test.go

network_policy_test.goNormalize NetworkPolicy tests for constants and ordering +31/-19

Normalize NetworkPolicy tests for constants and ordering

• Imports factory constants for DNS/KubeAPI ports, uses stable protocol variables, and switches several assertions to ConsistOf to avoid ordering sensitivity while validating egress port sets.

internal/network/network_policy_test.go

Documentation (1) +1 / -1

log_file_metrics_exporter_types.goClarify AllowIngressMetrics intent and required egress +1/-1

Clarify AllowIngressMetrics intent and required egress

• Updates the ruleset constant comment to document that AllowIngressMetrics now permits DNS and Kubernetes API egress for secure metrics token validation.

api/logging/v1alpha1/log_file_metrics_exporter_types.go

[jcantrill]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Clee2691, jcantrill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [jcantrill]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jcantrill]

/lgtm

[qodo-for-rh-openshift]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 9 rules

1. Unrestricted egress destinations 🐞 Bug ⛨ Security

Description

NetworkPolicyTypeAllowIngressMetrics now enables egress via
NetworkPolicyTypeRestrictIngressEgressWithProtocols, but the generated egress rule specifies only
ports (UDP named "dns" and TCP 6443) and no destination peers, so it allows outbound traffic to any
destination on those ports. This contradicts the API/factory documentation that claims egress is
restricted to DNS and the Kubernetes API server and weakens the intended security boundary.

Code

internal/factory/network_policy.go[R62-66]

+// 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)
}

Relevance

⭐ Low

History treats “restrict egress” as port-only allowlist; PRs 3129/3135 implemented this without
destination peers.

PR-#3129
PR-#3135

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

AllowIngressMetrics now calls the restrict helper (which unconditionally creates an egress rule).
That helper’s egress rule only adds ports (no destination peers), and the runtime builder
implementation shown only appends port rules without any API for to peers, so the resulting
NetworkPolicy cannot actually restrict egress destinations as the comments claim.

internal/factory/network_policy.go[62-66]
internal/factory/network_policy.go[68-90]
internal/runtime/network_policy.go[134-177]
api/logging/v1alpha1/log_file_metrics_exporter_types.go[43-54]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AllowIngressMetrics` is documented as restricting egress to DNS and the Kubernetes API server, but the generated NetworkPolicy egress rule includes only `ports` and no `to` peers. In Kubernetes NetworkPolicy semantics, this permits egress to **any destination** as long as the destination port matches, which is broader than described.

### Issue Context
- `NetworkPolicyTypeAllowIngressMetrics` now delegates to `NetworkPolicyTypeRestrictIngressEgressWithProtocols`, which always creates an egress rule.
- The builder currently only supports adding ports; it does not provide a way to set destination peers (`to`).

### Fix Focus Areas
- internal/factory/network_policy.go[62-90]
- internal/runtime/network_policy.go[134-177]
- api/logging/v1alpha1/log_file_metrics_exporter_types.go[43-54]

### What to change
Implement one of the following (prefer #1 if the goal is true destination restriction):
1. **Add destination restrictions**: extend the `runtime.EgressRuleBuilder` (and underlying rule construction) to support setting `NetworkPolicyPeer` destinations (`To`), and in `AllowIngressMetrics` (or the helper) restrict egress peers to the actual DNS and kube-apiserver endpoints you intend to allow.
2. **Make documentation accurate**: if port-only egress is the intended design, update the API/factory comments to explicitly state that egress is allowed on the DNS and KubeAPI ports **to any destination**, not restricted to those specific services.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[coderabbitai]

🧹 Nitpick comments (1)

internal/metrics/logfilemetricexporter/metric_exporter_test.go (1)

235-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting a shared egress-port matcher.

The same KubeAPI-TCP + DNS-UDP ConsistOf block is duplicated here and in internal/network/network_policy_test.go (and reportedly in internal/factory/network_policy_test.go per the stack). A small shared helper (e.g., returning the two expected networkingv1.NetworkPolicyPort matchers) would reduce duplication and keep future port changes in sync across suites.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/metrics/logfilemetricexporter/metric_exporter_test.go` around lines
235 - 246, The egress-port assertion in the metrics exporter test duplicates the
same KubeAPI-TCP plus DNS-UDP matcher used elsewhere, so extract a shared helper
for the expected ports. Add a reusable function or test helper around the
existing `ConsistOf`/`networkingv1.NetworkPolicyPort` setup, then update this
`metric_exporter_test.go` check and the matching assertions in
`internal/network/network_policy_test.go` and
`internal/factory/network_policy_test.go` to use it.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/metrics/logfilemetricexporter/metric_exporter_test.go`:
- Around line 235-246: The egress-port assertion in the metrics exporter test
duplicates the same KubeAPI-TCP plus DNS-UDP matcher used elsewhere, so extract
a shared helper for the expected ports. Add a reusable function or test helper
around the existing `ConsistOf`/`networkingv1.NetworkPolicyPort` setup, then
update this `metric_exporter_test.go` check and the matching assertions in
`internal/network/network_policy_test.go` and
`internal/factory/network_policy_test.go` to use it.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: fceab307-480b-4170-ae8a-0781d1ca2724

📥 Commits

Reviewing files that changed from the base of the PR and between 7f27cca and 096f37b.

📒 Files selected for processing (5)

• api/logging/v1alpha1/log_file_metrics_exporter_types.go
• internal/factory/network_policy.go
• internal/factory/network_policy_test.go
• internal/metrics/logfilemetricexporter/metric_exporter_test.go
• internal/network/network_policy_test.go

[openshift-ci]

@Clee2691: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-using-bundle	096f37b	link	false	/test e2e-using-bundle

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.