Skip to content

Commit 7c9a05e

Browse files
trgeigertrgeiger
committed
Clean up and organize authorization package
Move all authorization-related code to the authorization package, rename anything using 'auth' to 'authorization' to avoid confusion with authentication.
1 parent 5b9fd7a commit 7c9a05e

File tree

4 files changed

+77
-73
lines changed

4 files changed

+77
-73
lines changed

‎cmd/operator-controller/main.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import (
6161
"github.com/operator-framework/operator-controller/internal/operator-controller/action"
6262
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
6363
"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
64+
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
6465
"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/cache"
6566
catalogclient "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/client"
6667
"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager"
@@ -409,16 +410,16 @@ func run() error {
409410
crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()),
410411
}
411412

412-
acm := applier.NewAuthClientMapper(clientRestConfigMapper, mgr.GetConfig())
413+
acm := authorization.NewAuthorizationClientMapper(clientRestConfigMapper, mgr.GetConfig())
413414
acm.NewForConfig = func(cfg *rest.Config) (authorizationv1client.AuthorizationV1Interface, error) {
414415
// *AuthorizationV1Client implements AuthorizationV1Interface
415416
return authorizationv1client.NewForConfig(cfg)
416417
}
417418

418419
helmApplier := &applier.Helm{
419-
ActionClientGetter: acg,
420-
Preflights: preflights,
421-
AuthClientMapper: acm,
420+
ActionClientGetter: acg,
421+
Preflights: preflights,
422+
AuthorizationClientMapper: acm,
422423
}
423424

424425
cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper())

‎internal/operator-controller/applier/helm.go‎

Lines changed: 6 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ import (
1818
corev1 "k8s.io/api/core/v1"
1919
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2020
apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
21-
authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
22-
"k8s.io/client-go/rest"
2321
"sigs.k8s.io/controller-runtime/pkg/client"
2422
"sigs.k8s.io/controller-runtime/pkg/log"
2523

@@ -56,36 +54,10 @@ type Preflight interface {
5654
Upgrade(context.Context, *release.Release) error
5755
}
5856

59-
type RestConfigMapper func(context.Context, client.Object, *rest.Config) (*rest.Config, error)
60-
61-
type NewForConfigFunc func(*rest.Config) (authorizationv1client.AuthorizationV1Interface, error)
62-
63-
type AuthClientMapper struct {
64-
rcm RestConfigMapper
65-
baseCfg *rest.Config
66-
NewForConfig NewForConfigFunc
67-
}
68-
69-
func (acm *AuthClientMapper) GetAuthenticationClient(ctx context.Context, ext *ocv1.ClusterExtension) (authorizationv1client.AuthorizationV1Interface, error) {
70-
authcfg, err := acm.rcm(ctx, ext, acm.baseCfg)
71-
if err != nil {
72-
return nil, err
73-
}
74-
75-
return acm.NewForConfig(authcfg)
76-
}
77-
7857
type Helm struct {
79-
ActionClientGetter helmclient.ActionClientGetter
80-
Preflights []Preflight
81-
AuthClientMapper AuthClientMapper
82-
}
83-
84-
func NewAuthClientMapper(rcm RestConfigMapper, baseCfg *rest.Config) AuthClientMapper {
85-
return AuthClientMapper{
86-
rcm: rcm,
87-
baseCfg: baseCfg,
88-
}
58+
ActionClientGetter helmclient.ActionClientGetter
59+
Preflights []Preflight
60+
AuthorizationClientMapper authorization.AuthorizationClientMapper
8961
}
9062

9163
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
@@ -110,14 +82,14 @@ func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.Clu
11082

11183
func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
11284
if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) {
113-
authclient, err := h.AuthClientMapper.GetAuthenticationClient(ctx, ext)
85+
authclient, err := h.AuthorizationClientMapper.GetAuthorizationClient(ctx, ext)
11486
if err != nil {
11587
return nil, "", err
11688
}
11789

118-
err = h.checkContentPermissions(ctx, contentFS, authclient, ext)
90+
err = h.AuthorizationClientMapper.CheckContentPermissions(ctx, contentFS, authclient, ext)
11991
if err != nil {
120-
return nil, "", err
92+
return nil, "", fmt.Errorf("failed checking content permissions: %w", err)
12193
}
12294
}
12395

@@ -195,23 +167,6 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
195167
return relObjects, state, nil
196168
}
197169

