Skip to content

Commit 3267edb

Browse files
fxiang1fxiang1
authored
Fix empty subject attempt #2 (#71)
* Fix empty subject attempt #2 Signed-off-by: fxiang1 <[email protected]> * Switch to new builder image Signed-off-by: fxiang1 <[email protected]> * Use Go 1.24 image Signed-off-by: fxiang1 <[email protected]> * Try with Go 1.23 Signed-off-by: fxiang1 <[email protected]> * Bump workflow Go version Signed-off-by: fxiang1 <[email protected]> * Bump ubi image Signed-off-by: fxiang1 <[email protected]> --------- Signed-off-by: fxiang1 <[email protected]>
1 parent 5b01809 commit 3267edb

File tree

7 files changed

+61
-42
lines changed

7 files changed

+61
-42
lines changed

‎.github/workflows/go-presubmit.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ on:
88

99
env:
1010
# Common versions
11-
GO_VERSION: '1.21'
11+
GO_VERSION: '1.23'
1212
GO_REQUIRED_MIN_VERSION: ''
1313
GOPATH: '/home/runner/work/cluster-permission/cluster-permission/go'
1414
defaults:

‎Dockerfile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
FROM registry.ci.openshift.org/stolostron/builder:go1.21-linux AS builder
1+
FROM registry.ci.openshift.org/stolostron/builder:go1.23-linux AS builder
22

33
WORKDIR /go/src/github.com/open-cluster-management-io/cluster-permission
44
COPY . .
55
RUN make -f Makefile build
66

7-
FROM registry.access.redhat.com/ubi8/ubi-minimal:latest
7+
FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
88

99
RUN microdnf update && \
1010
microdnf clean all

‎Dockerfile.rhtap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_8_1.21 AS plugin-builder
1+
FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_9_1.23 AS plugin-builder
22

33
WORKDIR /go/src/github.com/open-cluster-management-io/cluster-permission
44
COPY . .
55
RUN rm -fr vendor && \
66
go mod vendor && \
77
make -f Makefile build
88

9-
FROM registry.access.redhat.com/ubi8/ubi-minimal:latest
9+
FROM registry.access.redhat.com/ubi9/ubi-minimal:latest
1010

1111
RUN microdnf update && \
1212
microdnf clean all

