✨OPRUN-3873: Add e2e tests for NetworkPolicies #2013

anik120 · 2025-06-05T14:56:43Z

Description

This PR introduces a new e2e test to continuously validate the NetworkPolicy objects for the catalogd and operator-controller components.

The new test, TestNetworkPolicyJustifications, achieves the following:

Documents NetworkPolicies deployed: A registry of all allowed network policies (allowedNetworkPolicies) is defined using the RFC: OLMv1 Static Network Policies. Every rule within these policies is explicitly registered in the registry with a clear, written justification of a minimum length. This ensures that every rule's purpose is documented and understood.
Prevents Regressions: The test validates that the NetworkPolicy objects deployed in the cluster exactly match the definitions in a registry (allowedNetworkPolicies). This prevents undocumented or accidental changes to our network rules.
Improves Security Posture: Enforces thoughtful consideration for any change to existing network rules, improving overall security posture. Also ensures that other, unrelated network policies won't inadvertently break existing functionalities.

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2025-06-05T14:56:47Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`b61ee05`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/684883845e058b0008e2ca5c
😎 Deploy Preview	https://deploy-preview-2013--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

openshift-ci · 2025-06-05T14:56:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign perdasilva for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

joelanford · 2025-06-05T17:23:58Z

test/e2e/network_policy_test.go

+			},
+			{
+				Ports:         []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(9443)}},
+				Justification: "Permits Kubernetes API server to reach catalogd's admission webhook for CRD validation, ensuring integrity of catalog resources.",


It's a mutating webhook that sets a label for the metadata.name that can be used by selectors in ClusterExtension objects to choose specific catalogs by name.

Suggested change

Justification: "Permits Kubernetes API server to reach catalogd's admission webhook for CRD validation, ensuring integrity of catalog resources.",

Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",

joelanford · 2025-06-05T17:24:30Z

test/e2e/network_policy_test.go

+			},
+			{
+				Ports:         []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(8443)}},
+				Justification: "Enables operator-controller to query catalog metadata from catalogd. This is a core function for bundle resolution and operator discovery.",


Suggested change

Justification: "Enables operator-controller to query catalog metadata from catalogd. This is a core function for bundle resolution and operator discovery.",

Justification: "Enables clients (e.g. operator-controller) to query catalog metadata from catalogd. This is a core function for bundle resolution and operator discovery.",

joelanford · 2025-06-05T17:25:16Z

test/e2e/network_policy_test.go

