WRKLDS-1126:e2e network policy tests#637
Merged
openshift-merge-bot[bot] merged 1 commit intoMay 20, 2026
Merged
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@kaleemsiddiqu: This pull request references WRKLDS-1126 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Ginkgo end-to-end tests and helpers to validate, enforce, and reconcile kube-scheduler-operator NetworkPolicy objects, plus test pod/namespace scaffolding and an increased e2e test timeout. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant KubeAPI as Kubernetes API
participant Operator as Operator/Reconciler
participant Server as Server Pod
participant Client as Client Pod
Test->>KubeAPI: Create Namespace, Server Pod/Service
KubeAPI-->>Test: Pod scheduled and ready
Test->>KubeAPI: Ensure NetworkPolicy exists / apply policy
KubeAPI-->>Test: Policy stored
Test->>KubeAPI: Create Client Pod (continuous probe)
Client->>Server: agnhost connect (port/IP)
alt Allowed
Server-->>Client: TCP handshake succeeds
Client-->>Test: Logs "CONN_OK"
else Blocked
Client-->>Test: Logs "CONN_FAIL"
end
alt Policy mutated or deleted
Test->>KubeAPI: Delete/Patch NetworkPolicy
KubeAPI-->>Operator: Notify change
Operator->>KubeAPI: Reconcile -> restore policy
KubeAPI-->>Test: Policy restored
Test->>KubeAPI: Poll until spec/UID matches expected
end
Test->>KubeAPI: Cleanup resources
KubeAPI-->>Test: Resources removed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 8 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@kaleemsiddiqu: This pull request references WRKLDS-1126 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
test/e2e/network_policy_enforcement.go (1)
70-79: Assert the plaindefault-denystate before addingallow-ingress.The first blocked connectivity check happens after both policies exist, so this doesn't isolate whether
default-denyitself is working.Suggested change
g.GinkgoWriter.Printf("creating default-deny policy in %s\n", nsName) _, err = kubeClient.NetworkingV1().NetworkPolicies(nsName).Create(context.TODO(), defaultDenyPolicy("default-deny", nsName), metav1.CreateOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) + expectConnectivity(kubeClient, nsName, clientLabels, serverIPs, 8080, false) g.By("Adding ingress allow only and verifying traffic is still blocked") g.GinkgoWriter.Printf("creating allow-ingress policy in %s\n", nsName)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 70 - 79, The test never asserts connectivity immediately after creating the default-deny policy, so add a connectivity check right after creating defaultDenyPolicy("default-deny", nsName) to verify traffic is blocked before creating allowIngressPolicy; specifically, after the Create call that uses kubeClient.NetworkingV1().NetworkPolicies(nsName).Create(...) with defaultDenyPolicy, call expectConnectivity(kubeClient, nsName, clientLabels, serverIPs, 8080, false) to assert default-deny is effective, then proceed to create allowIngressPolicy and the subsequent checks.test/e2e/network_policy.go (1)
91-108: Exercisedefault-deny-allreconciliation in both namespaces.This spec validates
default-deny-allinopenshift-kube-schedulerearlier, but only deletes/mutates the operator copy here. A scheduler-namespace reconciliation regression would currently go unnoticed.Suggested change
g.By("Capturing expected NetworkPolicy specs") expectedSchedulerPolicy := getNetworkPolicy(ctx, kubeClient, schedulerNamespace, schedulerPolicyName) + expectedSchedulerDefaultDeny := getNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) expectedSchedulerOperatorPolicy := getNetworkPolicy(ctx, kubeClient, schedulerOperatorNamespace, schedulerOperatorPolicyName) expectedSchedulerOperatorDefaultDeny := getNetworkPolicy(ctx, kubeClient, schedulerOperatorNamespace, defaultDenyAllPolicyName) g.By("Deleting main policies and waiting for restoration") restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerPolicy) + restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerDefaultDeny) restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerOperatorPolicy) g.By("Deleting default-deny-all policy in operator namespace and waiting for restoration") restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerOperatorDefaultDeny) @@ g.By("Mutating main policies and waiting for reconciliation") mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace, schedulerPolicyName) + mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerOperatorNamespace, schedulerOperatorPolicyName)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy.go` around lines 91 - 108, The test only exercises the operator copy of the default-deny-all policy; capture and exercise the scheduler-namespace copy too by calling getNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) (e.g. expectedSchedulerDefaultDeny), then call restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerDefaultDeny) and mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) alongside the existing operator-namespace calls so reconciliation for default-deny-all is validated in both schedulerNamespace and schedulerOperatorNamespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/helpers.go`:
- Around line 571-581: The helper expectConnectivity currently silently returns
when serverIPs is empty, causing false passes; update expectConnectivity to fail
fast when no destination IPs were resolved by adding a guard at the top that
checks len(serverIPs) == 0 and calls the test failure mechanism (e.g., g.Fail or
Ginkgo's Expect/Fail) with a clear message that includes namespace and relevant
context (e.g., clientLabels or port), then return; this ensures missing
service/DNS resolution is reported immediately before calling
expectConnectivityForIP.
- Around line 453-457: The non-empty assertion is checking created.Status.PodIPs
directly which skips the fallback logic in podIPs(); change the check to use the
podIPs helper instead. In the createServerPod flow, call podIPs(created) (as
assigned to ips) and assert o.Expect(ips).NotTo(o.BeEmpty()) rather than
inspecting created.Status.PodIPs, so clusters that only populate Status.PodIP
will pass; keep the subsequent usage of ips as-is.
In `@test/e2e/network_policy_enforcement.go`:
- Around line 88-98: The testSchedulerNetworkPolicyEnforcement function is
missing synchronization with the operator rollout; before fetching
operator-scoped NetworkPolicies or pods (e.g., before the Get on
"kube-scheduler-operator" and the other checks later), call
test.WaitForOperator(...) for the kube-scheduler-operator to ensure the operator
and its target namespace are ready; add similar WaitForOperator calls near the
other blocks referenced (lines around 130-139, 150-159, 180-189) so that
testSchedulerNetworkPolicyEnforcement waits for the operator (use
schedulerOperatorNamespace and the operator name) before touching
operator-scoped policies or pods.
- Around line 118-127: The DNS check is probing the Service VIP on port 53 which
tests service translation rather than the NetworkPolicy that allows egress to
openshift-dns pods on port 5353; change the test to target the DNS pod IPs (use
serviceClusterIPs(dnsSvc) is wrong for this purpose) or resolve the pod IPs for
the "openshift-dns" pods (e.g., list pods in namespace "openshift-dns" with
label matching dns-default) and call expectConnectivity(kubeClient,
schedulerOperatorNamespace, schedulerOperatorLabels, <dnsPodIPs>, 5353, true)
and similarly update the installer/pruner check to use the pod IPs and port 5353
instead of 53 so the assertion exercises the actual NetworkPolicy rule.
- Around line 165-167: The test assertions contradict the scheduler-operator
NetworkPolicy which allows TCP 8443 regardless of source, so update the two
failing cases to expect allow: change expectConnectivity(kubeClient,
"openshift-etcd", map[string]string{"test":"client"}, schedulerOpIPs, 8443,
false) to use true, and update the g.By/GinkgoWriter expectation strings to
reflect "should be allowed" (or similar) for openshift-etcd with custom app
label; ensure both occurrences (the block using g.By(...) and the corresponding
one around lines ~195-197) are changed so the message and the expectConnectivity
boolean match the configured 8443 ingress rule.
- Around line 55-65: Remove the unnecessary assertion that server.Status.PodIPs
is non-empty and rely on the existing podIPs(server) fallback logic;
specifically, delete or skip the
o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) check and simply call
serverIPs := podIPs(server) after obtaining the server pod (variable server) so
pods with only PodIP populated still succeed. Ensure the code continues to call
waitForPodReady(...) and prints serverIPs as before.
---
Nitpick comments:
In `@test/e2e/network_policy_enforcement.go`:
- Around line 70-79: The test never asserts connectivity immediately after
creating the default-deny policy, so add a connectivity check right after
creating defaultDenyPolicy("default-deny", nsName) to verify traffic is blocked
before creating allowIngressPolicy; specifically, after the Create call that
uses kubeClient.NetworkingV1().NetworkPolicies(nsName).Create(...) with
defaultDenyPolicy, call expectConnectivity(kubeClient, nsName, clientLabels,
serverIPs, 8080, false) to assert default-deny is effective, then proceed to
create allowIngressPolicy and the subsequent checks.
In `@test/e2e/network_policy.go`:
- Around line 91-108: The test only exercises the operator copy of the
default-deny-all policy; capture and exercise the scheduler-namespace copy too
by calling getNetworkPolicy(ctx, kubeClient, schedulerNamespace,
defaultDenyAllPolicyName) (e.g. expectedSchedulerDefaultDeny), then call
restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerDefaultDeny) and
mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace,
defaultDenyAllPolicyName) alongside the existing operator-namespace calls so
reconciliation for default-deny-all is validated in both schedulerNamespace and
schedulerOperatorNamespace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f8cf9ca9-1d0b-4cbd-a82a-252b18cc8e2d
📒 Files selected for processing (3)
test/e2e/helpers.gotest/e2e/network_policy.gotest/e2e/network_policy_enforcement.go
Comment on lines
+571
to
+581
| func expectConnectivity(kubeClient kubernetes.Interface, namespace string, clientLabels map[string]string, serverIPs []string, port int32, shouldSucceed bool) { | ||
| g.GinkgoHelper() | ||
|
|
||
| for _, ip := range serverIPs { | ||
| family := "IPv4" | ||
| if isIPv6(ip) { | ||
| family = "IPv6" | ||
| } | ||
| g.GinkgoWriter.Printf("checking %s connectivity %s -> %s expected=%t\n", family, namespace, formatIPPort(ip, port), shouldSucceed) | ||
| expectConnectivityForIP(kubeClient, namespace, clientLabels, ip, port, shouldSucceed) | ||
| } |
There was a problem hiding this comment.
Fail fast when no destination IPs were resolved.
If serverIPs is empty, this helper does nothing and the caller gets a false pass. That can hide real setup failures, especially for service-based checks.
Suggested fix
func expectConnectivity(kubeClient kubernetes.Interface, namespace string, clientLabels map[string]string, serverIPs []string, port int32, shouldSucceed bool) {
g.GinkgoHelper()
+ o.Expect(serverIPs).NotTo(o.BeEmpty(), "no target IPs resolved for connectivity check in %s", namespace)
for _, ip := range serverIPs {
family := "IPv4"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/helpers.go` around lines 571 - 581, The helper expectConnectivity
currently silently returns when serverIPs is empty, causing false passes; update
expectConnectivity to fail fast when no destination IPs were resolved by adding
a guard at the top that checks len(serverIPs) == 0 and calls the test failure
mechanism (e.g., g.Fail or Ginkgo's Expect/Fail) with a clear message that
includes namespace and relevant context (e.g., clientLabels or port), then
return; this ensures missing service/DNS resolution is reported immediately
before calling expectConnectivityForIP.
Comment on lines
+165
to
+167
| g.By("Testing metrics port 8443 from openshift-etcd with custom app label: should be denied") | ||
| g.GinkgoWriter.Printf("expecting deny from openshift-etcd with custom label to %v:8443 (custom app labels not allowed in etcd namespace)\n", schedulerOpIPs) | ||
| expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, false) |
There was a problem hiding this comment.
These expectations contradict the current 8443 ingress rule.
manifests/0000_25_kube-scheduler-operator_01_networkpolicy_operator.yaml allows TCP 8443 with no from clause, so source namespace and labels are ignored. Both assertions currently expect a deny on traffic that the manifest explicitly allows.
Suggested fix
- expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, false)
+ expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, true)Also applies to: 195-197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/network_policy_enforcement.go` around lines 165 - 167, The test
assertions contradict the scheduler-operator NetworkPolicy which allows TCP 8443
regardless of source, so update the two failing cases to expect allow: change
expectConnectivity(kubeClient, "openshift-etcd",
map[string]string{"test":"client"}, schedulerOpIPs, 8443, false) to use true,
and update the g.By/GinkgoWriter expectation strings to reflect "should be
allowed" (or similar) for openshift-etcd with custom app label; ensure both
occurrences (the block using g.By(...) and the corresponding one around lines
~195-197) are changed so the message and the expectConnectivity boolean match
the configured 8443 ingress rule.
Contributor
Author
|
/retest |
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
from
April 16, 2026 12:01
65c2774 to
a7573df
Compare
|
@kaleemsiddiqu: This pull request references WRKLDS-1126 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (6)
test/e2e/network_policy_enforcement.go (4)
195-197:⚠️ Potential issue | 🔴 CriticalSame manifest contradiction as above.
The test expects deny for
openshift-etcdon port 8443, but the manifest'snamespaceSelector: {}allows all namespaces.🐛 Proposed fix
- g.By("Testing metrics policy: etcd namespace with custom app label should be denied") - g.GinkgoWriter.Printf("expecting deny from openshift-etcd with custom label to %v:8443 (custom app labels not allowed in etcd namespace)\n", schedulerOpIPs) - expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "metrics-client"}, schedulerOpIPs, 8443, false) + g.By("Testing metrics policy: etcd namespace can access metrics (open to all namespaces)") + g.GinkgoWriter.Printf("expecting allow from openshift-etcd to %v:8443 (metrics open to all namespaces)\n", schedulerOpIPs) + expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "metrics-client"}, schedulerOpIPs, 8443, true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 195 - 197, The test calls expectConnectivity(...) asserting a deny from the openshift-etcd namespace for custom app labels, but the NetworkPolicy manifest currently has namespaceSelector: {} which matches all namespaces—fix by aligning the manifest with the test: update the NetworkPolicy's namespaceSelector to target only the intended namespace(s) (e.g., restrict to openshift-etcd via a label selector) or, if the policy is intended to be global, change the test expectation in expectConnectivity(...) to expect allow; locate the namespaceSelector in the NetworkPolicy resource and the expectConnectivity(...) call in the test to make the consistent change.
165-167:⚠️ Potential issue | 🔴 CriticalTest expectation contradicts the NetworkPolicy manifest.
The manifest at
manifests/0000_25_kube-scheduler-operator_01_networkpolicy_operator.yamldefines:ingress: - ports: - port: 8443 from: - namespaceSelector: {}
namespaceSelector: {}matches all namespaces, includingopenshift-etcd. This test expectsfalse(deny), but the policy allows traffic from any namespace to port 8443.🐛 Proposed fix
- g.By("Testing metrics port 8443 from openshift-etcd with custom app label: should be denied") - g.GinkgoWriter.Printf("expecting deny from openshift-etcd with custom label to %v:8443 (custom app labels not allowed in etcd namespace)\n", schedulerOpIPs) - expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, false) + g.By("Testing metrics port 8443 from openshift-etcd with custom app label: should be allowed") + g.GinkgoWriter.Printf("expecting allow from openshift-etcd to %v:8443 (metrics open to all namespaces)\n", schedulerOpIPs) + expectConnectivity(kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 165 - 167, The test's expectation contradicts the NetworkPolicy: the policy allows ingress to port 8443 from any namespace (namespaceSelector: {}), so update the test in network_policy_enforcement.go (the g.By/g.GinkgoWriter.Printf and expectConnectivity invocation) to expect allow instead of deny—change the expectConnectivity call for "openshift-etcd" with map[string]string{"test":"client"} to expect true and revise the printed message to indicate an allowed connection to port 8443; reference the expectConnectivity call and the g.GinkgoWriter.Printf line when making this change.
61-65:⚠️ Potential issue | 🟠 MajorUse
podIPs()helper for the non-empty assertion.Same issue as in
helpers.go- directly checkingStatus.PodIPscan fail on clusters where onlyStatus.PodIPis populated.🐛 Proposed fix
server, err := kubeClient.CoreV1().Pods(nsName).Get(context.TODO(), serverName, metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) serverIPs := podIPs(server) + o.Expect(serverIPs).NotTo(o.BeEmpty(), "pod %s/%s has no assigned IPs", nsName, serverName) g.GinkgoWriter.Printf("server pod %s/%s ips=%v\n", nsName, serverName, serverIPs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 61 - 65, The test currently asserts non-empty by checking server.Status.PodIPs which can be empty on some clusters; replace that assertion to use the podIPs() helper instead. Specifically, after retrieving server via kubeClient.CoreV1().Pods(...).Get(...), call podIPs(server) and assert o.Expect(podIPs(server)).NotTo(o.BeEmpty()) (you can reuse the existing serverIPs := podIPs(server) binding and assert against serverIPs) so the check works whether IPs are in Status.PodIPs or Status.PodIP.
118-127:⚠️ Potential issue | 🟠 MajorDNS connectivity check uses wrong port.
The manifest allows egress to DNS pods on port
5353(UDP), but this test probes the service VIP on port53. This tests service translation behavior rather than the NetworkPolicy rule.To validate the actual policy, either:
- Test against DNS pod IPs on port
5353, or- Accept this tests the practical "can reach DNS" behavior (but note it doesn't validate the policy rule itself)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 118 - 127, The test currently targets the DNS service VIP (dnsIPs from serviceClusterIPs(dnsSvc)) on port 53, which checks service translation rather than the NetworkPolicy allowing egress to DNS pods on UDP 5353; change the test to resolve the DNS pod IPs and probe port 5353 instead: fetch DNS pod IPs from the "openshift-dns" pods (use kubeClient.CoreV1().Pods(...) filtering the DNS pods used by the cluster), replace serviceClusterIPs(dnsSvc) with the pod IP list (e.g., dnsPodIPs) and call expectConnectivity(..., dnsPodIPs, 5353, true) for both the schedulerOperatorNamespace/schedulerOperatorLabels and installerLabels checks so the test validates the actual policy targeting pod IP:5353.test/e2e/helpers.go (2)
453-458:⚠️ Potential issue | 🟠 MajorUse the
podIPs()helper for the non-empty assertion.The direct check on
created.Status.PodIPsat line 455 can fail on clusters where onlyStatus.PodIPis populated. ThepodIPs()helper already handles this fallback.🐛 Proposed fix
created, err := kubeClient.CoreV1().Pods(namespace).Get(context.TODO(), name, metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) - ips := podIPs(created) + o.Expect(ips).NotTo(o.BeEmpty(), "pod %s/%s has no assigned IPs", namespace, name) g.GinkgoWriter.Printf("server pod %s/%s ips=%v\n", namespace, name, ips)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 453 - 458, The test currently asserts on created.Status.PodIPs which can be empty when only Status.PodIP is set; instead call the podIPs(created) helper for the non-empty assertion. Replace the o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) check with o.Expect(podIPs(created)).NotTo(o.BeEmpty()), or compute ips := podIPs(created) first and assert o.Expect(ips).NotTo(o.BeEmpty()) before using ips in the GinkgoWriter.Printf; this ensures compatibility with clusters that populate only Status.PodIP.
570-582:⚠️ Potential issue | 🟠 MajorFail fast when
serverIPsis empty.If
serverIPsis empty, the loop does nothing and the caller receives a false pass. This can hide real setup failures, especially when service resolution fails.🐛 Proposed fix
func expectConnectivity(kubeClient kubernetes.Interface, namespace string, clientLabels map[string]string, serverIPs []string, port int32, shouldSucceed bool) { g.GinkgoHelper() + o.Expect(serverIPs).NotTo(o.BeEmpty(), "no target IPs resolved for connectivity check in %s", namespace) for _, ip := range serverIPs {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 570 - 582, The expectConnectivity function can silently succeed when serverIPs is empty; add an early check at the top of expectConnectivity to fail fast if len(serverIPs) == 0 by calling g.Fail (or another test-failure helper) with a clear message including namespace, clientLabels and port so the test reports the missing server IPs instead of falsely passing; keep the rest of the loop and still call expectConnectivityForIP for each IP when non-empty.
🧹 Nitpick comments (2)
test/e2e/helpers.go (2)
113-127: Named ports would be missed by this check.
IntValue()returns0for string-typedIntOrString(named ports). If a policy uses named ports, this function won't match them correctly. For these tests this is likely fine since all ports are numeric, but worth noting if this helper is reused elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 113 - 127, The helper hasPort misses named ports because IntOrString.IntValue() returns 0 for string-typed ports; update hasPort to handle both Int and String forms of p.Port by inspecting the IntOrString type (p.Port.Type or check p.Port.StrVal != ""): if it's an int, compare p.Port.IntValue() to the provided port; if it's a string, compare p.Port.StrVal to the expected name (or to strconv.Itoa(int(port)) if you intend numeric-string matches) and preserve the protocol check; update imports (strconv) as needed and keep the function name hasPort and parameter list or extend the function signature if you need to accept a port name explicitly.
48-51: Modernize deprecated sets API.
sets.NewString()is deprecated in k8s.io/apimachinery in favor of the genericsets.New[string](). Update both occurrences at lines 48 and 70.♻️ Proposed refactor
- policyTypes := sets.NewString() + policyTypes := sets.New[string]() for _, policyType := range policy.Spec.PolicyTypes { policyTypes.Insert(string(policyType)) }Also at line 70:
- if sets.NewString(expr.Values...).Equal(sets.NewString(values...)) { + if sets.New[string](expr.Values...).Equal(sets.New[string](values...)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 48 - 51, The code uses the deprecated sets.NewString() API to build policyTypes from policy.Spec.PolicyTypes; replace both usages of sets.NewString() with the generic constructor sets.New[string]() and keep the same Insert loop (e.g., initialize policyTypes := sets.New[string]() and then for _, policyType := range policy.Spec.PolicyTypes { policyTypes.Insert(string(policyType)) }) to modernize the API while preserving behavior; update both occurrences where sets.NewString() is called.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/e2e/helpers.go`:
- Around line 453-458: The test currently asserts on created.Status.PodIPs which
can be empty when only Status.PodIP is set; instead call the podIPs(created)
helper for the non-empty assertion. Replace the
o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) check with
o.Expect(podIPs(created)).NotTo(o.BeEmpty()), or compute ips := podIPs(created)
first and assert o.Expect(ips).NotTo(o.BeEmpty()) before using ips in the
GinkgoWriter.Printf; this ensures compatibility with clusters that populate only
Status.PodIP.
- Around line 570-582: The expectConnectivity function can silently succeed when
serverIPs is empty; add an early check at the top of expectConnectivity to fail
fast if len(serverIPs) == 0 by calling g.Fail (or another test-failure helper)
with a clear message including namespace, clientLabels and port so the test
reports the missing server IPs instead of falsely passing; keep the rest of the
loop and still call expectConnectivityForIP for each IP when non-empty.
In `@test/e2e/network_policy_enforcement.go`:
- Around line 195-197: The test calls expectConnectivity(...) asserting a deny
from the openshift-etcd namespace for custom app labels, but the NetworkPolicy
manifest currently has namespaceSelector: {} which matches all namespaces—fix by
aligning the manifest with the test: update the NetworkPolicy's
namespaceSelector to target only the intended namespace(s) (e.g., restrict to
openshift-etcd via a label selector) or, if the policy is intended to be global,
change the test expectation in expectConnectivity(...) to expect allow; locate
the namespaceSelector in the NetworkPolicy resource and the
expectConnectivity(...) call in the test to make the consistent change.
- Around line 165-167: The test's expectation contradicts the NetworkPolicy: the
policy allows ingress to port 8443 from any namespace (namespaceSelector: {}),
so update the test in network_policy_enforcement.go (the
g.By/g.GinkgoWriter.Printf and expectConnectivity invocation) to expect allow
instead of deny—change the expectConnectivity call for "openshift-etcd" with
map[string]string{"test":"client"} to expect true and revise the printed message
to indicate an allowed connection to port 8443; reference the expectConnectivity
call and the g.GinkgoWriter.Printf line when making this change.
- Around line 61-65: The test currently asserts non-empty by checking
server.Status.PodIPs which can be empty on some clusters; replace that assertion
to use the podIPs() helper instead. Specifically, after retrieving server via
kubeClient.CoreV1().Pods(...).Get(...), call podIPs(server) and assert
o.Expect(podIPs(server)).NotTo(o.BeEmpty()) (you can reuse the existing
serverIPs := podIPs(server) binding and assert against serverIPs) so the check
works whether IPs are in Status.PodIPs or Status.PodIP.
- Around line 118-127: The test currently targets the DNS service VIP (dnsIPs
from serviceClusterIPs(dnsSvc)) on port 53, which checks service translation
rather than the NetworkPolicy allowing egress to DNS pods on UDP 5353; change
the test to resolve the DNS pod IPs and probe port 5353 instead: fetch DNS pod
IPs from the "openshift-dns" pods (use kubeClient.CoreV1().Pods(...) filtering
the DNS pods used by the cluster), replace serviceClusterIPs(dnsSvc) with the
pod IP list (e.g., dnsPodIPs) and call expectConnectivity(..., dnsPodIPs, 5353,
true) for both the schedulerOperatorNamespace/schedulerOperatorLabels and
installerLabels checks so the test validates the actual policy targeting pod
IP:5353.
---
Nitpick comments:
In `@test/e2e/helpers.go`:
- Around line 113-127: The helper hasPort misses named ports because
IntOrString.IntValue() returns 0 for string-typed ports; update hasPort to
handle both Int and String forms of p.Port by inspecting the IntOrString type
(p.Port.Type or check p.Port.StrVal != ""): if it's an int, compare
p.Port.IntValue() to the provided port; if it's a string, compare p.Port.StrVal
to the expected name (or to strconv.Itoa(int(port)) if you intend numeric-string
matches) and preserve the protocol check; update imports (strconv) as needed and
keep the function name hasPort and parameter list or extend the function
signature if you need to accept a port name explicitly.
- Around line 48-51: The code uses the deprecated sets.NewString() API to build
policyTypes from policy.Spec.PolicyTypes; replace both usages of
sets.NewString() with the generic constructor sets.New[string]() and keep the
same Insert loop (e.g., initialize policyTypes := sets.New[string]() and then
for _, policyType := range policy.Spec.PolicyTypes {
policyTypes.Insert(string(policyType)) }) to modernize the API while preserving
behavior; update both occurrences where sets.NewString() is called.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 702adaae-058b-4f56-97ff-f6fcd01296e0
📒 Files selected for processing (4)
Makefiletest/e2e/helpers.gotest/e2e/network_policy.gotest/e2e/network_policy_enforcement.go
✅ Files skipped from review due to trivial changes (1)
- test/e2e/network_policy.go
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
from
April 17, 2026 11:07
a7573df to
dafff17
Compare
|
@kaleemsiddiqu: This pull request references WRKLDS-1126 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (6)
test/e2e/helpers.go (2)
457-461:⚠️ Potential issue | 🟠 MajorUse
podIPs(created)for the non-empty check too.Line 459 bypasses the fallback in
podIPs(), so this helper still fails on clusters that only populateStatus.PodIP.Suggested fix
created, err := kubeClient.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) ips := podIPs(created) + o.Expect(ips).NotTo(o.BeEmpty(), "pod %s/%s has no assigned IPs", namespace, name) g.GinkgoWriter.Printf("server pod %s/%s ips=%v\n", namespace, name, ips)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 457 - 461, The non-empty check uses created.Status.PodIPs directly and bypasses the fallback logic in podIPs(), so replace that check to call podIPs(created) instead; specifically update the expectation that currently references created.Status.PodIPs to use podIPs(created) (keep the Get call, o.Expect(err).NotTo(o.HaveOccurred()), and the subsequent ips := podIPs(created) as-is).
577-587:⚠️ Potential issue | 🟠 MajorFail fast when no destination IPs were resolved.
If
serverIPsis empty, this helper does nothing and the caller gets a false pass instead of a setup failure.Suggested fix
func expectConnectivity(ctx context.Context, kubeClient kubernetes.Interface, namespace string, clientLabels map[string]string, serverIPs []string, port int32, shouldSucceed bool) { g.GinkgoHelper() + o.Expect(serverIPs).NotTo(o.BeEmpty(), "no target IPs resolved for connectivity check in %s", namespace) for _, ip := range serverIPs { family := "IPv4"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 577 - 587, The helper expectConnectivity currently no-ops when serverIPs is empty causing silent false passes; add a guard at the start of expectConnectivity that checks if len(serverIPs) == 0 and fail fast (e.g., call g.Fail or another Ginkgo fail/abort helper) with a clear message like "no destination IPs resolved for namespace ...", so tests abort immediately when no destination IPs are found.test/e2e/network_policy_enforcement.go (4)
62-65:⚠️ Potential issue | 🟠 MajorDon’t require
Status.PodIPsbefore callingpodIPs().This repeats the same fallback bug as
createServerPod: valid pods can havePodIPpopulated beforePodIPs.Suggested fix
server, err := kubeClient.CoreV1().Pods(nsName).Get(ctx, serverName, metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) serverIPs := podIPs(server) + o.Expect(serverIPs).NotTo(o.BeEmpty(), "pod %s/%s has no assigned IPs", nsName, serverName) g.GinkgoWriter.Printf("server pod %s/%s ips=%v\n", nsName, serverName, serverIPs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 62 - 65, The test wrongly asserts server.Status.PodIPs is non-empty before calling podIPs(), which breaks pods that only populate Status.PodIP; remove the o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) check and instead call serverIPs := podIPs(server) and assert that serverIPs is non-empty (or assert that either server.Status.PodIP or server.Status.PodIPs contains an IP). Also mirror the same fix pattern used in createServerPod to avoid the repeated fallback bug.
169-171:⚠️ Potential issue | 🟠 MajorThese 8443 deny expectations contradict the rest of the metrics suite.
The suite title says metrics ingress is open, and other cases in this file already expect
8443to be allowed fromdefault,openshift-monitoring,openshift-console, and the operator namespace. Keepingopenshift-etcdas a deny here makes the matrix internally inconsistent.Suggested fix
- g.By("Testing metrics port 8443 from openshift-etcd with custom app label: should be denied") - g.GinkgoWriter.Printf("expecting deny from openshift-etcd with custom label to %v:8443 (custom app labels not allowed in etcd namespace)\n", schedulerOpIPs) - expectConnectivity(ctx, kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, false) + g.By("Testing metrics port 8443 from openshift-etcd with custom app label: should be allowed") + g.GinkgoWriter.Printf("expecting allow from openshift-etcd with custom label to %v:8443\n", schedulerOpIPs) + expectConnectivity(ctx, kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, true) - g.By("Testing metrics policy: etcd namespace with custom app label should be denied") - g.GinkgoWriter.Printf("expecting deny from openshift-etcd with custom label to %v:8443 (custom app labels not allowed in etcd namespace)\n", schedulerOpIPs) - expectConnectivity(ctx, kubeClient, "openshift-etcd", map[string]string{"test": "metrics-client"}, schedulerOpIPs, 8443, false) + g.By("Testing metrics policy: etcd namespace with custom app label can access metrics") + g.GinkgoWriter.Printf("expecting allow from openshift-etcd with custom label to %v:8443\n", schedulerOpIPs) + expectConnectivity(ctx, kubeClient, "openshift-etcd", map[string]string{"test": "metrics-client"}, schedulerOpIPs, 8443, true)Also applies to: 200-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 169 - 171, The test incorrectly expects denies for port 8443 from "openshift-etcd" which contradicts the suite title and other test cases; update the two occurrences where expectConnectivity is called for namespace "openshift-etcd" with label {"test":"client"} (the call to expectConnectivity(ctx, kubeClient, "openshift-etcd", map[string]string{"test": "client"}, schedulerOpIPs, 8443, false) and the duplicate at the later block) to expect allow by changing the final argument from false to true so the metrics ingress matrix is consistent with the rest of the suite.
120-129:⚠️ Potential issue | 🟠 MajorProbe DNS pod IPs on 5353, not the service VIP on 53.
This check currently depends on Service translation instead of the policy rule itself. If the policy allows egress to
openshift-dnspods on5353, a correct policy can still fail here, or pass for the wrong reason.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 120 - 129, The test is probing the Service VIP on port 53 but should probe DNS pod IPs on port 5353; replace the serviceClusterIPs(dnsSvc) usage with the actual DNS pod IPs and call expectConnectivity(..., dnsPodIPs, 5353, true) so the policy is validated against pod-targeted egress rules. Locate the block using dnsSvc/serviceClusterIPs and update it to list pods in the "openshift-dns" namespace (filter by the DNS pod label used in the cluster), extract their IPs into dnsPodIPs, and use schedulerOperatorNamespace, schedulerOperatorLabels, schedulerNamespace and installerLabels unchanged when invoking expectConnectivity with port 5353.
89-95:⚠️ Potential issue | 🟠 MajorWait for the operator before touching operator-scoped pods or policies.
These specs start creating pods and reading
openshift-kube-scheduler-operatorresources immediately after client creation. On slower clusters that races rollout and makes the suite flaky.Apply the same guard at the top of each operator-scoped spec.Suggested fix
kubeClient, err := kubernetes.NewForConfig(kubeConfig) o.Expect(err).NotTo(o.HaveOccurred()) + o.Expect(test.WaitForOperator(ctx, kubeClient)).NotTo(o.HaveOccurred())Also applies to: 132-138, 153-159, 184-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 89 - 95, The spec starts creating pods and reading operator-scoped resources immediately after NewClientConfigForTest()/kubernetes.NewForConfig(), causing races; add an explicit wait for the scheduler operator to be ready at the top of testSchedulerNetworkPolicyEnforcement() (e.g., call the existing helper that waits for the openshift-kube-scheduler-operator deployment/namespace/operator to be available) before creating pods or touching operator-scoped policies, and apply the same guard to the other operator-scoped specs referenced (the blocks around lines 132-138, 153-159, 184-189) so all tests wait for operator readiness first.
🧹 Nitpick comments (1)
test/e2e/network_policy.go (1)
91-108: Includeopenshift-kube-scheduler/default-deny-allin the reconcile path.This spec validates that policy earlier, but never deletes or mutates it here. A controller regression limited to the scheduler namespace default-deny would still pass this test.
Suggested coverage update
g.By("Capturing expected NetworkPolicy specs") + expectedSchedulerDefaultDeny := getNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) expectedSchedulerPolicy := getNetworkPolicy(ctx, kubeClient, schedulerNamespace, schedulerPolicyName) expectedSchedulerOperatorPolicy := getNetworkPolicy(ctx, kubeClient, schedulerOperatorNamespace, schedulerOperatorPolicyName) expectedSchedulerOperatorDefaultDeny := getNetworkPolicy(ctx, kubeClient, schedulerOperatorNamespace, defaultDenyAllPolicyName) g.By("Deleting main policies and waiting for restoration") + restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerDefaultDeny) restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerPolicy) restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerOperatorPolicy) g.By("Deleting default-deny-all policy in operator namespace and waiting for restoration") restoreNetworkPolicy(ctx, kubeClient, expectedSchedulerOperatorDefaultDeny) g.By("Mutating main policies and waiting for reconciliation") + mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace, schedulerPolicyName) mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerOperatorNamespace, schedulerOperatorPolicyName)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy.go` around lines 91 - 108, The test currently captures and exercises default-deny in the operator namespace (expectedSchedulerOperatorDefaultDeny/defaultDenyAllPolicyName) but never captures or mutates the scheduler-namespace default-deny, leaving openshift-kube-scheduler/default-deny-all untested; update the test to call getNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) to capture that policy and add matching restoreNetworkPolicy(...) and mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName) calls (following the patterns used for expectedSchedulerPolicy and expectedSchedulerOperatorDefaultDeny) so the scheduler-namespace default-deny is included in the delete/restore and mutate/reconcile paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/e2e/helpers.go`:
- Around line 457-461: The non-empty check uses created.Status.PodIPs directly
and bypasses the fallback logic in podIPs(), so replace that check to call
podIPs(created) instead; specifically update the expectation that currently
references created.Status.PodIPs to use podIPs(created) (keep the Get call,
o.Expect(err).NotTo(o.HaveOccurred()), and the subsequent ips := podIPs(created)
as-is).
- Around line 577-587: The helper expectConnectivity currently no-ops when
serverIPs is empty causing silent false passes; add a guard at the start of
expectConnectivity that checks if len(serverIPs) == 0 and fail fast (e.g., call
g.Fail or another Ginkgo fail/abort helper) with a clear message like "no
destination IPs resolved for namespace ...", so tests abort immediately when no
destination IPs are found.
In `@test/e2e/network_policy_enforcement.go`:
- Around line 62-65: The test wrongly asserts server.Status.PodIPs is non-empty
before calling podIPs(), which breaks pods that only populate Status.PodIP;
remove the o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) check and instead
call serverIPs := podIPs(server) and assert that serverIPs is non-empty (or
assert that either server.Status.PodIP or server.Status.PodIPs contains an IP).
Also mirror the same fix pattern used in createServerPod to avoid the repeated
fallback bug.
- Around line 169-171: The test incorrectly expects denies for port 8443 from
"openshift-etcd" which contradicts the suite title and other test cases; update
the two occurrences where expectConnectivity is called for namespace
"openshift-etcd" with label {"test":"client"} (the call to
expectConnectivity(ctx, kubeClient, "openshift-etcd", map[string]string{"test":
"client"}, schedulerOpIPs, 8443, false) and the duplicate at the later block) to
expect allow by changing the final argument from false to true so the metrics
ingress matrix is consistent with the rest of the suite.
- Around line 120-129: The test is probing the Service VIP on port 53 but should
probe DNS pod IPs on port 5353; replace the serviceClusterIPs(dnsSvc) usage with
the actual DNS pod IPs and call expectConnectivity(..., dnsPodIPs, 5353, true)
so the policy is validated against pod-targeted egress rules. Locate the block
using dnsSvc/serviceClusterIPs and update it to list pods in the "openshift-dns"
namespace (filter by the DNS pod label used in the cluster), extract their IPs
into dnsPodIPs, and use schedulerOperatorNamespace, schedulerOperatorLabels,
schedulerNamespace and installerLabels unchanged when invoking
expectConnectivity with port 5353.
- Around line 89-95: The spec starts creating pods and reading operator-scoped
resources immediately after NewClientConfigForTest()/kubernetes.NewForConfig(),
causing races; add an explicit wait for the scheduler operator to be ready at
the top of testSchedulerNetworkPolicyEnforcement() (e.g., call the existing
helper that waits for the openshift-kube-scheduler-operator
deployment/namespace/operator to be available) before creating pods or touching
operator-scoped policies, and apply the same guard to the other operator-scoped
specs referenced (the blocks around lines 132-138, 153-159, 184-189) so all
tests wait for operator readiness first.
---
Nitpick comments:
In `@test/e2e/network_policy.go`:
- Around line 91-108: The test currently captures and exercises default-deny in
the operator namespace
(expectedSchedulerOperatorDefaultDeny/defaultDenyAllPolicyName) but never
captures or mutates the scheduler-namespace default-deny, leaving
openshift-kube-scheduler/default-deny-all untested; update the test to call
getNetworkPolicy(ctx, kubeClient, schedulerNamespace, defaultDenyAllPolicyName)
to capture that policy and add matching restoreNetworkPolicy(...) and
mutateAndRestoreNetworkPolicy(ctx, kubeClient, schedulerNamespace,
defaultDenyAllPolicyName) calls (following the patterns used for
expectedSchedulerPolicy and expectedSchedulerOperatorDefaultDeny) so the
scheduler-namespace default-deny is included in the delete/restore and
mutate/reconcile paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f45b17b-11c3-4b69-8924-711ec8a243ae
📒 Files selected for processing (4)
Makefiletest/e2e/helpers.gotest/e2e/network_policy.gotest/e2e/network_policy_enforcement.go
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
liouk
suggested changes
Apr 21, 2026
openshift-ci
Bot
added
the
approved
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
from
April 28, 2026 14:06
dafff17 to
feaab93
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
test/e2e/network_policy_enforcement.go (3)
162-191: AddWaitForOperatorcall for consistency.Same pattern applies here—the test creates pods in the operator namespace and relies on the NetworkPolicy being present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 162 - 191, In testMetricsOpenButOtherPortsBlocked add the same WaitForOperator invocation used in other tests before creating pods so the operator's NetworkPolicy is present: call the test helper WaitForOperator(ctx, kubeClient, "kube-scheduler-operator") (or the project’s equivalent WaitForOperator function) at the start of the function (before createServerPod is called) so createServerPod, expectConnectivity, and related checks run only after the operator has reconciled its NetworkPolicy.
141-160: AddWaitForOperatorcall for consistency.This test function also accesses operator-scoped resources without waiting for operator readiness. Same concern applies as with
testSchedulerNetworkPolicyEnforcement().Suggested fix
func testCrossNamespaceIngressEnforcement() { ctx := context.Background() kubeConfig, err := test.NewClientConfigForTest() o.Expect(err).NotTo(o.HaveOccurred()) kubeClient, err := kubernetes.NewForConfig(kubeConfig) o.Expect(err).NotTo(o.HaveOccurred()) + g.By("Waiting for kube scheduler operator to be ready") + err = test.WaitForOperator(ctx, kubeClient) + o.Expect(err).NotTo(o.HaveOccurred()) + g.By("Creating test server pods in kube-scheduler-operator namespace")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 141 - 160, The testCrossNamespaceIngressEnforcement function is missing a WaitForOperator call before it interacts with operator-scoped resources; add a call to WaitForOperator (the same helper used in testSchedulerNetworkPolicyEnforcement) early in testCrossNamespaceIngressEnforcement, before creating server pods or calling createServerPod and expectConnectivity, to ensure the operator is ready; reference the WaitForOperator helper and the function testCrossNamespaceIngressEnforcement when making this change.
193-228: AddWaitForOperatorcall for consistency.Same pattern applies to this test function as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 193 - 228, The testMetricsIngressOpenAccess function is missing the standard WaitForOperator call; after creating the server pod (after the createServerPod call and g.DeferCleanup(cleanupSchedulerOp)) insert a call to WaitForOperator with the same context and client used elsewhere (e.g., WaitForOperator(ctx, kubeClient) or the project’s established signature) so the operator is ready before running the expectConnectivity checks.test/e2e/helpers.go (1)
130-196: Remove unused helper functions and their internal-only dependents.Four functions are unused:
hasIngressFromNamespace,hasIngressAllowAll,hasIngressFromPolicyGroup, andhasEgressAllowAllTCP. Additionally,namespaceSelectorMatchesandhasAnyTCPPortare only called by these unused functions, making them indirectly unused. If these were added for future use, add them when needed rather than keeping dead code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 130 - 196, Remove the dead helper functions and their internal-only dependents: delete hasIngressFromNamespace, hasIngressAllowAll, hasIngressFromPolicyGroup, hasEgressAllowAllTCP plus the helper functions namespaceSelectorMatches and hasAnyTCPPort that are only used by them; ensure you also remove any references or tests that call these functions and run the build/tests to confirm nothing else depends on them, updating imports if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/helpers.go`:
- Around line 130-196: Remove the dead helper functions and their internal-only
dependents: delete hasIngressFromNamespace, hasIngressAllowAll,
hasIngressFromPolicyGroup, hasEgressAllowAllTCP plus the helper functions
namespaceSelectorMatches and hasAnyTCPPort that are only used by them; ensure
you also remove any references or tests that call these functions and run the
build/tests to confirm nothing else depends on them, updating imports if needed.
In `@test/e2e/network_policy_enforcement.go`:
- Around line 162-191: In testMetricsOpenButOtherPortsBlocked add the same
WaitForOperator invocation used in other tests before creating pods so the
operator's NetworkPolicy is present: call the test helper WaitForOperator(ctx,
kubeClient, "kube-scheduler-operator") (or the project’s equivalent
WaitForOperator function) at the start of the function (before createServerPod
is called) so createServerPod, expectConnectivity, and related checks run only
after the operator has reconciled its NetworkPolicy.
- Around line 141-160: The testCrossNamespaceIngressEnforcement function is
missing a WaitForOperator call before it interacts with operator-scoped
resources; add a call to WaitForOperator (the same helper used in
testSchedulerNetworkPolicyEnforcement) early in
testCrossNamespaceIngressEnforcement, before creating server pods or calling
createServerPod and expectConnectivity, to ensure the operator is ready;
reference the WaitForOperator helper and the function
testCrossNamespaceIngressEnforcement when making this change.
- Around line 193-228: The testMetricsIngressOpenAccess function is missing the
standard WaitForOperator call; after creating the server pod (after the
createServerPod call and g.DeferCleanup(cleanupSchedulerOp)) insert a call to
WaitForOperator with the same context and client used elsewhere (e.g.,
WaitForOperator(ctx, kubeClient) or the project’s established signature) so the
operator is ready before running the expectConnectivity checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3ccf0f95-0456-450c-bec5-e2e0cb13b917
📒 Files selected for processing (4)
Makefiletest/e2e/helpers.gotest/e2e/network_policy.gotest/e2e/network_policy_enforcement.go
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
from
April 29, 2026 14:43
feaab93 to
e2dda69
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
test/e2e/helpers.go (2)
459-464:⚠️ Potential issue | 🟠 Major
createServerPodshould assert onips, notcreated.Status.PodIPs.Line 461 bypasses the fallback logic in
podIPs()and can reject valid pods.Proposed fix
created, err := kubeClient.CoreV1().Pods(namespace).Get(ctx, name, metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) ips := podIPs(created) + o.Expect(ips).NotTo(o.BeEmpty(), "pod %s/%s has no assigned IPs", namespace, name) g.GinkgoWriter.Printf("server pod %s/%s ips=%v\n", namespace, name, ips)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 459 - 464, The test currently asserts on created.Status.PodIPs which bypasses podIPs() fallback logic; instead, call podIPs(created) (already stored in ips) and assert that ips is not empty—i.e., replace the o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) check with o.Expect(ips).NotTo(o.BeEmpty()) so the test validates the resolved IP list from the podIPs(created) helper and retains the g.GinkgoWriter.Printf that logs ips.
579-590:⚠️ Potential issue | 🟠 MajorFail fast when no destination IPs are resolved.
expectConnectivitycurrently no-ops on emptyserverIPs, which can produce false-positive passes.Proposed fix
func expectConnectivity(ctx context.Context, kubeClient kubernetes.Interface, namespace string, clientLabels map[string]string, serverIPs []string, port int32, shouldSucceed bool) { g.GinkgoHelper() + o.Expect(serverIPs).NotTo(o.BeEmpty(), "no target IPs resolved for connectivity check: namespace=%s port=%d labels=%v", namespace, port, clientLabels) for _, ip := range serverIPs { family := "IPv4"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/helpers.go` around lines 579 - 590, The function expectConnectivity should fail fast when serverIPs is empty to avoid false-positive test passes; at the top of expectConnectivity (the function that takes ctx, kubeClient, namespace, clientLabels, serverIPs, port, shouldSucceed) add a check for len(serverIPs) == 0 and call g.GinkgoFail (or the appropriate Ginkgo fail helper) with a clear message like "no destination IPs resolved for namespace=<namespace> clientLabels=<clientLabels>" so the test fails immediately instead of silently succeeding.test/e2e/network_policy_enforcement.go (2)
62-66:⚠️ Potential issue | 🟠 MajorUse
podIPs(server)for the non-empty assertion, notStatus.PodIPsdirectly.Line 64 can fail even when Line 65 fallback would produce a valid IP list.
Proposed fix
server, err := kubeClient.CoreV1().Pods(nsName).Get(ctx, serverName, metav1.GetOptions{}) o.Expect(err).NotTo(o.HaveOccurred()) - o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) serverIPs := podIPs(server) + o.Expect(serverIPs).NotTo(o.BeEmpty(), "pod %s/%s has no assigned IPs", nsName, serverName) g.GinkgoWriter.Printf("server pod %s/%s ips=%v\n", nsName, serverName, serverIPs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 62 - 66, The test asserts non-empty Pod IPs using server.Status.PodIPs which can be empty even though podIPs(server) returns valid addresses; update the assertion to call podIPs(server) instead (replace the o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) check with o.Expect(podIPs(server)).NotTo(o.BeEmpty())), keep the subsequent serverIPs := podIPs(server) and the GinkgoWriter log unchanged, and ensure you reference the same server variable returned by kubeClient.CoreV1().Pods(...).Get(...) so the podIPs function is used consistently.
89-100:⚠️ Potential issue | 🟠 MajorWait for operator readiness before operator-namespace policy/pod checks.
These specs can race rollout and intermittently fail without a
test.WaitForOperator(...)gate.Proposed fix pattern (apply to each listed test function)
func testSchedulerNetworkPolicyEnforcement() { ctx := context.Background() kubeConfig, err := test.NewClientConfigForTest() o.Expect(err).NotTo(o.HaveOccurred()) kubeClient, err := kubernetes.NewForConfig(kubeConfig) o.Expect(err).NotTo(o.HaveOccurred()) + g.By("Waiting for kube scheduler operator to be ready") + err = test.WaitForOperator(ctx, kubeClient) + o.Expect(err).NotTo(o.HaveOccurred()) + schedulerOperatorLabels := map[string]string{"app": "openshift-kube-scheduler-operator"}Also applies to: 141-151, 162-172, 193-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/network_policy_enforcement.go` around lines 89 - 100, The testSchedulerNetworkPolicyEnforcement function currently queries resources in the operator namespace before ensuring the operator is ready, which can race; insert a call to test.WaitForOperator(ctx, "kube-scheduler-operator", schedulerOperatorNamespace) (or the appropriate test.WaitForOperator variant used elsewhere) immediately after creating kubeClient and before checking the NetworkPolicy "kube-scheduler-operator"; apply the same pattern in the other test functions referenced (the functions covering the 141-151, 162-172, 193-203 ranges) so each test waits for the operator readiness for the corresponding operator name/namespace before performing namespace NetworkPolicy or Pod checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/helpers.go`:
- Around line 130-219: Several helper functions in test/e2e/helpers.go (e.g.,
hasIngressFromNamespace, hasIngressAllowAll, namespaceSelectorMatches,
hasIngressFromPolicyGroup, logEgressAllowAll, hasEgressAllowAllTCP,
hasAnyTCPPort) are currently unused and trigger golangci-lint errors; fix by
either (A) wiring them into the e2e tests that validate NetworkPolicy behavior
(call hasIngressFromNamespace/hasIngressAllowAll/hasIngressFromPolicyGroup when
asserting ingress rules, call hasEgressAllowAllTCP/logEgressAllowAll when
asserting egress rules, or call namespaceSelectorMatches/hasAnyTCPPort from
existing test helpers), or (B) remove the unused functions from the PR if they
are not needed; alternatively, if you intend to keep them but not use them yet,
add a short comment and a deliberate usage sentinel in tests (e.g., assign to
blank identifier) or add a lint suppression only after team approval.
---
Duplicate comments:
In `@test/e2e/helpers.go`:
- Around line 459-464: The test currently asserts on created.Status.PodIPs which
bypasses podIPs() fallback logic; instead, call podIPs(created) (already stored
in ips) and assert that ips is not empty—i.e., replace the
o.Expect(created.Status.PodIPs).NotTo(o.BeEmpty()) check with
o.Expect(ips).NotTo(o.BeEmpty()) so the test validates the resolved IP list from
the podIPs(created) helper and retains the g.GinkgoWriter.Printf that logs ips.
- Around line 579-590: The function expectConnectivity should fail fast when
serverIPs is empty to avoid false-positive test passes; at the top of
expectConnectivity (the function that takes ctx, kubeClient, namespace,
clientLabels, serverIPs, port, shouldSucceed) add a check for len(serverIPs) ==
0 and call g.GinkgoFail (or the appropriate Ginkgo fail helper) with a clear
message like "no destination IPs resolved for namespace=<namespace>
clientLabels=<clientLabels>" so the test fails immediately instead of silently
succeeding.
In `@test/e2e/network_policy_enforcement.go`:
- Around line 62-66: The test asserts non-empty Pod IPs using
server.Status.PodIPs which can be empty even though podIPs(server) returns valid
addresses; update the assertion to call podIPs(server) instead (replace the
o.Expect(server.Status.PodIPs).NotTo(o.BeEmpty()) check with
o.Expect(podIPs(server)).NotTo(o.BeEmpty())), keep the subsequent serverIPs :=
podIPs(server) and the GinkgoWriter log unchanged, and ensure you reference the
same server variable returned by kubeClient.CoreV1().Pods(...).Get(...) so the
podIPs function is used consistently.
- Around line 89-100: The testSchedulerNetworkPolicyEnforcement function
currently queries resources in the operator namespace before ensuring the
operator is ready, which can race; insert a call to test.WaitForOperator(ctx,
"kube-scheduler-operator", schedulerOperatorNamespace) (or the appropriate
test.WaitForOperator variant used elsewhere) immediately after creating
kubeClient and before checking the NetworkPolicy "kube-scheduler-operator";
apply the same pattern in the other test functions referenced (the functions
covering the 141-151, 162-172, 193-203 ranges) so each test waits for the
operator readiness for the corresponding operator name/namespace before
performing namespace NetworkPolicy or Pod checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 122f2a10-1820-4ceb-85eb-b68ca8202e3e
📒 Files selected for processing (4)
Makefiletest/e2e/helpers.gotest/e2e/network_policy.gotest/e2e/network_policy_enforcement.go
Comment on lines
+130
to
+219
| func hasIngressFromNamespace(rules []networkingv1.NetworkPolicyIngressRule, port int32, namespace string) bool { | ||
| for _, rule := range rules { | ||
| if !hasPort(rule.Ports, corev1.ProtocolTCP, port) { | ||
| continue | ||
| } | ||
| for _, peer := range rule.From { | ||
| if namespaceSelectorMatches(peer.NamespaceSelector, namespace) { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func hasIngressAllowAll(rules []networkingv1.NetworkPolicyIngressRule, port int32) bool { | ||
| for _, rule := range rules { | ||
| if !hasPort(rule.Ports, corev1.ProtocolTCP, port) { | ||
| continue | ||
| } | ||
| if len(rule.From) == 0 { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func namespaceSelectorMatches(selector *metav1.LabelSelector, namespace string) bool { | ||
| if selector == nil { | ||
| return false | ||
| } | ||
| if selector.MatchLabels != nil { | ||
| if selector.MatchLabels["kubernetes.io/metadata.name"] == namespace { | ||
| return true | ||
| } | ||
| } | ||
| for _, expr := range selector.MatchExpressions { | ||
| if expr.Key != "kubernetes.io/metadata.name" { | ||
| continue | ||
| } | ||
| if expr.Operator != metav1.LabelSelectorOpIn { | ||
| continue | ||
| } | ||
| for _, value := range expr.Values { | ||
| if value == namespace { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func hasIngressFromPolicyGroup(rules []networkingv1.NetworkPolicyIngressRule, port int32, policyGroupLabelKey string) bool { | ||
| for _, rule := range rules { | ||
| if !hasPort(rule.Ports, corev1.ProtocolTCP, port) { | ||
| continue | ||
| } | ||
| for _, peer := range rule.From { | ||
| if peer.NamespaceSelector == nil || peer.NamespaceSelector.MatchLabels == nil { | ||
| continue | ||
| } | ||
| if _, ok := peer.NamespaceSelector.MatchLabels[policyGroupLabelKey]; ok { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func logEgressAllowAll(policy *networkingv1.NetworkPolicy) { | ||
| g.GinkgoHelper() | ||
| if hasEgressAllowAll(policy.Spec.Egress) { | ||
| g.GinkgoWriter.Printf("networkpolicy %s/%s: egress allow-all rule present\n", policy.Namespace, policy.Name) | ||
| return | ||
| } | ||
| g.GinkgoWriter.Printf("networkpolicy %s/%s: no egress allow-all rule\n", policy.Namespace, policy.Name) | ||
| } | ||
|
|
||
| func hasEgressAllowAllTCP(rules []networkingv1.NetworkPolicyEgressRule) bool { | ||
| for _, rule := range rules { | ||
| if len(rule.To) != 0 { | ||
| continue | ||
| } | ||
| if hasAnyTCPPort(rule.Ports) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func hasAnyTCPPort(ports []networkingv1.NetworkPolicyPort) bool { |
There was a problem hiding this comment.
Resolve unused helper lint errors before merge.
The current change introduces multiple functions that static analysis reports as unused; if golangci-lint is required, this will block CI. Please either wire these helpers into active tests or remove them from this PR.
Also applies to: 500-510, 592-627, 710-781
🧰 Tools
🪛 golangci-lint (2.11.4)
[error] 130-130: func hasIngressFromNamespace is unused
(unused)
[error] 144-144: func hasIngressAllowAll is unused
(unused)
[error] 156-156: func namespaceSelectorMatches is unused
(unused)
[error] 181-181: func hasIngressFromPolicyGroup is unused
(unused)
[error] 207-207: func hasEgressAllowAllTCP is unused
(unused)
[error] 219-219: func hasAnyTCPPort is unused
(unused)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/helpers.go` around lines 130 - 219, Several helper functions in
test/e2e/helpers.go (e.g., hasIngressFromNamespace, hasIngressAllowAll,
namespaceSelectorMatches, hasIngressFromPolicyGroup, logEgressAllowAll,
hasEgressAllowAllTCP, hasAnyTCPPort) are currently unused and trigger
golangci-lint errors; fix by either (A) wiring them into the e2e tests that
validate NetworkPolicy behavior (call
hasIngressFromNamespace/hasIngressAllowAll/hasIngressFromPolicyGroup when
asserting ingress rules, call hasEgressAllowAllTCP/logEgressAllowAll when
asserting egress rules, or call namespaceSelectorMatches/hasAnyTCPPort from
existing test helpers), or (B) remove the unused functions from the PR if they
are not needed; alternatively, if you intend to keep them but not use them yet,
add a short comment and a deliberate usage sentinel in tests (e.g., assign to
blank identifier) or add a lint suppression only after team approval.
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
4 times, most recently
from
April 30, 2026 15:12
e020141 to
515750d
Compare
Contributor
Author
|
/retest |
liouk
suggested changes
May 18, 2026
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
from
May 18, 2026 12:07
6d5c623 to
3094f63
Compare
openshift-ci-robot
removed
the
verified
Member
|
/lgtm |
Contributor
Author
|
/verified by @kaleemsiddiqu |
openshift-ci-robot
added
the
verified
|
@kaleemsiddiqu: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
xueqzhan
reviewed
May 18, 2026
Signed-off-by: Kaleemullah Siddiqui <ksiddiqu@redhat.com>
kaleemsiddiqu
force-pushed
the
network-policy-tests
branch
from
May 19, 2026 12:46
3094f63 to
b3e14bd
Compare
openshift-ci-robot
removed
the
verified
Contributor
|
We can wait for ci to passed |
Contributor
Author
|
/verified by @kaleemsiddiqu |
openshift-ci-robot
added
the
verified
|
@kaleemsiddiqu: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
Contributor
|
@kaleemsiddiqu: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Contributor
|
/approve Thanks for the timing improvement! |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xueqzhan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Contributor
|
/lgtm |
Contributor
Author
|
/cherry-pick release-4.22 |
|
@kaleemsiddiqu: new pull request created: #652 DetailsIn response to this:
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
network policy tests
Summary by CodeRabbit