‎api/v1alpha1/clusterpermission_types.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ type ClusterRoleBinding struct {
7373
// Besides the typical subject for a binding, a ManagedServiceAccount can be used as a subject as well.
7474
// If both subject and subjects exist then only subjects will be used.
7575
// +optional
76-
Subject rbacv1.Subject `json:"subject,omitempty"`
76+
Subject *rbacv1.Subject `json:"subject,omitempty"`
7777

7878
// Subjects contains an array of references to objects or user identities a ClusterPermission binding applies to.
7979
// Besides the typical subject for a binding, a ManagedServiceAccount can be used as a subject as well.
@@ -112,13 +112,13 @@ type RoleBinding struct {
112112
// Besides the typical subject for a binding, a ManagedServiceAccount can be used as a subject as well.
113113
// If both subject and subjects exist then only subjects will be used.
114114
// +optional
115-
rbacv1.Subject `json:"subject"`
115+
*rbacv1.Subject `json:"subject,omitempty"`
116116

117117
// Subjects contains an array of references to objects or user identities a ClusterPermission binding applies to.
118118
// Besides the typical subject for a binding, a ManagedServiceAccount can be used as a subject as well.
119119
// If both subject and subjects exist then only subjects will be used.
120120
// +optional
121-
Subjects []rbacv1.Subject `json:"subjects"`
121+
Subjects []rbacv1.Subject `json:"subjects,omitempty"`
122122

123123
// RoleRef contains information that points to the role being used
124124
// +required

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 10 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/clusterpermission_controller.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,12 +283,15 @@ func (r *ClusterPermissionReconciler) validateSubject(ctx context.Context, subje
283283
return nil
284284
}
285285

286-
func getSubjects(subject rbacv1.Subject, subjects []rbacv1.Subject) []rbacv1.Subject {
286+
func getSubjects(subject *rbacv1.Subject, subjects []rbacv1.Subject) []rbacv1.Subject {
287287
if len(subjects) > 0 {
288288
return subjects
289289
} else {
290290
// should be safe since one of them has to exist due to CRD validation
291-
return []rbacv1.Subject{subject}
291+
if subject == nil {
292+
return []rbacv1.Subject{}
293+
}
294+
return []rbacv1.Subject{*subject}
292295
}
293296
}
294297

@@ -739,7 +742,7 @@ func (r *ClusterPermissionReconciler) clusterPermissionUsesManagedServiceAccount
739742
if r.subjectIsManagedServiceAccount(cp.Spec.ClusterRoleBinding.Subject) {
740743
return true
741744
}
742-
if slices.ContainsFunc(cp.Spec.ClusterRoleBinding.Subjects, r.subjectIsManagedServiceAccount) {
745+
if slices.ContainsFunc(cp.Spec.ClusterRoleBinding.Subjects, r.subjectIsManagedServiceAccountByValue) {
743746
return true
744747
}
745748
}
@@ -750,7 +753,7 @@ func (r *ClusterPermissionReconciler) clusterPermissionUsesManagedServiceAccount
750753
if r.subjectIsManagedServiceAccount(crb.Subject) {
751754
return true
752755
}
753-
if slices.ContainsFunc(crb.Subjects, r.subjectIsManagedServiceAccount) {
756+
if slices.ContainsFunc(crb.Subjects, r.subjectIsManagedServiceAccountByValue) {
754757
return true
755758
}
756759
}
@@ -762,7 +765,7 @@ func (r *ClusterPermissionReconciler) clusterPermissionUsesManagedServiceAccount
762765
if r.subjectIsManagedServiceAccount(rb.Subject) {
763766
return true
764767
}
765-
if slices.ContainsFunc(rb.Subjects, r.subjectIsManagedServiceAccount) {
768+
if slices.ContainsFunc(rb.Subjects, r.subjectIsManagedServiceAccountByValue) {
766769
return true
767770
}
768771
}
@@ -772,6 +775,14 @@ func (r *ClusterPermissionReconciler) clusterPermissionUsesManagedServiceAccount
772775
}
773776

774777
// subjectIsManagedServiceAccount checks if a subject is a ManagedServiceAccount
775-
func (r *ClusterPermissionReconciler) subjectIsManagedServiceAccount(subject rbacv1.Subject) bool {
778+
func (r *ClusterPermissionReconciler) subjectIsManagedServiceAccount(subject *rbacv1.Subject) bool {
779+
if subject == nil {
780+
return false
781+
}
776782
return subject.APIGroup == msav1beta1.GroupVersion.Group && subject.Kind == "ManagedServiceAccount"
777783
}
784+
785+
// subjectIsManagedServiceAccountByValue is a wrapper for use with slices.ContainsFunc
786+
func (r *ClusterPermissionReconciler) subjectIsManagedServiceAccountByValue(subject rbacv1.Subject) bool {
787+
return r.subjectIsManagedServiceAccount(&subject)
788+
}

‎controllers/clusterpermission_controller_test.go‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ var _ = Describe("ClusterPermission controller", func() {
108108
}},
109109
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
110110
Subjects: []rbacv1.Subject{},
111-
Subject: rbacv1.Subject{
111+
Subject: &rbacv1.Subject{
112112
Kind: "ServiceAccount",
113113
Name: "klusterlet-work-sa",
114114
Namespace: "open-cluster-management-agent",
@@ -119,7 +119,7 @@ var _ = Describe("ClusterPermission controller", func() {
119119
Namespace: "default",
120120
RoleRef: cpv1alpha1.RoleRef{Kind: "ClusterRole"},
121121
Subjects: []rbacv1.Subject{},
122-
Subject: rbacv1.Subject{
122+
Subject: &rbacv1.Subject{
123123
Kind: "ServiceAccount",
124124
Name: "klusterlet-work-sa",
125125
Namespace: "open-cluster-management-agent",
@@ -216,7 +216,7 @@ var _ = Describe("ClusterPermission controller", func() {
216216
}},
217217
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
218218
Subjects: []rbacv1.Subject{},
219-
Subject: rbacv1.Subject{
219+
Subject: &rbacv1.Subject{
220220
APIGroup: "authentication.open-cluster-management.io",
221221
Kind: "ManagedServiceAccount",
222222
Name: msaName,
@@ -227,7 +227,7 @@ var _ = Describe("ClusterPermission controller", func() {
227227
Namespace: "default",
228228
RoleRef: cpv1alpha1.RoleRef{Kind: "ClusterRole"},
229229
Subjects: []rbacv1.Subject{},
230-
Subject: rbacv1.Subject{
230+
Subject: &rbacv1.Subject{
231231
APIGroup: "authentication.open-cluster-management.io",
232232
Kind: "ManagedServiceAccount",
233233
Name: msaName,
@@ -274,7 +274,7 @@ var _ = Describe("ClusterPermission controller", func() {
274274
NamespaceSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}},
275275
RoleRef: cpv1alpha1.RoleRef{Kind: "ClusterRole"},
276276
Subjects: []rbacv1.Subject{},
277-
Subject: rbacv1.Subject{
277+
Subject: &rbacv1.Subject{
278278
APIGroup: "authentication.open-cluster-management.io",
279279
Kind: "ManagedServiceAccount",
280280
Name: msaName,
@@ -322,7 +322,7 @@ var _ = Describe("ClusterPermission controller", func() {
322322
Name: "argocd-application-controller-1",
323323
},
324324
Subjects: []rbacv1.Subject{},
325-
Subject: rbacv1.Subject{
325+
Subject: &rbacv1.Subject{
326326
Namespace: "openshift-gitops",
327327
Kind: "ServiceAccount",
328328
Name: "sa-sample-existing",
@@ -337,7 +337,7 @@ var _ = Describe("ClusterPermission controller", func() {
337337
Name: "argocd-application-controller-2",
338338
},
339339
Subjects: []rbacv1.Subject{},
340-
Subject: rbacv1.Subject{
340+
Subject: &rbacv1.Subject{
341341
APIGroup: "rbac.authorization.k8s.io",
342342
Kind: "User",
343343
Name: "user1",
@@ -351,7 +351,7 @@ var _ = Describe("ClusterPermission controller", func() {
351351
Name: "argocd-application-controller-2",
352352
},
353353
Subjects: []rbacv1.Subject{},
354-
Subject: rbacv1.Subject{
354+
Subject: &rbacv1.Subject{
355355
APIGroup: "rbac.authorization.k8s.io",
356356
Kind: "User",
357357
Name: "user1",
@@ -366,7 +366,7 @@ var _ = Describe("ClusterPermission controller", func() {
366366
Name: "argocd-application-controller-3",
367367
},
368368
Subjects: []rbacv1.Subject{},
369-
Subject: rbacv1.Subject{
369+
Subject: &rbacv1.Subject{
370370
APIGroup: "rbac.authorization.k8s.io",
371371
Kind: "Group",
372372
Name: "group1",
@@ -401,7 +401,7 @@ var _ = Describe("ClusterPermission controller", func() {
401401
Name: "argocd-application-controller-1",
402402
},
403403
Subjects: []rbacv1.Subject{},
404-
Subject: rbacv1.Subject{
404+
Subject: &rbacv1.Subject{
405405
Kind: "User",
406406
Name: "user1",
407407
},
@@ -490,7 +490,7 @@ var _ = Describe("ClusterPermission controller", func() {
490490
},
491491
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
492492
Subjects: []rbacv1.Subject{},
493-
Subject: rbacv1.Subject{
493+
Subject: &rbacv1.Subject{
494494
APIGroup: "authentication.open-cluster-management.io",
495495
Kind: "ManagedServiceAccount",
496496
Name: msaName + "1",
@@ -538,7 +538,7 @@ var _ = Describe("ClusterPermission controller", func() {
538538
RoleBindings: &[]cpv1alpha1.RoleBinding{
539539
{
540540
Subjects: []rbacv1.Subject{},
541-
Subject: rbacv1.Subject{
541+
Subject: &rbacv1.Subject{
542542
APIGroup: "authentication.open-cluster-management.io",
543543
Kind: "ManagedServiceAccount",
544544
Name: msaName,
@@ -624,7 +624,7 @@ var _ = Describe("ClusterPermission controller", func() {
624624
Name: "argocd-application-controller-1",
625625
},
626626
Subjects: []rbacv1.Subject{},
627-
Subject: rbacv1.Subject{
627+
Subject: &rbacv1.Subject{
628628
Namespace: "openshift-gitops",
629629
Kind: "ServiceAccount",
630630
Name: "sa-sample-existing",
@@ -652,7 +652,7 @@ var _ = Describe("ClusterPermission controller", func() {
652652
Kind: "ClusterRole",
653653
},
654654
Subjects: []rbacv1.Subject{},
655-
Subject: rbacv1.Subject{
655+
Subject: &rbacv1.Subject{
656656
Namespace: "openshift-gitops",
657657
Kind: "ServiceAccount",
658658
Name: "sa-sample-existing",
@@ -665,7 +665,7 @@ var _ = Describe("ClusterPermission controller", func() {
665665
Expect(k8sClient.Create(ctx, &clusterPermissionMissingName)).Should(Succeed())
666666

667667
By("Test getSubjects()")
668-
Expect(len(getSubjects(rbacv1.Subject{}, []rbacv1.Subject{
668+
Expect(len(getSubjects(&rbacv1.Subject{}, []rbacv1.Subject{
669669
{
670670
Namespace: "openshift-gitops",
671671
Kind: "ServiceAccount",
@@ -849,7 +849,7 @@ func TestGenerateSubjects(t *testing.T) {
849849
Scheme: testscheme,
850850
}
851851

852-
subjects, err := cpr.generateSubjects(context.TODO(), getSubjects(c.subject, c.subjects), c.clusterNamespace)
852+
subjects, err := cpr.generateSubjects(context.TODO(), getSubjects(&c.subject, c.subjects), c.clusterNamespace)
853853
if err != nil {
854854
t.Errorf("generateSubjects() error = %v", err)
855855
}
@@ -1345,7 +1345,7 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
13451345
},
13461346
Spec: cpv1alpha1.ClusterPermissionSpec{
13471347
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
1348-
Subject: rbacv1.Subject{
1348+
Subject: &rbacv1.Subject{
13491349
APIGroup: msav1beta1.GroupVersion.Group,
13501350
Kind: "ManagedServiceAccount",
13511351
Name: "test-msa",
@@ -1360,7 +1360,7 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
13601360
},
13611361
Spec: cpv1alpha1.ClusterPermissionSpec{
13621362
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
1363-
Subject: rbacv1.Subject{
1363+
Subject: &rbacv1.Subject{
13641364
Kind: "ServiceAccount",
13651365
Name: "regular-sa",
13661366
Namespace: "default",
@@ -1377,7 +1377,7 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
13771377
RoleBindings: &[]cpv1alpha1.RoleBinding{
13781378
{
13791379
Namespace: "default",
1380-
Subject: rbacv1.Subject{
1380+
Subject: &rbacv1.Subject{
13811381
APIGroup: msav1beta1.GroupVersion.Group,
13821382
Kind: "ManagedServiceAccount",
13831383
Name: "another-msa",
@@ -1418,7 +1418,7 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
14181418
},
14191419
Spec: cpv1alpha1.ClusterPermissionSpec{
14201420
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
1421-
Subject: rbacv1.Subject{
1421+
Subject: &rbacv1.Subject{
14221422
Kind: "ServiceAccount",
14231423
Name: "regular-sa",
14241424
Namespace: "default",
@@ -1433,7 +1433,7 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
14331433
},
14341434
Spec: cpv1alpha1.ClusterPermissionSpec{
14351435
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
1436-
Subject: rbacv1.Subject{
1436+
Subject: &rbacv1.Subject{
14371437
Kind: "User",
14381438
Name: "test-user",
14391439
},
@@ -1501,14 +1501,14 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
15011501
Spec: cpv1alpha1.ClusterPermissionSpec{
15021502
ClusterRoleBindings: &[]cpv1alpha1.ClusterRoleBinding{
15031503
{
1504-
Subject: rbacv1.Subject{
1504+
Subject: &rbacv1.Subject{
15051505
Kind: "ServiceAccount",
15061506
Name: "regular-sa",
15071507
Namespace: "default",
15081508
},
15091509
},
15101510
{
1511-
Subject: rbacv1.Subject{
1511+
Subject: &rbacv1.Subject{
15121512
APIGroup: msav1beta1.GroupVersion.Group,
15131513
Kind: "ManagedServiceAccount",
15141514
Name: "test-msa",
@@ -1554,7 +1554,7 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
15541554
},
15551555
Spec: cpv1alpha1.ClusterPermissionSpec{
15561556
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
1557-
Subject: rbacv1.Subject{
1557+
Subject: &rbacv1.Subject{
15581558
APIGroup: msav1beta1.GroupVersion.Group,
15591559
Kind: "ManagedServiceAccount",
15601560
Name: "test-msa",
@@ -1592,15 +1592,15 @@ func TestManagedClusterAddOnEventHandler(t *testing.T) {
15921592
},
15931593
Spec: cpv1alpha1.ClusterPermissionSpec{
15941594
ClusterRoleBinding: &cpv1alpha1.ClusterRoleBinding{
1595-
Subject: rbacv1.Subject{
1595+
Subject: &rbacv1.Subject{
15961596
Kind: "ServiceAccount",
15971597
Name: "regular-sa",
15981598
Namespace: "default",
15991599
},
16001600
},
16011601
ClusterRoleBindings: &[]cpv1alpha1.ClusterRoleBinding{
16021602
{
1603-
Subject: rbacv1.Subject{
1603+
Subject: &rbacv1.Subject{
16041604
APIGroup: msav1beta1.GroupVersion.Group,
16051605
Kind: "ManagedServiceAccount",
16061606
Name: "test-msa-1",

0 commit comments

Comments
 (0)