198-
// Check if RBAC allows the installer service account necessary permissions on the objects in the contentFS
199-
func (h *Helm) checkContentPermissions(ctx context.Context, contentFS fs.FS, authcl authorizationv1client.AuthorizationV1Interface, ext *ocv1.ClusterExtension) error {
200-
reg, err := convert.ParseFS(ctx, contentFS)
201-
if err != nil {
202-
return err
203-
}
204-
205-
plain, err := convert.Convert(reg, ext.Spec.Namespace, []string{corev1.NamespaceAll})
206-
if err != nil {
207-
return err
208-
}
209-
210-
err = authorization.CheckObjectPermissions(ctx, authcl, plain.Objects, ext)
211-
212-
return err
213-
}
214-
215170
func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, *release.Release, string, error) {
216171
currentRelease, err := cl.Get(ext.GetName())
217172

‎internal/operator-controller/applier/helm_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -115,13 +115,13 @@ func newPassingSSRRAuthClient() authorizationv1client.AuthorizationV1Interface {
115115
}
116116
}
117117

118-
// Helper builds an AuthClientMapper with the passing SSRR
119-
func newPassingAuthClientMapper() applier.AuthClientMapper {
118+
// Helper builds an AuthorizationClientMapper with the passing SSRR
119+
func newPassingAuthorizationClientMapper() applier.AuthorizationClientMapper {
120120
fakeRestConfig := &rest.Config{Host: "fake-server"}
121121
mockRCM := func(ctx context.Context, obj client.Object, cfg *rest.Config) (*rest.Config, error) {
122122
return cfg, nil
123123
}
124-
acm := applier.NewAuthClientMapper(mockRCM, fakeRestConfig)
124+
acm := applier.NewAuthorizationClientMapper(mockRCM, fakeRestConfig)
125125
acm.NewForConfig = func(*rest.Config) (authorizationv1client.AuthorizationV1Interface, error) {
126126
return newPassingSSRRAuthClient(), nil
127127
}
@@ -131,9 +131,9 @@ func newPassingAuthClientMapper() applier.AuthClientMapper {
131131
// Helper builds a Helm applier with passing SSRR
132132
func buildHelmApplier(mockAcg *mockActionGetter, preflights []applier.Preflight) applier.Helm {
133133
return applier.Helm{
134-
ActionClientGetter: mockAcg,
135-
AuthClientMapper: newPassingAuthClientMapper(),
136-
Preflights: preflights,
134+
ActionClientGetter: mockAcg,
135+
AuthorizationClientMapper: newPassingAuthorizationClientMapper(),
136+
Preflights: preflights,
137137
}
138138
}
139139

‎internal/operator-controller/authorization/authorization.go‎

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,79 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"io/fs"
78
"slices"
89
"strings"
910

10-
authorizationv1 "k8s.io/api/authorization/v1"
11+
ocv1 "github.com/operator-framework/operator-controller/api/v1"
12+
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
13+
authv1 "k8s.io/api/authorization/v1"
14+
corev1 "k8s.io/api/core/v1"
1115
rbacv1 "k8s.io/api/rbac/v1"
12-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
16+
17+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1318
authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
19+
"k8s.io/client-go/rest"
1420
"sigs.k8s.io/controller-runtime/pkg/client"
15-
16-
ocv1 "github.com/operator-framework/operator-controller/api/v1"
1721
)
1822

1923
const (
2024
SelfSubjectRulesReview string = "SelfSubjectRulesReview"
2125
SelfSubjectAccessReview string = "SelfSubjectAccessReview"
2226
)
2327

24-
func CheckObjectPermissions(ctx context.Context, authcl authorizationv1client.AuthorizationV1Interface, objects []client.Object, ext *ocv1.ClusterExtension) error {
25-
ssrr := &authorizationv1.SelfSubjectRulesReview{
26-
Spec: authorizationv1.SelfSubjectRulesReviewSpec{
28+
type RestConfigMapper func(context.Context, client.Object, *rest.Config) (*rest.Config, error)
29+
30+
type NewForConfigFunc func(*rest.Config) (authorizationv1client.AuthorizationV1Interface, error)
31+
32+
type AuthorizationClientMapper struct {
33+
rcm RestConfigMapper
34+
baseCfg *rest.Config
35+
NewForConfig NewForConfigFunc
36+
}
37+
38+
func NewAuthorizationClientMapper(rcm RestConfigMapper, baseCfg *rest.Config) AuthorizationClientMapper {
39+
return AuthorizationClientMapper{
40+
rcm: rcm,
41+
baseCfg: baseCfg,
42+
}
43+
}
44+
45+
func (acm *AuthorizationClientMapper) GetAuthorizationClient(ctx context.Context, ext *ocv1.ClusterExtension) (authorizationv1client.AuthorizationV1Interface, error) {
46+
authcfg, err := acm.rcm(ctx, ext, acm.baseCfg)
47+
if err != nil {
48+
return nil, err
49+
}
50+
51+
return acm.NewForConfig(authcfg)
52+
}
53+
54+
// Check if RBAC allows the installer service account necessary permissions on the objects in the contentFS
55+
func (acm *AuthorizationClientMapper) CheckContentPermissions(ctx context.Context, contentFS fs.FS, authcl authorizationv1client.AuthorizationV1Interface, ext *ocv1.ClusterExtension) error {
56+
reg, err := convert.ParseFS(ctx, contentFS)
57+
if err != nil {
58+
return err
59+
}
60+
61+
plain, err := convert.Convert(reg, ext.Spec.Namespace, []string{corev1.NamespaceAll})
62+
if err != nil {
63+
return err
64+
}
65+
66+
err = checkObjectPermissions(ctx, authcl, plain.Objects, ext)
67+
68+
return err
69+
}
70+
71+
func checkObjectPermissions(ctx context.Context, authcl authorizationv1client.AuthorizationV1Interface, objects []client.Object, ext *ocv1.ClusterExtension) error {
72+
ssrr := &authv1.SelfSubjectRulesReview{
73+
Spec: authv1.SelfSubjectRulesReviewSpec{
2774
Namespace: ext.Spec.Namespace,
2875
},
2976
}
3077

31-
ssrr, err := authcl.SelfSubjectRulesReviews().Create(ctx, ssrr, metav1.CreateOptions{})
78+
opts := v1.CreateOptions{}
79+
ssrr, err := authcl.SelfSubjectRulesReviews().Create(ctx, ssrr, opts)
3280
if err != nil {
3381
return err
3482
}
@@ -50,14 +98,14 @@ func CheckObjectPermissions(ctx context.Context, authcl authorizationv1client.Au
5098
})
5199
}
52100

53-
resAttrs := []authorizationv1.ResourceAttributes{}
54101
namespacedErrs := []error{}
55102
clusterScopedErrs := []error{}
56103
requiredVerbs := []string{"get", "create", "update", "list", "watch", "delete", "patch"}
104+
resAttrs := make([]authv1.ResourceAttributes, 0, len(requiredVerbs)*len(objects))
57105

58106
for _, o := range objects {
59107
for _, verb := range requiredVerbs {
60-
resAttrs = append(resAttrs, authorizationv1.ResourceAttributes{
108+
resAttrs = append(resAttrs, authv1.ResourceAttributes{
61109
Namespace: o.GetNamespace(),
62110
Verb: verb,
63111
Resource: sanitizeResourceName(o.GetObjectKind().GroupVersionKind().Kind),
@@ -70,7 +118,7 @@ func CheckObjectPermissions(ctx context.Context, authcl authorizationv1client.Au
70118
for _, resAttr := range resAttrs {
71119
if !canI(resAttr, rules) {
72120
if resAttr.Namespace != "" {
73-
namespacedErrs = append(namespacedErrs, fmt.Errorf("cannot %s %s %s in namespace %s",
121+
namespacedErrs = append(namespacedErrs, fmt.Errorf("cannot %q %q %q in namespace %q",
74122
resAttr.Verb,
75123
strings.TrimSuffix(resAttr.Resource, "s"),
76124
resAttr.Name,
@@ -92,7 +140,7 @@ func CheckObjectPermissions(ctx context.Context, authcl authorizationv1client.Au
92140
}
93141

94142
// Checks if the rules allow the verb on the GroupVersionKind in resAttr
95-
func canI(resAttr authorizationv1.ResourceAttributes, rules []rbacv1.PolicyRule) bool {
143+
func canI(resAttr authv1.ResourceAttributes, rules []rbacv1.PolicyRule) bool {
96144
var canI bool
97145
for _, rule := range rules {
98146
if (slices.Contains(rule.APIGroups, resAttr.Group) || slices.Contains(rule.APIGroups, "*")) &&

0 commit comments

Comments
 (0)