diagnostics: AggregatedLogging ClusterRoleBindings false positive fix

The clusterRoleBindings check can output false alarm even if `cluster-reader`
role is assigned to fluentd service account and logging works as expected.

The check used to query for `cluster-readers` CRB, but the common command
`oc adm policy add-cluster-role-to-user cluster-reader system:serviceaccount:logging:aggregated-logging-fluentd`
no longer appers to add the SA into `cluster-readers` group but instead
creates cluster-reader-1 CRB.

```
$ oc get clusterrolebindings cluster-readers -o yaml
...
roleRef:
  name: cluster-reader
...
userNames: null

$ oc get clusterrolebindings
NAME              ROLE            USERS  GROUPS                  SERVICE ACCOUNTS                   SUBJECTS
...
cluster-reader    /cluster-reader                                management-infra/management-admin
cluster-reader-0  /cluster-reader                                default/router
cluster-reader-1  /cluster-reader                                logging/aggregated-logging-fluentd
cluster-readers   /cluster-reader        system:cluster-readers
...
```

This fix queries all clusterrolebindings, iterates over those, that have
role `cluster-reader` and then validates there is a `cluster-reader` entry
for `system:serviceaccount:logging:aggregated-logging-fluentd`

Author: Jan Wozniak

pkg/oc/admin/diagnostics/diagnostics/cluster/aggregated_logging/clusterrolebindings.go

@@ -7,7 +7,7 @@ import (
 	"k8s.io/kubernetes/pkg/apis/rbac"
 )
 
-const clusterReaderRoleBindingName = "cluster-readers"
+const clusterReaderRoleBindingRoleName = "cluster-reader"
 
 var clusterReaderRoleBindingNames = sets.NewString(fluentdServiceAccountName)
 
@@ -22,15 +22,20 @@ the following:
 
 func checkClusterRoleBindings(r diagnosticReporter, adapter clusterRoleBindingsAdapter, project string) {
 	r.Debug("AGL0600", "Checking ClusterRoleBindings...")
-	crb, err := adapter.getClusterRoleBinding(clusterReaderRoleBindingName)
+	crbs, err := adapter.listClusterRoleBindings()
 	if err != nil {
 		r.Error("AGL0605", err, fmt.Sprintf("There was an error while trying to retrieve the ClusterRoleBindings for the logging stack: %s", err))
 		return
 	}
 	boundServiceAccounts := sets.NewString()
-	for _, subject := range crb.Subjects {
-		if subject.Kind == rbac.ServiceAccountKind && subject.Namespace == project {
-			boundServiceAccounts.Insert(subject.Name)
+	for _, crb := range crbs.Items {
+		if crb.RoleRef.Name != clusterReaderRoleBindingRoleName {
+			continue
+		}
+		for _, subject := range crb.Subjects {
+			if subject.Kind == rbac.ServiceAccountKind && subject.Namespace == project {
+				boundServiceAccounts.Insert(subject.Name)
+			}
 		}
 	}
 	for _, name := range clusterReaderRoleBindingNames.List() {

pkg/oc/admin/diagnostics/diagnostics/cluster/aggregated_logging/clusterrolebindings_test.go

@@ -13,28 +13,36 @@ import (
 
 type fakeRoleBindingDiagnostic struct {
 	fakeDiagnostic
-	fakeClusterRoleBinding authapi.ClusterRoleBinding
+	fakeClusterRoleBindingList authapi.ClusterRoleBindingList
 }
 
 func newFakeRoleBindingDiagnostic(t *testing.T) *fakeRoleBindingDiagnostic {
 	return &fakeRoleBindingDiagnostic{
-		fakeDiagnostic: *newFakeDiagnostic(t),
+		fakeDiagnostic:             *newFakeDiagnostic(t),
+		fakeClusterRoleBindingList: authapi.ClusterRoleBindingList{},
 	}
 }
 
-func (f *fakeRoleBindingDiagnostic) getClusterRoleBinding(name string) (*authapi.ClusterRoleBinding, error) {
+func (f *fakeRoleBindingDiagnostic) listClusterRoleBindings() (*authapi.ClusterRoleBindingList, error) {
 	if f.err != nil {
 		return nil, f.err
 	}
-	return &f.fakeClusterRoleBinding, nil
+	return &f.fakeClusterRoleBindingList, nil
 }
 func (f *fakeRoleBindingDiagnostic) addBinding(name string, namespace string) {
 	ref := kapi.ObjectReference{
 		Name:      name,
 		Kind:      rbac.ServiceAccountKind,
 		Namespace: namespace,
 	}
-	f.fakeClusterRoleBinding.Subjects = append(f.fakeClusterRoleBinding.Subjects, ref)
+	if len(f.fakeClusterRoleBindingList.Items) == 0 {
+		f.fakeClusterRoleBindingList.Items = append(f.fakeClusterRoleBindingList.Items, authapi.ClusterRoleBinding{
+			RoleRef: kapi.ObjectReference{
+				Name: clusterReaderRoleBindingRoleName,
+			},
+		})
+	}
+	f.fakeClusterRoleBindingList.Items[0].Subjects = append(f.fakeClusterRoleBindingList.Items[0].Subjects, ref)
 }
 
 // Test error when client error

pkg/oc/admin/diagnostics/diagnostics/cluster/aggregated_logging/diagnostic.go

@@ -86,8 +86,8 @@ func (d *AggregatedLogging) getScc(name string) (*securityapi.SecurityContextCon
 	return d.SCCClient.SecurityContextConstraints().Get(name, metav1.GetOptions{})
 }
 
-func (d *AggregatedLogging) getClusterRoleBinding(name string) (*authapi.ClusterRoleBinding, error) {
-	return d.CRBClient.ClusterRoleBindings().Get(name, metav1.GetOptions{})
+func (d *AggregatedLogging) listClusterRoleBindings() (*authapi.ClusterRoleBindingList, error) {
+	return d.CRBClient.ClusterRoleBindings().List(metav1.ListOptions{})
 }
 
 func (d *AggregatedLogging) routes(project string, options metav1.ListOptions) (*routesapi.RouteList, error) {

pkg/oc/admin/diagnostics/diagnostics/cluster/aggregated_logging/interfaces.go

@@ -28,7 +28,7 @@ type sccAdapter interface {
 }
 
 type clusterRoleBindingsAdapter interface {
-	getClusterRoleBinding(name string) (*authapi.ClusterRoleBinding, error)
+	listClusterRoleBindings() (*authapi.ClusterRoleBindingList, error)
 }
 
 //deploymentConfigAdapter is an abstraction to retrieve resource for validating dcs