Skip to content

Commit 60c4264

Browse files
Merge pull request #18888 from openshift-cherrypick-robot/cherry-pick-18641-to-release-3.9
Automatic merge from submit-queue. [release-3.9] diagnostics: AggregatedLogging ClusterRoleBindings false negative fix This is an automated cherry-pick of #18641 /assign sosiouxme
2 parents 1c027c1 + 59a7133 commit 60c4264

File tree

4 files changed

+26
-13
lines changed

4 files changed

+26
-13
lines changed

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"k8s.io/kubernetes/pkg/apis/rbac"
88
)
99

10-
const clusterReaderRoleBindingName = "cluster-readers"
10+
const clusterReaderRoleBindingRoleName = "cluster-reader"
1111

1212
var clusterReaderRoleBindingNames = sets.NewString(fluentdServiceAccountName)
1313

@@ -22,15 +22,20 @@ the following:
2222

2323
func checkClusterRoleBindings(r diagnosticReporter, adapter clusterRoleBindingsAdapter, project string) {
2424
r.Debug("AGL0600", "Checking ClusterRoleBindings...")
25-
crb, err := adapter.getClusterRoleBinding(clusterReaderRoleBindingName)
25+
crbs, err := adapter.listClusterRoleBindings()
2626
if err != nil {
2727
r.Error("AGL0605", err, fmt.Sprintf("There was an error while trying to retrieve the ClusterRoleBindings for the logging stack: %s", err))
2828
return
2929
}
3030
boundServiceAccounts := sets.NewString()
31-
for _, subject := range crb.Subjects {
32-
if subject.Kind == rbac.ServiceAccountKind && subject.Namespace == project {
33-
boundServiceAccounts.Insert(subject.Name)
31+
for _, crb := range crbs.Items {
32+
if crb.RoleRef.Name != clusterReaderRoleBindingRoleName {
33+
continue
34+
}
35+
for _, subject := range crb.Subjects {
36+
if subject.Kind == rbac.ServiceAccountKind && subject.Namespace == project {
37+
boundServiceAccounts.Insert(subject.Name)
38+
}
3439
}
3540
}
3641
for _, name := range clusterReaderRoleBindingNames.List() {

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,36 @@ import (
1313

1414
type fakeRoleBindingDiagnostic struct {
1515
fakeDiagnostic
16-
fakeClusterRoleBinding authapi.ClusterRoleBinding
16+
fakeClusterRoleBindingList authapi.ClusterRoleBindingList
1717
}
1818

1919
func newFakeRoleBindingDiagnostic(t *testing.T) *fakeRoleBindingDiagnostic {
2020
return &fakeRoleBindingDiagnostic{
21-
fakeDiagnostic: *newFakeDiagnostic(t),
21+
fakeDiagnostic: *newFakeDiagnostic(t),
22+
fakeClusterRoleBindingList: authapi.ClusterRoleBindingList{},
2223
}
2324
}
2425

25-
func (f *fakeRoleBindingDiagnostic) getClusterRoleBinding(name string) (*authapi.ClusterRoleBinding, error) {
26+
func (f *fakeRoleBindingDiagnostic) listClusterRoleBindings() (*authapi.ClusterRoleBindingList, error) {
2627
if f.err != nil {
2728
return nil, f.err
2829
}
29-
return &f.fakeClusterRoleBinding, nil
30+
return &f.fakeClusterRoleBindingList, nil
3031
}
3132
func (f *fakeRoleBindingDiagnostic) addBinding(name string, namespace string) {
3233
ref := kapi.ObjectReference{
3334
Name: name,
3435
Kind: rbac.ServiceAccountKind,
3536
Namespace: namespace,
3637
}
37-
f.fakeClusterRoleBinding.Subjects = append(f.fakeClusterRoleBinding.Subjects, ref)
38+
if len(f.fakeClusterRoleBindingList.Items) == 0 {
39+
f.fakeClusterRoleBindingList.Items = append(f.fakeClusterRoleBindingList.Items, authapi.ClusterRoleBinding{
40+
RoleRef: kapi.ObjectReference{
41+
Name: clusterReaderRoleBindingRoleName,
42+
},
43+
})
44+
}
45+
f.fakeClusterRoleBindingList.Items[0].Subjects = append(f.fakeClusterRoleBindingList.Items[0].Subjects, ref)
3846
}
3947

4048
// Test error when client error

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ func (d *AggregatedLogging) getScc(name string) (*securityapi.SecurityContextCon
8686
return d.SCCClient.SecurityContextConstraints().Get(name, metav1.GetOptions{})
8787
}
8888

89-
func (d *AggregatedLogging) getClusterRoleBinding(name string) (*authapi.ClusterRoleBinding, error) {
90-
return d.CRBClient.ClusterRoleBindings().Get(name, metav1.GetOptions{})
89+
func (d *AggregatedLogging) listClusterRoleBindings() (*authapi.ClusterRoleBindingList, error) {
90+
return d.CRBClient.ClusterRoleBindings().List(metav1.ListOptions{})
9191
}
9292

9393
func (d *AggregatedLogging) routes(project string, options metav1.ListOptions) (*routesapi.RouteList, error) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ type sccAdapter interface {
2828
}
2929

3030
type clusterRoleBindingsAdapter interface {
31-
getClusterRoleBinding(name string) (*authapi.ClusterRoleBinding, error)
31+
listClusterRoleBindings() (*authapi.ClusterRoleBindingList, error)
3232
}
3333

3434
//deploymentConfigAdapter is an abstraction to retrieve resource for validating dcs

0 commit comments

Comments
 (0)