+		EgressRules: []EgressRule{
+			{
+				// Empty Ports and To means allow all egress
+				Justification: "Permits catalogd to fetch catalog images from various container registries and communicate with the Kubernetes API server for its operational needs.",


Suggested change

Justification: "Permits catalogd to fetch catalog images from various container registries and communicate with the Kubernetes API server for its operational needs.",

Justification: "Permits catalogd to fetch catalog images from arbitrary container registries and communicate with the Kubernetes API server for its operational needs.",

joelanford · 2025-06-05T17:26:00Z

test/e2e/network_policy_test.go

+		EgressRules: []EgressRule{
+			{
+				// Empty Ports and To means allow all egress
+				Justification: "Enables operator-controller to pull bundle images, connect to catalogd's HTTPS server for metadata, and interact with the Kubernetes API server.",


Suggested change

Justification: "Enables operator-controller to pull bundle images, connect to catalogd's HTTPS server for metadata, and interact with the Kubernetes API server.",

Justification: "Enables operator-controller to pull bundle images from arbitrary image registries, connect to catalogd's HTTPS server for metadata, and interact with the Kubernetes API server.",

anik120 · 2025-06-05T18:43:31Z

@joelanford incorporated your suggestions..also fyi had to change the struct definitions a bit because I realized the policies are defined as a single ingress/egress policy with multiple ports, instead of multiple ingress/egress policies, for each NetworkPolicy object. PTAL, thanks!

codecov · 2025-06-05T19:58:27Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.16%. Comparing base (8f81c23) to head (b61ee05).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2013      +/-   ##
==========================================
- Coverage   69.17%   69.16%   -0.02%     
==========================================
  Files          79       79              
  Lines        7037     7037              
==========================================
- Hits         4868     4867       -1     
- Misses       1887     1888       +1     
  Partials      282      282

Flag	Coverage Δ
e2e	`43.00% <ø> (ø)`
unit	`60.05% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

test/e2e/network_policy_test.go

dtfranz · 2025-06-10T05:47:27Z

test/e2e/network_policy_test.go

+				{
+					Port:          []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(8443)}},
+					Justification: "Allows Prometheus to scrape metrics from catalogd, which is essential for monitoring its performance and health.",
+				},
+				{
+					Port:          []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(9443)}},
+					Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",
+				},
+				{
+					Port:          []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(7443)}},
+					Justification: "Enables clients (eg. operator-controller) to query catalog metadata from catalogd, which is a core function for bundle resolution and operator discovery.",
+				},


Suggested change

{

Port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(8443)}},

Justification: "Allows Prometheus to scrape metrics from catalogd, which is essential for monitoring its performance and health.",

},

{

Port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(9443)}},

Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",

},

{

Port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(7443)}},

Justification: "Enables clients (eg. operator-controller) to query catalog metadata from catalogd, which is a core function for bundle resolution and operator discovery.",

},

{

Port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(7443)}},

Justification: "Allows Prometheus to scrape metrics from catalogd, which is essential for monitoring its performance and health.",

},

{

Port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(9443)}},

Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",

},

{

Port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(8443)}},

Justification: "Enables clients (eg. operator-controller) to query catalog metadata from catalogd, which is a core function for bundle resolution and operator discovery.",

},

that's a great catch thank you @dtfranz!! Updated the PR, PTAL

perdasilva · 2025-06-10T07:51:59Z

test/e2e/network_policy_test.go

+					Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",
+				},
+				{
+					Port:          []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(7443)}},


nit: maybe move the ports to consts?

👍🏽 moved them to constants

perdasilva · 2025-06-10T07:55:03Z

test/e2e/network_policy_test.go

+	operatorManagerSelector = "control-plane=operator-controller-controller-manager"
+)
+
+type PortWithJustification struct {


nit: do these need to be exported? (PortWithJustification, IngressRule, EgressRule, etc.)

Good point, un-exported them

perdasilva · 2025-06-10T07:57:51Z

test/e2e/network_policy_test.go

+				},
+				{
+					Port:          []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(9443)}},
+					Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",


kube apiserver and dns ports could change downstream (or in different installations). We might want a way to configure those =S

hhhmm really good point.

So I was looking at the manifests downstream: looks like we have the same ports configured for now (see catalogd's network policy and operator-controller's network policy).

We could configure this using a env vars:

# Makefile export E2E_CATALOGD_METRICS_PORT ?= 7443 export E2E_SERVER_PORT ?= 8443

and then read those env vars in the test, which would enable us to configure these variable in the Makefile downstream (or anywhere else).

But it doesn't look like we have to do that for now?

This comment did make me look into my usage of

const olmv1systemNamespace = olmv1-system

and now I switched to an existing helper function getComponentNamespace from the metrics_test.go file, so thank you for that :)

camilamacedo86 · 2025-06-10T21:17:26Z

test/e2e/network_policy_test.go

+func intOrStrPtr(port int32) *intstr.IntOrString {
+	val := intstr.FromInt(int(port))
+	return &val
+}


Suggested change

func intOrStrPtr(port int32) *intstr.IntOrString {

val := intstr.FromInt(int(port))

return &val

}

We do not need the helper.
We can use k8s.io/apimachinery/pkg/util/intstr (since we import this lib already)
and then something like Port: intstr.FromInt(NUMBER)

@camilamacedo86 I did try that first but fyi Port is of type *intstr.IntOrString but intstr.FromInt(Number) returns a intstr.IntOrString, and you can't really do a &intstr.FromInt(Number). Which is why I decided to store the intstr.IntOrString returned by FromInt in a variable, and then return the address of that variable from the helper function, so that that can be used for setting Port

Hi @anik120

We are using it here:

port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(catalogdMetricsPort)}},

We do not need to create a helper for this one. We can

&intstr.IntOrString{Type: intstr.Int, IntVal: PORT_NUMBER}

Like

port: []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: &intstr.IntOrString{Type: intstr.Int, IntVal: catalogdMetricsPort}}},

camilamacedo86 · 2025-06-10T21:21:15Z

test/e2e/network_policy_test.go

+			ports: []portWithJustification{
+				{
+					port:          []networkingv1.NetworkPolicyPort{{Protocol: ptr.To(corev1.ProtocolTCP), Port: intOrStrPtr(catalogdMetricsPort)}},
+					justification: "Allows Prometheus to scrape metrics from catalogd, which is essential for monitoring its performance and health.",


Is it really working?
@dtfranz did you not need to apply the NP to scrape the metrics?

I'm pretty sure it will work without the additional NP if prometheus lives in another namespace.

test/e2e/network_policy_test.go

anik120 · 2025-06-11T18:53:57Z

@joelanford @perdasilva regarding the discussion about possibly switching this to a unit test, I see the Quickstart target in the Makefile, and ack the discussion about using steps from that target (separated out) to generate the manifests and before starting the unit tests, and then consuming the generated manifests from the unit test.

I don't see a natural home for the unit test though. Maybe that's actually a good signal that this should be a e2e test instead.

Thoughts?

anik120 requested a review from a team as a code owner June 5, 2025 14:56

openshift-ci bot requested review from gallettilance and rashmigottipati June 5, 2025 14:56

anik120 force-pushed the np-tests branch from 98150b6 to 41840eb Compare June 5, 2025 15:35

joelanford reviewed Jun 5, 2025

View reviewed changes

anik120 force-pushed the np-tests branch from 41840eb to 6091ffa Compare June 5, 2025 18:38

anik120 force-pushed the np-tests branch from 6091ffa to e8d954d Compare June 5, 2025 19:49

anik120 force-pushed the np-tests branch 2 times, most recently from 5496cd9 to ec22c0a Compare June 6, 2025 15:48

dtfranz reviewed Jun 10, 2025

View reviewed changes

test/e2e/network_policy_test.go Outdated Show resolved Hide resolved

dtfranz reviewed Jun 10, 2025

View reviewed changes

perdasilva reviewed Jun 10, 2025

View reviewed changes

anik120 force-pushed the np-tests branch from ec22c0a to 7506ef0 Compare June 10, 2025 19:03

OPRUN-3873: Add e2e tests for NetworkPolicies

b61ee05

anik120 force-pushed the np-tests branch from 7506ef0 to b61ee05 Compare June 10, 2025 19:12

camilamacedo86 reviewed Jun 10, 2025

View reviewed changes

test/e2e/network_policy_test.go Show resolved Hide resolved

	Justification: "Permits Kubernetes API server to reach catalogd's admission webhook for CRD validation, ensuring integrity of catalog resources.",
	Justification: "Permits Kubernetes API server to reach catalogd's mutating admission webhook, ensuring integrity of catalog resources.",

	Justification: "Enables operator-controller to query catalog metadata from catalogd. This is a core function for bundle resolution and operator discovery.",
	Justification: "Enables clients (e.g. operator-controller) to query catalog metadata from catalogd. This is a core function for bundle resolution and operator discovery.",

	Justification: "Permits catalogd to fetch catalog images from various container registries and communicate with the Kubernetes API server for its operational needs.",
	Justification: "Permits catalogd to fetch catalog images from arbitrary container registries and communicate with the Kubernetes API server for its operational needs.",

✨OPRUN-3873: Add e2e tests for NetworkPolicies #2013

Are you sure you want to change the base?

✨OPRUN-3873: Add e2e tests for NetworkPolicies #2013

Uh oh!

Conversation

anik120 commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Checklist

Uh oh!

netlify bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

openshift-ci bot commented Jun 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anik120 commented Jun 5, 2025

Uh oh!

codecov bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anik120 Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anik120 commented Jun 11, 2025

Uh oh!

Uh oh!

anik120 commented Jun 5, 2025 •

edited

Loading

netlify bot commented Jun 5, 2025 •

edited

Loading

codecov bot commented Jun 5, 2025 •

edited

Loading

anik120 Jun 10, 2025 •

edited

Loading