Skip to content

Commit 2fe87da

Browse files
shaneuttopenshift-merge-bot[bot]
authored andcommitted
feat: add rbac mgmt to gwapi ctrl
Signed-off-by: Shane Utt <[email protected]>
1 parent ca112b7 commit 2fe87da

File tree

4 files changed

+142
-1
lines changed

4 files changed

+142
-1
lines changed

manifests/00-cluster-role.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ rules:
164164
- gatewayclasses
165165
- gateways
166166
- httproutes
167+
- grpcroutes
167168
verbs:
168169
- '*'
169170

pkg/operator/controller/gatewayapi/controller.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
107107
return reconcile.Result{}, err
108108
}
109109

110+
if err := r.ensureGatewayAPIRBAC(ctx); err != nil {
111+
return reconcile.Result{}, err
112+
}
113+
110114
if !r.config.GatewayAPIControllerEnabled {
111115
return reconcile.Result{}, nil
112116
}

pkg/operator/controller/gatewayapi/controller_test.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
configv1 "github.com/openshift/api/config/v1"
1313

14+
rbacv1 "k8s.io/api/rbac/v1"
1415
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1617
"k8s.io/apimachinery/pkg/runtime"
@@ -30,6 +31,11 @@ func Test_Reconcile(t *testing.T) {
3031
ObjectMeta: metav1.ObjectMeta{Name: name},
3132
}
3233
}
34+
clusterRole := func(name string) *rbacv1.ClusterRole {
35+
return &rbacv1.ClusterRole{
36+
ObjectMeta: metav1.ObjectMeta{Name: name},
37+
}
38+
}
3339
tests := []struct {
3440
name string
3541
gatewayAPIEnabled bool
@@ -58,6 +64,8 @@ func Test_Reconcile(t *testing.T) {
5864
crd("grpcroutes.gateway.networking.k8s.io"),
5965
crd("httproutes.gateway.networking.k8s.io"),
6066
crd("referencegrants.gateway.networking.k8s.io"),
67+
clusterRole("system:openshift:gateway-api:aggregate-to-admin"),
68+
clusterRole("system:openshift:gateway-api:aggregate-to-view"),
6169
},
6270
expectUpdate: []client.Object{},
6371
expectDelete: []client.Object{},
@@ -73,6 +81,8 @@ func Test_Reconcile(t *testing.T) {
7381
crd("grpcroutes.gateway.networking.k8s.io"),
7482
crd("httproutes.gateway.networking.k8s.io"),
7583
crd("referencegrants.gateway.networking.k8s.io"),
84+
clusterRole("system:openshift:gateway-api:aggregate-to-admin"),
85+
clusterRole("system:openshift:gateway-api:aggregate-to-view"),
7686
},
7787
expectUpdate: []client.Object{},
7888
expectDelete: []client.Object{},
@@ -83,6 +93,7 @@ func Test_Reconcile(t *testing.T) {
8393
scheme := runtime.NewScheme()
8494
configv1.Install(scheme)
8595
apiextensionsv1.AddToScheme(scheme)
96+
rbacv1.AddToScheme(scheme)
8697

8798
for _, tc := range tests {
8899
t.Run(tc.name, func(t *testing.T) {
@@ -119,9 +130,10 @@ func Test_Reconcile(t *testing.T) {
119130
assert.Equal(t, ctrl.Started, tc.expectStartCtrl, "fake controller should have been started")
120131
cmpOpts := []cmp.Option{
121132
cmpopts.EquateEmpty(),
122-
cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Annotations", "ResourceVersion"),
133+
cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Labels", "Annotations", "ResourceVersion"),
123134
cmpopts.IgnoreFields(metav1.TypeMeta{}, "Kind", "APIVersion"),
124135
cmpopts.IgnoreFields(apiextensionsv1.CustomResourceDefinition{}, "Spec"),
136+
cmpopts.IgnoreFields(rbacv1.ClusterRole{}, "Rules", "AggregationRule"),
125137
}
126138
if diff := cmp.Diff(tc.expectCreate, cl.Added, cmpOpts...); diff != "" {
127139
t.Fatalf("found diff between expected and actual creates: %s", diff)
@@ -140,6 +152,7 @@ func TestReconcileOnlyStartsControllerOnce(t *testing.T) {
140152
scheme := runtime.NewScheme()
141153
configv1.Install(scheme)
142154
apiextensionsv1.AddToScheme(scheme)
155+
rbacv1.AddToScheme(scheme)
143156
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects().Build()
144157
cl := &testutil.FakeClientRecorder{fakeClient, t, []client.Object{}, []client.Object{}, []client.Object{}}
145158
ctrl := &testutil.FakeController{t, false, make(chan struct{})}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package gatewayapi
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/openshift/cluster-ingress-operator/pkg/manifests"
8+
9+
rbacv1 "k8s.io/api/rbac/v1"
10+
"k8s.io/apimachinery/pkg/api/errors"
11+
"k8s.io/apimachinery/pkg/types"
12+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
13+
14+
"github.com/google/go-cmp/cmp"
15+
"github.com/google/go-cmp/cmp/cmpopts"
16+
)
17+
18+
var managedClusterRoles = []*rbacv1.ClusterRole{
19+
manifests.GatewayAPIAdminClusterRole(),
20+
manifests.GatewayAPIViewClusterRole(),
21+
}
22+
23+
// ensureGatewayAPIRBAC ensures that all RBAC resources for Gateway API are
24+
// deployed to the cluster, and up to date.
25+
func (r *reconciler) ensureGatewayAPIRBAC(ctx context.Context) error {
26+
var errs []error
27+
28+
for _, clusterRole := range managedClusterRoles {
29+
if err := r.ensureClusterRole(ctx, clusterRole); err != nil {
30+
errs = append(errs, err)
31+
}
32+
}
33+
34+
return utilerrors.NewAggregate(errs)
35+
}
36+
37+
// ensureClusterRole ensures that a specific ClusterRole RBAC resource is
38+
// deployed to the cluster, and up to date.
39+
func (r *reconciler) ensureClusterRole(ctx context.Context, desired *rbacv1.ClusterRole) error {
40+
clusterRoleAlreadyExists, current, err := r.currentClusterRole(ctx, desired.Name)
41+
if err != nil {
42+
return err
43+
}
44+
45+
if clusterRoleAlreadyExists {
46+
if updated, err := r.updateClusterRole(ctx, current, desired); err != nil {
47+
return err
48+
} else if updated {
49+
if _, _, err := r.currentClusterRole(ctx, desired.Name); err != nil {
50+
return err
51+
}
52+
}
53+
} else {
54+
return r.createClusterRole(ctx, desired)
55+
}
56+
57+
return nil
58+
}
59+
60+
// currentClusterRole retrieves a ClusterRole from the cluster, given the name
61+
// of that role.
62+
func (r *reconciler) currentClusterRole(ctx context.Context, name string) (bool, *rbacv1.ClusterRole, error) {
63+
var existingClusterRole rbacv1.ClusterRole
64+
65+
if err := r.client.Get(ctx, types.NamespacedName{Name: name}, &existingClusterRole); err != nil {
66+
if errors.IsNotFound(err) {
67+
return false, nil, nil
68+
}
69+
return false, nil, fmt.Errorf("failed to get ClusterRole %s: %w", name, err)
70+
}
71+
72+
return true, &existingClusterRole, nil
73+
}
74+
75+
// createClusterRole creates the desired ClusterRole on the cluster.
76+
func (r *reconciler) createClusterRole(ctx context.Context, desired *rbacv1.ClusterRole) error {
77+
desired.ResourceVersion = ""
78+
79+
if err := r.client.Create(ctx, desired); err != nil {
80+
return fmt.Errorf("failed to create ClusterRole %s: %w", desired.Name, err)
81+
}
82+
83+
log.Info("Created ClusterRole", "name", desired.Name)
84+
85+
return nil
86+
}
87+
88+
// updateClusterRole verifies whether the current ClusterRole is up to date with
89+
// what is desired, and performs an update if not.
90+
func (r *reconciler) updateClusterRole(ctx context.Context, current, desired *rbacv1.ClusterRole) (bool, error) {
91+
changed, updated := clusterRoleChanged(current, desired)
92+
if !changed {
93+
return false, nil
94+
}
95+
96+
// Diff before updating because the client may mutate the object.
97+
diff := cmp.Diff(current, updated, cmpopts.EquateEmpty())
98+
if err := r.client.Update(ctx, updated); err != nil {
99+
return false, fmt.Errorf("failed to update ClusterRole %s: %w", updated.Name, err)
100+
}
101+
102+
log.Info("updated ClusterRole", "name", updated.Name, "diff", diff)
103+
104+
return true, nil
105+
}
106+
107+
// clusterRoleChanged indicates whether there are changes between the current
108+
// ClusterRole and what is expected, and provides the updated version of the
109+
// ClusterRole if so.
110+
func clusterRoleChanged(current, expected *rbacv1.ClusterRole) (bool, *rbacv1.ClusterRole) {
111+
if cmp.Equal(current.Rules, expected.Rules) &&
112+
cmp.Equal(current.ObjectMeta.Labels, expected.ObjectMeta.Labels) &&
113+
cmp.Equal(current.ObjectMeta.Annotations, expected.ObjectMeta.Annotations) {
114+
return false, nil
115+
}
116+
117+
updated := current.DeepCopy()
118+
updated.ObjectMeta.Labels = expected.ObjectMeta.Labels
119+
updated.ObjectMeta.Annotations = expected.ObjectMeta.Annotations
120+
updated.Rules = expected.Rules
121+
122+
return true, updated
123+
}

0 commit comments

Comments
 (0)