diff --git a/pkg/cgu/cgu_test.go b/pkg/cgu/cgu_test.go index c33160f87..c9c648529 100644 --- a/pkg/cgu/cgu_test.go +++ b/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{ diff --git a/pkg/rbac/clusterrole.go b/pkg/rbac/clusterrole.go index f2fa4e905..5c17bc833 100644 --- a/pkg/rbac/clusterrole.go +++ b/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 != "" { diff --git a/pkg/rbac/clusterrole_test.go b/pkg/rbac/clusterrole_test.go index c16bca117..1c54d851a 100644 --- a/pkg/rbac/clusterrole_test.go +++ b/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) } } } diff --git a/pkg/rbac/clusterrolebinding.go b/pkg/rbac/clusterrolebinding.go index ac7007cfc..183357912 100644 --- a/pkg/rbac/clusterrolebinding.go +++ b/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 != "" { diff --git a/pkg/rbac/role.go b/pkg/rbac/role.go index 9648051e8..e17498b3b 100644 --- a/pkg/rbac/role.go +++ b/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 != "" { diff --git a/pkg/rbac/role_test.go b/pkg/rbac/role_test.go index ce1f8d99e..99e0becb4 100644 --- a/pkg/rbac/role_test.go +++ b/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{})), diff --git a/pkg/rbac/rolebinding.go b/pkg/rbac/rolebinding.go index e977ba23b..46cd2f1c3 100644 --- a/pkg/rbac/rolebinding.go +++ b/pkg/rbac/rolebinding.go @@ -38,7 +38,7 @@ func NewRoleBindingBuilder(apiClient *clients.Settings, "Initializing new rolebinding structure with the following params: "+ "name: %s, namespace: %s, role: %s, subject %v", name, nsname, role, subject) - builder := RoleBindingBuilder{ + builder := &RoleBindingBuilder{ apiClient: apiClient, Definition: &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -57,17 +57,21 @@ func NewRoleBindingBuilder(apiClient *clients.Settings, glog.V(100).Infof("The name of the rolebinding is empty") builder.errorMsg = "RoleBinding 'name' cannot be empty" + + return builder } if nsname == "" { glog.V(100).Infof("The namespace of the rolebinding is empty") builder.errorMsg = "RoleBinding 'nsname' cannot be empty" + + return builder } builder.WithSubjects([]rbacv1.Subject{subject}) - return &builder + return builder } // WithSubjects adds specified Subject to the RoleBinding. @@ -83,9 +87,7 @@ func (builder *RoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject) *Role glog.V(100).Infof("The list of subjects is empty") builder.errorMsg = "cannot create rolebinding with empty subject" - } - if builder.errorMsg != "" { return builder } @@ -94,15 +96,15 @@ func (builder *RoleBindingBuilder) WithSubjects(subjects []rbacv1.Subject) *Role glog.V(100).Infof("The rolebinding subject kind must be one of 'ServiceAccount', 'User', or 'Group'") builder.errorMsg = "rolebinding subject kind must be one of 'ServiceAccount', 'User', 'Group'" + + return builder } if subject.Name == "" { glog.V(100).Infof("The rolebinding subject name cannot be empty") builder.errorMsg = "rolebinding subject name cannot be empty" - } - if builder.errorMsg != "" { return builder } } @@ -140,7 +142,7 @@ func (builder *RoleBindingBuilder) WithOptions(options ...RoleBindingAdditionalO func PullRoleBinding(apiClient *clients.Settings, name, nsname string) (*RoleBindingBuilder, error) { glog.V(100).Infof("Pulling existing rolebinding name %s under namespace %s from cluster", name, nsname) - builder := RoleBindingBuilder{ + builder := &RoleBindingBuilder{ apiClient: apiClient, Definition: &rbacv1.RoleBinding{ ObjectMeta: metav1.ObjectMeta{ @@ -153,13 +155,13 @@ func PullRoleBinding(apiClient *clients.Settings, name, nsname string) (*RoleBin if name == "" { glog.V(100).Infof("The name of the rolebinding is empty") - builder.errorMsg = "rolebinding 'name' cannot be empty" + return builder, fmt.Errorf("rolebinding 'name' cannot be empty") } if nsname == "" { glog.V(100).Infof("The namespace of the rolebinding is empty") - builder.errorMsg = "rolebinding 'namespace' cannot be empty" + return builder, fmt.Errorf("rolebinding 'namespace' cannot be empty") } if !builder.Exists() { @@ -168,7 +170,7 @@ func PullRoleBinding(apiClient *clients.Settings, name, nsname string) (*RoleBin builder.Definition = builder.Object - return &builder, nil + return builder, nil } // Create generates a RoleBinding and stores the created object in struct. @@ -261,13 +263,13 @@ func (builder *RoleBindingBuilder) 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 != "" {