From 5651a7961eb7d7b69088836a82abafb3e89fa1b8 Mon Sep 17 00:00:00 2001
From: Brandon Palm <bpalm@redhat.com>
Date: Thu, 2 Jan 2025 14:17:34 -0600
Subject: [PATCH] networkpolicy: adjust return consistency

---
 pkg/networkpolicy/multinetegressrule.go       |  4 ----
 pkg/networkpolicy/multinetegressrule_test.go  |  6 ++++++
 pkg/networkpolicy/multinetingressrule.go      | 10 +---------
 pkg/networkpolicy/multinetingressrule_test.go | 12 ------------
 pkg/networkpolicy/multinetworkpolicy.go       |  4 ----
 5 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/pkg/networkpolicy/multinetegressrule.go b/pkg/networkpolicy/multinetegressrule.go
index 1b7d51fab..727378820 100644
--- a/pkg/networkpolicy/multinetegressrule.go
+++ b/pkg/networkpolicy/multinetegressrule.go
@@ -126,10 +126,6 @@ func (builder *EgressRuleBuilder) WithOptions(options ...EgressAdditionalOptions
 func (builder *EgressRuleBuilder) WithPeerPodSelector(podSelector metav1.LabelSelector) *EgressRuleBuilder {
 	glog.V(100).Infof("Adding peer pod selector %v to EgressRule", podSelector)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.definition.To = append(builder.definition.To, v1beta1.MultiNetworkPolicyPeer{PodSelector: &podSelector})
 
 	return builder
diff --git a/pkg/networkpolicy/multinetegressrule_test.go b/pkg/networkpolicy/multinetegressrule_test.go
index ef3f90bd7..d89a1d94a 100644
--- a/pkg/networkpolicy/multinetegressrule_test.go
+++ b/pkg/networkpolicy/multinetegressrule_test.go
@@ -113,6 +113,8 @@ func TestEgressWithPeerPodSelector(t *testing.T) {
 	assert.Len(t, builder.definition.To, 1)
 	assert.Equal(t, builder.definition.To[0].PodSelector.MatchLabels["app"], "nginx")
 
+	builder = NewEgressRuleBuilder()
+
 	//nolint:goconst
 	builder.errorMsg = "error"
 
@@ -205,6 +207,8 @@ func TestEgressWithPeerPodSelectorAndCIDR(t *testing.T) {
 	assert.Equal(t, builder.definition.To[0].PodSelector.MatchLabels["app"], "nginx")
 	assert.Equal(t, builder.definition.To[0].IPBlock.CIDR, "192.168.0.1/24")
 
+	builder = NewEgressRuleBuilder()
+
 	// Test invalid CIDR
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
@@ -214,6 +218,8 @@ func TestEgressWithPeerPodSelectorAndCIDR(t *testing.T) {
 
 	assert.Equal(t, builder.errorMsg, "Invalid CIDR argument 192.55.55.55")
 
+	builder = NewEgressRuleBuilder()
+
 	// Test with exception
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
diff --git a/pkg/networkpolicy/multinetingressrule.go b/pkg/networkpolicy/multinetingressrule.go
index 630697364..684c42d83 100644
--- a/pkg/networkpolicy/multinetingressrule.go
+++ b/pkg/networkpolicy/multinetingressrule.go
@@ -125,10 +125,6 @@ func (builder *IngressRuleBuilder) WithOptions(options ...IngressAdditionalOptio
 func (builder *IngressRuleBuilder) WithPeerPodSelector(podSelector metav1.LabelSelector) *IngressRuleBuilder {
 	glog.V(100).Infof("Adding peer pod selector %v to Ingress Rule", podSelector)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.definition.From = append(
 		builder.definition.From, v1beta1.MultiNetworkPolicyPeer{
 			PodSelector: &podSelector,
@@ -203,10 +199,6 @@ func (builder *IngressRuleBuilder) WithPeerPodSelectorAndCIDR(
 	podSelector metav1.LabelSelector, cidr string, except ...[]string) *IngressRuleBuilder {
 	glog.V(100).Infof("Adding peer pod selector %v and CIDR %s to IngressRule", podSelector, cidr)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.WithPeerPodSelector(podSelector)
 	builder.WithCIDR(cidr, except...)
 
@@ -238,7 +230,7 @@ func (builder *IngressRuleBuilder) validate() (bool, error) {
 	if builder.definition == nil {
 		glog.V(100).Infof("The %s is undefined", objectName)
 
-		builder.errorMsg = msg.UndefinedCrdObjectErrString(objectName)
+		return false, fmt.Errorf(msg.UndefinedCrdObjectErrString(objectName))
 	}
 
 	if builder.errorMsg != "" {
diff --git a/pkg/networkpolicy/multinetingressrule_test.go b/pkg/networkpolicy/multinetingressrule_test.go
index d59f07af1..98419f05c 100644
--- a/pkg/networkpolicy/multinetingressrule_test.go
+++ b/pkg/networkpolicy/multinetingressrule_test.go
@@ -112,18 +112,6 @@ func TestIngressWithPeerPodSelector(t *testing.T) {
 
 	assert.Len(t, builder.definition.From, 1)
 	assert.Equal(t, builder.definition.From[0].PodSelector.MatchLabels["app"], "nginx")
-
-	builder = NewIngressRuleBuilder()
-
-	builder.errorMsg = "error"
-
-	builder.WithPeerPodSelector(metav1.LabelSelector{
-		MatchLabels: map[string]string{
-			"app": "nginx",
-		},
-	})
-
-	assert.Len(t, builder.definition.From, 0)
 }
 
 func TestIngressWithPeerNamespaceSelector(t *testing.T) {
diff --git a/pkg/networkpolicy/multinetworkpolicy.go b/pkg/networkpolicy/multinetworkpolicy.go
index 4bd673b26..6bb69c677 100644
--- a/pkg/networkpolicy/multinetworkpolicy.go
+++ b/pkg/networkpolicy/multinetworkpolicy.go
@@ -84,10 +84,6 @@ func (builder *MultiNetworkPolicyBuilder) WithPodSelector(podSelector metav1.Lab
 		"Creating MultiNetworkPolicy %s in %s namespace with the podSelector defined: %v",
 		builder.Definition.Name, builder.Definition.Namespace, podSelector)
 
-	if builder.errorMsg != "" {
-		return builder
-	}
-
 	builder.Definition.Spec.PodSelector = podSelector
 
 	return builder