From 3a57026da891aa672fc0357c696c8b603eb79545 Mon Sep 17 00:00:00 2001
From: Brandon Palm <bpalm@redhat.com>
Date: Fri, 3 Jan 2025 09:22:03 -0600
Subject: [PATCH] nmstate: adjust return consistency (#848)

---
 pkg/nmstate/nmstate.go          |  6 ++++--
 pkg/nmstate/nodenetworkstate.go |  8 ++++----
 pkg/nmstate/policy.go           | 20 ++++++++++----------
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/pkg/nmstate/nmstate.go b/pkg/nmstate/nmstate.go
index ee89277a6..26cbbd131 100644
--- a/pkg/nmstate/nmstate.go
+++ b/pkg/nmstate/nmstate.go
@@ -44,7 +44,7 @@ func NewBuilder(apiClient *clients.Settings, name string) *Builder {
 		return nil
 	}
 
-	builder := Builder{
+	builder := &Builder{
 		apiClient: apiClient.Client,
 		Definition: &nmstateV1.NMState{
 			ObjectMeta: metav1.ObjectMeta{
@@ -57,9 +57,11 @@ func NewBuilder(apiClient *clients.Settings, name string) *Builder {
 		glog.V(100).Infof("The name of the NMState is empty")
 
 		builder.errorMsg = "NMState 'name' cannot be empty"
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // Exists checks whether the given NMState exists.
diff --git a/pkg/nmstate/nodenetworkstate.go b/pkg/nmstate/nodenetworkstate.go
index ac9137542..3548d95ce 100644
--- a/pkg/nmstate/nodenetworkstate.go
+++ b/pkg/nmstate/nodenetworkstate.go
@@ -195,7 +195,7 @@ func PullNodeNetworkState(apiClient *clients.Settings, name string) (*StateBuild
 		return nil, err
 	}
 
-	stateBuilder := StateBuilder{
+	stateBuilder := &StateBuilder{
 		apiClient: apiClient.Client,
 		Object: &nmstateV1alpha1.NodeNetworkState{
 			ObjectMeta: metav1.ObjectMeta{
@@ -214,7 +214,7 @@ func PullNodeNetworkState(apiClient *clients.Settings, name string) (*StateBuild
 		return nil, fmt.Errorf("nodeNetworkState object %s does not exist", name)
 	}
 
-	return &stateBuilder, nil
+	return stateBuilder, nil
 }
 
 // validate will check that the builder and builder definition are properly initialized before
@@ -231,13 +231,13 @@ func (builder *StateBuilder) validate() (bool, error) {
 	if builder.Object == nil {
 		glog.V(100).Infof("The %s is undefined", resourceCRD)
 
-		builder.errorMsg = msg.UndefinedCrdObjectErrString(resourceCRD)
+		return false, fmt.Errorf(msg.UndefinedCrdObjectErrString(resourceCRD))
 	}
 
 	if builder.apiClient == nil {
 		glog.V(100).Infof("The %s builder apiclient is nil", resourceCRD)
 
-		builder.errorMsg = fmt.Sprintf("%s builder cannot have nil apiClient", resourceCRD)
+		return false, fmt.Errorf("%s builder cannot have nil apiClient", resourceCRD)
 	}
 
 	if builder.errorMsg != "" {
diff --git a/pkg/nmstate/policy.go b/pkg/nmstate/policy.go
index 9df3a4135..be6e505c9 100644
--- a/pkg/nmstate/policy.go
+++ b/pkg/nmstate/policy.go
@@ -63,7 +63,7 @@ func NewPolicyBuilder(apiClient *clients.Settings, name string, nodeSelector map
 		return nil
 	}
 
-	builder := PolicyBuilder{
+	builder := &PolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &nmstateV1.NodeNetworkConfigurationPolicy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -78,6 +78,8 @@ func NewPolicyBuilder(apiClient *clients.Settings, name string, nodeSelector map
 		glog.V(100).Infof("The name of the NodeNetworkConfigurationPolicy is empty")
 
 		builder.errorMsg = "nodeNetworkConfigurationPolicy 'name' cannot be empty"
+
+		return builder
 	}
 
 	if len(nodeSelector) == 0 {
@@ -86,7 +88,7 @@ func NewPolicyBuilder(apiClient *clients.Settings, name string, nodeSelector map
 		builder.errorMsg = "nodeNetworkConfigurationPolicy 'nodeSelector' cannot be empty map"
 	}
 
-	return &builder
+	return builder
 }
 
 // Get returns NodeNetworkConfigurationPolicy object if found.
@@ -260,15 +262,15 @@ func (builder *PolicyBuilder) WithBondInterface(slavePorts []string, bondName, m
 		glog.V(100).Infof("error to add Bond mode %s, allowed modes are %v", mode, allowedBondModes)
 
 		builder.errorMsg = "invalid Bond mode parameter"
+
+		return builder
 	}
 
 	if bondName == "" {
 		glog.V(100).Infof("The bondName can not be empty string")
 
 		builder.errorMsg = "The bondName is empty sting"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -298,13 +300,13 @@ func (builder *PolicyBuilder) WithVlanInterface(baseInterface string, vlanID uin
 		glog.V(100).Infof("The baseInterface can not be empty string")
 
 		builder.errorMsg = "nodenetworkconfigurationpolicy 'baseInterface' cannot be empty"
+
+		return builder
 	}
 
 	if vlanID > 4094 {
 		builder.errorMsg = "invalid vlanID, allowed vlanID values are between 0-4094"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -405,9 +407,7 @@ func (builder *PolicyBuilder) WithAbsentInterface(interfaceName string) *PolicyB
 		glog.V(100).Infof("The interfaceName can not be empty string")
 
 		builder.errorMsg = "nodenetworkconfigurationpolicy 'interfaceName' cannot be empty"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -493,13 +493,13 @@ func (builder *PolicyBuilder) validate() (bool, error) {
 	if builder.Definition == nil {
 		glog.V(100).Infof("The %s is undefined", resourceCRD)
 
-		builder.errorMsg = msg.UndefinedCrdObjectErrString(resourceCRD)
+		return false, fmt.Errorf(msg.UndefinedCrdObjectErrString(resourceCRD))
 	}
 
 	if builder.apiClient == nil {
 		glog.V(100).Infof("The %s builder apiclient is nil", resourceCRD)
 
-		builder.errorMsg = fmt.Sprintf("%s builder cannot have nil apiClient", resourceCRD)
+		return false, fmt.Errorf("%s builder cannot have nil apiClient", resourceCRD)
 	}
 
 	if builder.errorMsg != "" {