rbac: adjust return consistency

pkg/cgu/cgu_test.go

@@ -28,7 +28,6 @@ var (
 	}
 )
 
-//nolint:funlen
 func TestPullCgu(t *testing.T) {
 	generateCgu := func(name, namespace string) *v1alpha1.ClusterGroupUpgrade {
 		return &v1alpha1.ClusterGroupUpgrade{

pkg/rbac/clusterrole.go

@@ -38,7 +38,13 @@ func NewClusterRoleBuilder(apiClient *clients.Settings, name string, rule rbacv1
 			"name: %s, policy rule: %v",
 		name, rule)
 
-	builder := ClusterRoleBuilder{
+	if apiClient == nil {
+		glog.V(100).Infof("The apiClient is empty")
+
+		return nil
+	}
+
+	builder := &ClusterRoleBuilder{
 		apiClient: apiClient,
 		Definition: &rbacv1.ClusterRole{
 			ObjectMeta: metav1.ObjectMeta{
@@ -51,11 +57,13 @@ func NewClusterRoleBuilder(apiClient *clients.Settings, name string, rule rbacv1
 		glog.V(100).Infof("The name of the clusterrole is empty")
 
 		builder.errorMsg = "clusterrole 'name' cannot be empty"
+
+		return builder
 	}
 
 	builder.WithRules([]rbacv1.PolicyRule{rule})
 
-	return &builder
+	return builder
 }
 
 // WithRules appends additional rules to the clusterrole definition.
@@ -71,9 +79,7 @@ func (builder *ClusterRoleBuilder) WithRules(rules []rbacv1.PolicyRule) *Cluster
 		glog.V(100).Infof("The list of rules is empty")
 
 		builder.errorMsg = "cannot accept nil or empty slice as rules"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -82,9 +88,7 @@ func (builder *ClusterRoleBuilder) WithRules(rules []rbacv1.PolicyRule) *Cluster
 			glog.V(100).Infof("The clusterrole rule must contain at least one Verb entry")
 
 			builder.errorMsg = "clusterrole rule must contain at least one Verb entry"
-		}
 
-		if builder.errorMsg != "" {
 			return builder
 		}
 	}
@@ -135,7 +139,7 @@ func PullClusterRole(apiClient *clients.Settings, name string) (*ClusterRoleBuil
 		return nil, fmt.Errorf("the apiClient cannot be nil")
 	}
 
-	builder := ClusterRoleBuilder{
+	builder := &ClusterRoleBuilder{
 		apiClient: apiClient,
 		Definition: &rbacv1.ClusterRole{
 			ObjectMeta: metav1.ObjectMeta{
@@ -156,7 +160,7 @@ func PullClusterRole(apiClient *clients.Settings, name string) (*ClusterRoleBuil
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Create generates a clusterrole in the cluster and stores the created object in struct.
@@ -256,13 +260,13 @@ func (builder *ClusterRoleBuilder) 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 != "" {

pkg/rbac/clusterrole_test.go

@@ -60,10 +60,12 @@ func TestNewClusterRoleBuilder(t *testing.T) {
 		testClusterRole := NewClusterRoleBuilder(client, testCase.name, testCase.rule)
 		if testCase.client {
 			assert.NotNil(t, testClusterRole)
-		}
 
-		if len(testCase.expectedErrorText) > 0 {
-			assert.Equal(t, testCase.expectedErrorText, testClusterRole.errorMsg)
+			if len(testCase.expectedErrorText) > 0 {
+				assert.Equal(t, testCase.expectedErrorText, testClusterRole.errorMsg)
+			}
+		} else {
+			assert.Nil(t, testClusterRole)
 		}
 	}
 }

pkg/rbac/clusterrolebinding.go

@@ -37,7 +37,7 @@ func NewClusterRoleBindingBuilder(
 			"name: %s, clusterrole: %s, subject %v",
 		name, clusterRole, subject)
 
-	builder := ClusterRoleBindingBuilder{
+	builder := &ClusterRoleBindingBuilder{
 		apiClient: apiClient,
 		Definition: &rbacv1.ClusterRoleBinding{
 			ObjectMeta: metav1.ObjectMeta{
@@ -57,9 +57,11 @@ func NewClusterRoleBindingBuilder(
 		glog.V(100).Infof("The name of the clusterrolebinding is empty")
 
 		builder.errorMsg = "clusterrolebinding 'name' cannot be empty"
+
+		return builder
 	}
 
-	return &builder
+	return builder
 }
 
 // WithSubjects appends additional subjects to clusterrolebinding definition.
@@ -75,9 +77,7 @@ func (builder *ClusterRoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject
 		glog.V(100).Infof("The list of subjects is empty")
 
 		builder.errorMsg = "cannot accept nil or empty slice as subjects"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -86,15 +86,15 @@ func (builder *ClusterRoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject
 			glog.V(100).Infof("The clusterrolebinding subject kind must be one of 'ServiceAccount', 'User', or 'Group'")
 
 			builder.errorMsg = "clusterrolebinding subject kind must be one of 'ServiceAccount', 'User', or 'Group'"
+
+			return builder
 		}
 
 		if subject.Name == "" {
 			glog.V(100).Infof("The clusterrolebinding subject name cannot be empty")
 
 			builder.errorMsg = "clusterrolebinding subject name cannot be empty"
-		}
 
-		if builder.errorMsg != "" {
 			return builder
 		}
 	}
@@ -146,7 +146,7 @@ func PullClusterRoleBinding(apiClient *clients.Settings, name string) (*ClusterR
 	if name == "" {
 		glog.V(100).Infof("The name of the clusterrolebinding is empty")
 
-		builder.errorMsg = "clusterrolebinding 'name' cannot be empty"
+		return nil, fmt.Errorf("clusterrolebinding 'name' cannot be empty")
 	}
 
 	if !builder.Exists() {
@@ -203,7 +203,7 @@ func (builder *ClusterRoleBindingBuilder) Delete() error {
 
 	builder.Object = nil
 
-	return err
+	return nil
 }
 
 // Update modifies a clusterrolebinding object in the cluster.
@@ -252,13 +252,13 @@ func (builder *ClusterRoleBindingBuilder) 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 != "" {

pkg/rbac/role.go

@@ -34,6 +34,12 @@ func NewRoleBuilder(apiClient *clients.Settings, name, nsname string, rule rbacv
 		"Initializing new role structure with the following params: "+
 			"name: %s, namespace: %s, rule %v", name, nsname, rule)
 
+	if apiClient == nil {
+		glog.V(100).Infof("The apiClient is empty")
+
+		return nil
+	}
+
 	builder := &RoleBuilder{
 		apiClient: apiClient,
 		Definition: &rbacv1.Role{
@@ -78,9 +84,7 @@ func (builder *RoleBuilder) WithRules(rules []rbacv1.PolicyRule) *RoleBuilder {
 		glog.V(100).Infof("The list of rules is empty")
 
 		builder.errorMsg = "cannot create role with empty rule"
-	}
 
-	if builder.errorMsg != "" {
 		return builder
 	}
 
@@ -89,21 +93,23 @@ func (builder *RoleBuilder) WithRules(rules []rbacv1.PolicyRule) *RoleBuilder {
 			glog.V(100).Infof("The role has no verbs")
 
 			builder.errorMsg = "role must contain at least one Verb"
+
+			return builder
 		}
 
 		if len(rule.Resources) == 0 {
 			glog.V(100).Infof("The role has no resources")
 
 			builder.errorMsg = "role must contain at least one Resource"
+
+			return builder
 		}
 
 		if len(rule.APIGroups) == 0 {
 			glog.V(100).Infof("The role has no apigroups")
 
 			builder.errorMsg = "role must contain at least one APIGroup"
-		}
 
-		if builder.errorMsg != "" {
 			return builder
 		}
 	}
@@ -153,7 +159,7 @@ func PullRole(apiClient *clients.Settings, name, nsname string) (*RoleBuilder, e
 		return nil, fmt.Errorf("the apiClient cannot be nil")
 	}
 
-	builder := RoleBuilder{
+	builder := &RoleBuilder{
 		apiClient: apiClient,
 		Definition: &rbacv1.Role{
 			ObjectMeta: metav1.ObjectMeta{
@@ -181,7 +187,7 @@ func PullRole(apiClient *clients.Settings, name, nsname string) (*RoleBuilder, e
 
 	builder.Definition = builder.Object
 
-	return &builder, nil
+	return builder, nil
 }
 
 // Create makes a Role in the cluster and stores the created object in struct.
@@ -229,7 +235,7 @@ func (builder *RoleBuilder) Delete() error {
 
 	builder.Object = nil
 
-	return err
+	return nil
 }
 
 // Update modifies the existing Role object with role definition in builder.
@@ -282,13 +288,13 @@ func (builder *RoleBuilder) 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 != "" {

pkg/rbac/role_test.go

@@ -78,10 +78,12 @@ func TestNewRoleBuilder(t *testing.T) {
 		testPolicy := NewRoleBuilder(client, testCase.name, testCase.nsName, testCase.rule)
 		if testCase.client {
 			assert.NotNil(t, testPolicy)
-		}
 
-		if len(testCase.expectedErrorText) > 0 {
-			assert.Equal(t, testCase.expectedErrorText, testPolicy.errorMsg)
+			if len(testCase.expectedErrorText) > 0 {
+				assert.Equal(t, testCase.expectedErrorText, testPolicy.errorMsg)
+			}
+		} else {
+			assert.Nil(t, testPolicy)
 		}
 	}
 }
@@ -190,7 +192,7 @@ func TestRoleCreate(t *testing.T) {
 		},
 		{
 			testRole:      buildInvalidRoleTestBuilder(buildTestClientWithDummyObject()),
-			expectedError: fmt.Errorf("role must contain at least one APIGroup"),
+			expectedError: fmt.Errorf("role must contain at least one Verb"),
 		},
 		{
 			testRole:      buildValidRoleBuilder(clients.GetTestClients(clients.TestClientParams{})),
@@ -244,7 +246,7 @@ func TestRoleDelete(t *testing.T) {
 		},
 		{
 			testRole:      buildInvalidRoleTestBuilder(buildTestClientWithDummyObject()),
-			expectedError: fmt.Errorf("role must contain at least one APIGroup"),
+			expectedError: fmt.Errorf("role must contain at least one Verb"),
 		},
 		{
 			testRole:      buildValidRoleBuilder(clients.GetTestClients(clients.TestClientParams{})),
@@ -277,7 +279,7 @@ func TestRoleUpdate(t *testing.T) {
 		},
 		{
 			testRole:      buildInvalidRoleTestBuilder(buildTestClientWithDummyObject()),
-			expectedError: fmt.Errorf("role must contain at least one APIGroup"),
+			expectedError: fmt.Errorf("role must contain at least one Verb"),
 		},
 		{
 			testRole:      buildValidRoleBuilder(clients.GetTestClients(clients.TestClientParams{})),