Skip to content

Commit c1097cd

Browse files
aaronschweigaaronschweig
authored
fix: provide a restmapper to the webhook with a full scope instead of only the limited scope of the vws (#57)
* fix: provide a restmapper to the webhook with a full scope instead of only the limited scope of the vws * tests: provide some tests for the new function * tests: increase coverage in contextual package
1 parent 32a3474 commit c1097cd

File tree

7 files changed

+425
-17
lines changed

7 files changed

+425
-17
lines changed

‎.mockery.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ packages:
1414
github.com/openfga/api/proto/openfga/v1:
1515
config:
1616
include-interface-regex: OpenFGAServiceClient
17+
github.com/platform-mesh/rebac-authz-webhook/pkg/restmapper:
18+
config:
19+
include-interface-regex: Provider
1720
sigs.k8s.io/controller-runtime/pkg/client:
1821
config:
1922
include-interface-regex: ^Client

‎cmd/serve.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/platform-mesh/rebac-authz-webhook/pkg/handler/contextual"
3030
"github.com/platform-mesh/rebac-authz-webhook/pkg/handler/nonresourceattributes"
3131
"github.com/platform-mesh/rebac-authz-webhook/pkg/handler/orgs"
32+
"github.com/platform-mesh/rebac-authz-webhook/pkg/restmapper"
3233

3334
"github.com/kcp-dev/multicluster-provider/apiexport"
3435
openfgav1 "github.com/openfga/api/proto/openfga/v1"
@@ -120,14 +121,16 @@ var serveCmd = &cobra.Command{
120121
klog.Exit(err, "failed to get orgs workspace")
121122
}
122123

124+
mapperProvider := restmapper.New()
125+
123126
extraAttrClusterKey := serverCfg.Webhook.ClusterKey
124127

125128
mgr.GetWebhookServer().Register("/authz", authorization.New(
126129
klog.NewKlogr(),
127130
union.New(
128131
nonresourceattributes.New("/api"),
129132
orgs.New(fga, extraAttrClusterKey, ws.Spec.Cluster, storeRes.Stores[0].Id),
130-
contextual.New(mgr, fga, extraAttrClusterKey),
133+
contextual.New(mgr, fga, mapperProvider, extraAttrClusterKey),
131134
),
132135
))
133136

@@ -138,6 +141,10 @@ var serveCmd = &cobra.Command{
138141
klog.Exit(err, "unable to set up ready check")
139142
}
140143

144+
if err := mgr.Add(mapperProvider); err != nil {
145+
klog.Exit(err, "unable to register rest mapper provider")
146+
}
147+
141148
klog.Info("Starting provider")
142149
go func() {
143150
if err := provider.Run(ctx, mgr); err != nil {

‎pkg/handler/contextual/contextual.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package contextual
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"strings"
78

89
openfgav1 "github.com/openfga/api/proto/openfga/v1"
910
"github.com/platform-mesh/rebac-authz-webhook/pkg/authorization"
11+
"github.com/platform-mesh/rebac-authz-webhook/pkg/restmapper"
1012
"github.com/platform-mesh/rebac-authz-webhook/pkg/util"
1113
"k8s.io/apimachinery/pkg/runtime/schema"
1214
"k8s.io/apimachinery/pkg/types"
@@ -20,18 +22,20 @@ import (
2022
const maxRelationLength = 50
2123

2224
type contextualAuthorizer struct {
23-
clusterKey string
24-
mgr mcmanager.Manager
25-
fga openfgav1.OpenFGAServiceClient
25+
clusterKey string
26+
mgr mcmanager.Manager
27+
fga openfgav1.OpenFGAServiceClient
28+
mapperProvider restmapper.Provider
2629
}
2730

2831
var _ authorization.Handler = &contextualAuthorizer{}
2932

30-
func New(mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, clusterKey string) authorization.Handler {
33+
func New(mgr mcmanager.Manager, fga openfgav1.OpenFGAServiceClient, mapperProvider restmapper.Provider, clusterKey string) authorization.Handler {
3134
return &contextualAuthorizer{
32-
mgr: mgr,
33-
fga: fga,
34-
clusterKey: clusterKey,
35+
mgr: mgr,
36+
fga: fga,
37+
clusterKey: clusterKey,
38+
mapperProvider: mapperProvider,
3539
}
3640
}
3741

@@ -82,7 +86,12 @@ func (c *contextualAuthorizer) Handle(ctx context.Context, req authorization.Req
8286
Resource: attrs.Resource,
8387
}
8488

85-
restMapper := accountInfoCluster.GetRESTMapper()
89+
restMapper, ok := c.mapperProvider.Get(clusterName)
90+
if !ok {
91+
klog.ErrorS(errors.New("failed to get RESTMapper for cluster"), "clusterName", clusterName)
92+
return authorization.NoOpinion()
93+
}
94+
8695
gvk, err := restMapper.KindFor(gvr)
8796
if err != nil {
8897
klog.ErrorS(err, "failed to get GVK for GVR", "GVR", gvr)

‎pkg/handler/contextual/contextual_test.go‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func TestHandler(t *testing.T) {
2626
res authorization.Response
2727
fgaMocks func(openfga *mocks.OpenFGAServiceClient)
2828
k8sMocks func(client *mocks.Client, cluster *mocks.Cluster)
29+
rmpMocks func(rmp *mocks.Provider)
2930
}{
3031
{
3132
name: "should skip processing if no extra attrs present",
@@ -65,6 +66,43 @@ func TestHandler(t *testing.T) {
6566

6667
},
6768
},
69+
{
70+
name: "should skip processing if restmapper cannot be retrieved",
71+
req: authorization.Request{
72+
SubjectAccessReview: v1.SubjectAccessReview{
73+
Spec: v1.SubjectAccessReviewSpec{
74+
Extra: map[string]v1.ExtraValue{
75+
"authorization.kubernetes.io/cluster-name": {"a"},
76+
},
77+
ResourceAttributes: &v1.ResourceAttributes{},
78+
},
79+
},
80+
},
81+
res: authorization.NoOpinion(),
82+
k8sMocks: func(cl *mocks.Client, cluster *mocks.Cluster) {
83+
cl.EXPECT().
84+
Get(mock.Anything, mock.Anything, mock.Anything, mock.Anything).
85+
RunAndReturn(
86+
func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
87+
acc := obj.(*v1alpha1.AccountInfo)
88+
89+
*acc = v1alpha1.AccountInfo{
90+
Spec: v1alpha1.AccountInfoSpec{
91+
Account: v1alpha1.AccountLocation{
92+
OriginClusterId: "origin",
93+
Name: "origin-account",
94+
},
95+
},
96+
}
97+
return nil
98+
},
99+
)
100+
101+
},
102+
rmpMocks: func(rmp *mocks.Provider) {
103+
rmp.EXPECT().Get(mock.Anything).Return(nil, false)
104+
},
105+
},
68106
{
69107
name: "should process request non-parent, non-namespaced successfully",
70108
req: authorization.Request{
@@ -103,6 +141,8 @@ func TestHandler(t *testing.T) {
103141
},
104142
)
105143

144+
},
145+
rmpMocks: func(rmp *mocks.Provider) {
106146
rm := meta.NewDefaultRESTMapper([]schema.GroupVersion{})
107147

108148
gv := schema.GroupVersion{
@@ -117,7 +157,7 @@ func TestHandler(t *testing.T) {
117157
meta.RESTScopeRoot,
118158
)
119159

120-
cluster.EXPECT().GetRESTMapper().Return(rm)
160+
rmp.EXPECT().Get(mock.Anything).Return(rm, true)
121161
},
122162
fgaMocks: func(openfga *mocks.OpenFGAServiceClient) {
123163
openfga.EXPECT().Check(mock.Anything, mock.Anything).RunAndReturn(
@@ -181,7 +221,8 @@ func TestHandler(t *testing.T) {
181221
return nil
182222
},
183223
)
184-
224+
},
225+
rmpMocks: func(rmp *mocks.Provider) {
185226
rm := meta.NewDefaultRESTMapper([]schema.GroupVersion{})
186227

187228
gv := schema.GroupVersion{
@@ -196,7 +237,7 @@ func TestHandler(t *testing.T) {
196237
meta.RESTScopeNamespace,
197238
)
198239

199-
cluster.EXPECT().GetRESTMapper().Return(rm)
240+
rmp.EXPECT().Get(mock.Anything).Return(rm, true)
200241
},
201242
fgaMocks: func(openfga *mocks.OpenFGAServiceClient) {
202243
openfga.EXPECT().Check(mock.Anything, mock.Anything).RunAndReturn(
@@ -268,7 +309,8 @@ func TestHandler(t *testing.T) {
268309
return nil
269310
},
270311
)
271-
312+
},
313+
rmpMocks: func(rmp *mocks.Provider) {
272314
rm := meta.NewDefaultRESTMapper([]schema.GroupVersion{})
273315

274316
gv := schema.GroupVersion{
@@ -283,7 +325,7 @@ func TestHandler(t *testing.T) {
283325
meta.RESTScopeNamespace,
284326
)
285327

286-
cluster.EXPECT().GetRESTMapper().Return(rm)
328+
rmp.EXPECT().Get(mock.Anything).Return(rm, true)
287329
},
288330
fgaMocks: func(openfga *mocks.OpenFGAServiceClient) {
289331
openfga.EXPECT().Check(mock.Anything, mock.Anything).RunAndReturn(
@@ -346,7 +388,8 @@ func TestHandler(t *testing.T) {
346388
return nil
347389
},
348390
)
349-
391+
},
392+
rmpMocks: func(rmp *mocks.Provider) {
350393
rm := meta.NewDefaultRESTMapper([]schema.GroupVersion{})
351394

352395
gv := schema.GroupVersion{
@@ -361,7 +404,7 @@ func TestHandler(t *testing.T) {
361404
meta.RESTScopeRoot,
362405
)
363406

364-
cluster.EXPECT().GetRESTMapper().Return(rm)
407+
rmp.EXPECT().Get(mock.Anything).Return(rm, true)
365408
},
366409
fgaMocks: func(openfga *mocks.OpenFGAServiceClient) {
367410
openfga.EXPECT().Check(mock.Anything, mock.Anything).RunAndReturn(
@@ -398,6 +441,11 @@ func TestHandler(t *testing.T) {
398441
test.k8sMocks(client, cluster)
399442
}
400443

444+
rmp := mocks.NewProvider(t)
445+
if test.rmpMocks != nil {
446+
test.rmpMocks(rmp)
447+
}
448+
401449
mgr.EXPECT().GetCluster(mock.Anything, mock.Anything).Return(cluster, nil).Maybe()
402450
cluster.EXPECT().GetClient().Return(client).Maybe()
403451

@@ -406,7 +454,7 @@ func TestHandler(t *testing.T) {
406454
test.fgaMocks(openfga)
407455
}
408456

409-
h := contextual.New(mgr, openfga, "authorization.kubernetes.io/cluster-name")
457+
h := contextual.New(mgr, openfga, rmp, "authorization.kubernetes.io/cluster-name")
410458

411459
ctx := t.Context()
412460

0 commit comments

Comments
 (0)