Address return consistency

pkg/assisted/agent.go

@@ -300,7 +300,7 @@ func (builder *agentBuilder) Update() (*agentBuilder, error) {
 		glog.V(100).Infof("agent %s in namespace %s does not exist",
 			builder.Definition.Name, builder.Definition.Namespace)
 
-		return nil, fmt.Errorf(nonExistentMsg)
+		return builder, fmt.Errorf(nonExistentMsg)
 	}
 
 	err := builder.apiClient.Update(context.TODO(), builder.Definition)

pkg/assisted/agentserviceconfig.go

@@ -261,7 +261,7 @@ func (builder *AgentServiceConfigBuilder) WaitUntilDeployed(timeout time.Duratio
 	if !builder.Exists() {
 		glog.V(100).Infof("The agentserviceconfig does not exist on the cluster")
 
-		return builder, fmt.Errorf("cannot wait for non-existent agentserviceconfig to be deployed")
+		return nil, fmt.Errorf("cannot wait for non-existent agentserviceconfig to be deployed")
 	}
 
 	// Polls every retryInterval to determine if agentserviceconfig is in desired state.
@@ -308,7 +308,7 @@ func PullAgentServiceConfig(apiClient *clients.Settings) (*AgentServiceConfigBui
 		return nil, fmt.Errorf("the apiClient is nil")
 	}
 
-	builder := AgentServiceConfigBuilder{
+	builder := &AgentServiceConfigBuilder{
 		apiClient: apiClient.Client,
 		Definition: &agentInstallV1Beta1.AgentServiceConfig{
 			ObjectMeta: metav1.ObjectMeta{
@@ -323,7 +323,7 @@ func PullAgentServiceConfig(apiClient *clients.Settings) (*AgentServiceConfigBui
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get fetches the defined agentserviceconfig from the cluster.
@@ -381,7 +381,7 @@ func (builder *AgentServiceConfigBuilder) Update(force bool) (*AgentServiceConfi
 		glog.V(100).Infof("agentserviceconfig %s does not exist",
 			builder.Definition.Name)
 
-		return builder, fmt.Errorf("cannot update non-existent agentserviceconfig")
+		return nil, fmt.Errorf("cannot update non-existent agentserviceconfig")
 	}
 
 	err := builder.apiClient.Update(context.TODO(), builder.Definition)

pkg/console/console.go

@@ -63,7 +63,7 @@ func Pull(apiClient *clients.Settings, name string) (*Builder, error) {
 	if name == "" {
 		glog.V(100).Info("The name of the Console is empty")
 
-		return builder, fmt.Errorf("console 'name' cannot be empty")
+		return nil, fmt.Errorf("console 'name' cannot be empty")
 	}
 
 	glog.V(100).Infof("Pulling cluster console %s", name)

pkg/hive/clusterdeployment.go

@@ -30,6 +30,8 @@
 
 // NewABMClusterDeploymentBuilder creates a new instance of
 // ClusterDeploymentBuilder with platform type set to agentBareMetal.
+//
+//nolint:funlen
 func NewABMClusterDeploymentBuilder(
 	apiClient *clients.Settings,
 	name string,
@@ -135,7 +137,7 @@
 	if clusterInstallRef.Name == "" {
 		glog.V(100).Infof("The clusterInstallRef name of the clusterdeployment is empty")
 
-		builder.errorMsg = "clusterdeployment 'clusterInstallRef.name' cannot be empty"
+		builder.errorMsg = "clusterdeployment 'clusterInstallRef' cannot be empty"
 
 		return builder
 	}

pkg/icsp/icsp.go

@@ -68,23 +68,23 @@ func NewICSPBuilder(apiClient *clients.Settings, name, source string, mirrors []
 	if name == "" {
 		glog.V(100).Infof("The name of the ImageContentSourcePolicy is empty")
 
-		icspBuilder.errorMsg = "imageContentSourcePolicy 'name' cannot be empty"
+		icspBuilder.errorMsg = "ImageContentSourcePolicy 'name' cannot be empty"
 
 		return icspBuilder
 	}
 
 	if source == "" {
 		glog.V(100).Infof("The Source of the ImageContentSourcePolicy is empty")
 
-		icspBuilder.errorMsg = "imageContentSourcePolicy 'source' cannot be empty"
+		icspBuilder.errorMsg = "ImageContentSourcePolicy 'source' cannot be empty"
 
 		return icspBuilder
 	}
 
 	if len(mirrors) == 0 {
 		glog.V(100).Infof("The mirrors of the ImageContentSourcePolicy are empty")
 
-		icspBuilder.errorMsg = "imageContentSourcePolicy 'mirrors' cannot be empty"
+		icspBuilder.errorMsg = "ImageContentSourcePolicy 'mirrors' cannot be empty"
 
 		return icspBuilder
 	}
@@ -254,15 +254,15 @@ func (builder *ICSPBuilder) WithRepositoryDigestMirror(source string, mirrors []
 	if source == "" {
 		glog.V(100).Infof("The source is empty")
 
-		builder.errorMsg = "imageContentSourcePolicy 'source' cannot be empty"
+		builder.errorMsg = "'source' cannot be empty"
 
 		return builder
 	}
 
 	if len(mirrors) == 0 {
 		glog.V(100).Infof("Mirrors is empty")
 
-		builder.errorMsg = "imageContentSourcePolicy 'mirrors' cannot be empty"
+		builder.errorMsg = "'mirrors' cannot be empty"
 
 		return builder
 	}

pkg/lso/localvolumeset.go

@@ -95,7 +95,7 @@ func PullLocalVolumeSet(apiClient *clients.Settings, name, nsname string) (*Loca
 		return nil, err
 	}
 
-	builder := LocalVolumeSetBuilder{
+	builder := &LocalVolumeSetBuilder{
 		apiClient: apiClient.Client,
 		Definition: &lsov1alpha1.LocalVolumeSet{
 			ObjectMeta: metav1.ObjectMeta{
@@ -123,7 +123,7 @@ func PullLocalVolumeSet(apiClient *clients.Settings, name, nsname string) (*Loca
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get fetches existing localVolumeSet from cluster.

pkg/metallb/bgpadvertisement.go

@@ -254,7 +254,7 @@ func (builder *BGPAdvertisementBuilder) Update(force bool) (*BGPAdvertisementBui
 		}
 	}
 
-	return builder, err
+	return builder, nil
 }
 
 // WithAggregationLength4 adds the specified AggregationLength to the BGPAdvertisement.

pkg/metallb/bgppeer.go

@@ -156,13 +156,13 @@ func PullBGPPeer(apiClient *clients.Settings, name, nsname string) (*BGPPeerBuil
 	if name == "" {
 		glog.V(100).Infof("The name of the bgppeer is empty")
 
-		return nil, fmt.Errorf("bgppeer 'name' cannot be empty")
+		return nil, fmt.Errorf("bgppeer object name cannot be empty")
 	}
 
 	if nsname == "" {
 		glog.V(100).Infof("The namespace of the bgppeer is empty")
 
-		return nil, fmt.Errorf("bgppeer 'namespace' cannot be empty")
+		return nil, fmt.Errorf("bgppeer object namespace cannot be empty")
 	}
 
 	if !builder.Exists() {

pkg/metallb/l2advertisement.go

@@ -257,7 +257,7 @@ func (builder *L2AdvertisementBuilder) Update(force bool) (*L2AdvertisementBuild
 		}
 	}
 
-	return builder, err
+	return builder, nil
 }
 
 // WithNodeSelector adds the specified NodeSelectors to the L2Advertisement.

pkg/nad/builder.go

@@ -276,7 +276,7 @@ func (builder *Builder) GetString() (string, error) {
 		return "", err
 	}
 
-	return string(nadByte), err
+	return string(nadByte), nil
 }
 
 // fillConfigureString adds a configuration string to builder definition specs configuration if needed.

pkg/networkpolicy/multinetegressrule_test.go

@@ -114,7 +114,6 @@
 	assert.Equal(t, builder.definition.To[0].PodSelector.MatchLabels["app"], "nginx")
 
 	builder = NewEgressRuleBuilder()
-
 	//nolint:goconst
 	builder.errorMsg = "error"
 
@@ -207,9 +206,10 @@
 	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 = NewEgressRuleBuilder()
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": "nginx",
@@ -218,9 +218,10 @@
 
 	assert.Equal(t, builder.errorMsg, "Invalid CIDR argument 192.55.55.55")
 
 	builder = NewEgressRuleBuilder()
 
 	// Test with exception
+	builder = NewEgressRuleBuilder()
 	builder.WithPeerPodSelectorAndCIDR(metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			"app": "nginx",

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) {

pkg/networkpolicy/multinetworkpolicy.go

@@ -204,7 +204,7 @@ func PullMultiNetworkPolicy(apiClient *clients.Settings, name, nsname string) (*
 		return nil, err
 	}
 
-	builder := MultiNetworkPolicyBuilder{
+	builder := &MultiNetworkPolicyBuilder{
 		apiClient: apiClient.Client,
 		Definition: &v1beta1.MultiNetworkPolicy{
 			ObjectMeta: metav1.ObjectMeta{
@@ -236,7 +236,7 @@ func PullMultiNetworkPolicy(apiClient *clients.Settings, name, nsname string) (*
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns MultiNetworkPolicy object if found.

pkg/nfd/nodefeaturediscovery.go

@@ -46,6 +46,9 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string)
 		return nil
 	}
 
+	glog.V(100).Infof(
+		"Initializing Builder definition to NodeFeatureDiscovery object")
+
 	builder := &Builder{
 		apiClient: apiClient,
 	}
@@ -69,7 +72,8 @@ func NewBuilderFromObjectString(apiClient *clients.Settings, almExample string)
 	if builder.Definition == nil {
 		glog.V(100).Infof("The NodeFeatureDiscovery object definition is nil")
 
-		builder.errorMsg = "nodeFeatureDiscovery definition is nil"
+		//nolint:lll
+		builder.errorMsg = "Error initializing NodeFeatureDiscovery from alm-examples: invalid character 'i' looking for beginning of object key string"
 
 		return builder
 	}
@@ -249,7 +253,7 @@ func (builder *Builder) Update(force bool) (*Builder, error) {
 		}
 	}
 
-	return builder, err
+	return builder, nil
 }
 
 // getNodeFeatureDiscoveryFromAlmExample extracts the NodeFeatureDiscovery from the alm-examples block.

pkg/ocm/placementrule.go

@@ -91,7 +91,7 @@ func PullPlacementRule(apiClient *clients.Settings, name, nsname string) (*Place
 		return nil, err
 	}
 
-	builder := PlacementRuleBuilder{
+	builder := &PlacementRuleBuilder{
 		apiClient: apiClient.Client,
 		Definition: &placementrulev1.PlacementRule{
 			ObjectMeta: metav1.ObjectMeta{
@@ -119,7 +119,7 @@ func PullPlacementRule(apiClient *clients.Settings, name, nsname string) (*Place
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Exists checks whether the given placementrule exists.

pkg/olm/clusterserviceversion.go

@@ -45,7 +45,7 @@ func PullClusterServiceVersion(apiClient *clients.Settings, name, namespace stri
 		return nil, err
 	}
 
-	builder := ClusterServiceVersionBuilder{
+	builder := &ClusterServiceVersionBuilder{
 		apiClient: apiClient,
 		Definition: &oplmV1alpha1.ClusterServiceVersion{
 			ObjectMeta: metav1.ObjectMeta{
@@ -73,7 +73,7 @@ func PullClusterServiceVersion(apiClient *clients.Settings, name, namespace stri
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Get returns ClusterServiceVersion object if found.

pkg/pod/pod.go

@@ -1068,6 +1068,10 @@ func (builder *Builder) WithSecurityContext(securityContext *corev1.PodSecurityC
 
 	builder.isMutationAllowed("SecurityContext")
 
+	if builder.errorMsg != "" {
+		return builder
+	}
+
 	builder.Definition.Spec.SecurityContext = securityContext
 
 	return builder

pkg/rbac/clusterrole_test.go

@@ -31,7 +31,7 @@ func TestNewClusterRoleBuilder(t *testing.T) {
 			rule: rbacv1.PolicyRule{
 				Resources: []string{"pods"}, APIGroups: []string{"v1"}, Verbs: []string{"get"}},
 			client:            false,
-			expectedErrorText: "clusterRole builder cannot have nil apiClient",
+			expectedErrorText: "",
 		},
 		{
 			name: "",

pkg/rbac/clusterrolebinding.go

@@ -134,7 +134,7 @@ func (builder *ClusterRoleBindingBuilder) WithOptions(
 func PullClusterRoleBinding(apiClient *clients.Settings, name string) (*ClusterRoleBindingBuilder, error) {
 	glog.V(100).Infof("Pulling existing clusterrolebinding name %s from cluster", name)
 
-	builder := ClusterRoleBindingBuilder{
+	builder := &ClusterRoleBindingBuilder{
 		apiClient: apiClient,
 		Definition: &rbacv1.ClusterRoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
@@ -155,7 +155,7 @@ func PullClusterRoleBinding(apiClient *clients.Settings, name string) (*ClusterR
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Create generates a clusterrolebinding in the cluster and stores the created object in struct.

pkg/rbac/role_test.go

@@ -39,7 +39,7 @@ func TestNewRoleBuilder(t *testing.T) {
 			rule: rbacv1.PolicyRule{
 				Resources: []string{"pods"}, APIGroups: []string{"v1"}, Verbs: []string{"get"}},
 			client:            false,
-			expectedErrorText: "role builder cannot have nil apiClient",
+			expectedErrorText: "",
 		},
 		{
 			name:   "",

pkg/rbac/rolebinding.go

@@ -212,9 +212,13 @@ func (builder *RoleBindingBuilder) Delete() error {
 	err := builder.apiClient.RoleBindings(builder.Definition.Namespace).Delete(
 		context.TODO(), builder.Definition.Name, metav1.DeleteOptions{})
 
+	if err != nil {
+		return err
+	}
+
 	builder.Object = nil
 
-	return err
+	return nil
 }
 
 // Update modifies an existing RoleBinding in the cluster.

pkg/route/route.go

@@ -155,9 +155,7 @@ func (builder *Builder) WithTargetPortName(portName string) *Builder {
 		glog.V(100).Infof("Received empty route portName")
 
 		builder.errorMsg = "route target port name cannot be empty string"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 

pkg/scc/scc.go

@@ -570,6 +570,10 @@ func (builder *Builder) Delete() error {
 		return err
 	}
 
+	if err != nil {
+		return err
+	}
+
 	builder.Object = nil
 
 	return nil