Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit cbe1f67

Browse files
committedFeb 24, 2025··
refactoring (and more changes in the copied kubernetes code)
1 parent 49d899e commit cbe1f67

28 files changed

+458
-483
lines changed
 

‎.golangci.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ linters:
3838
- unused
3939
- whitespace
4040

41+
issues:
42+
exclude-dirs:
43+
- internal/operator-controller/authorization/internal/kubernetes
44+
4145
linters-settings:
4246
gci:
4347
sections:

‎Makefile

-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ manifests: $(CONTROLLER_GEN) #EXHELP Generate WebhookConfiguration, ClusterRole,
136136
.PHONY: generate
137137
generate: $(CONTROLLER_GEN) #EXHELP Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
138138
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./api/..."
139-
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./catalogd/api/..."
140139

141140
.PHONY: verify
142141
verify: tidy fmt generate manifests crd-ref-docs #HELP Verify all generated code is up-to-date.

‎api/v1/clusterextension_types.go

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1
1818

1919
import (
20+
rbacv1 "k8s.io/api/rbac/v1"
2021
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2122
)
2223

@@ -454,6 +455,8 @@ type ClusterExtensionStatus struct {
454455
//
455456
// +optional
456457
Install *ClusterExtensionInstallStatus `json:"install,omitempty"`
458+
459+
MissingRules map[string][]rbacv1.PolicyRule `json:"missingRules,omitempty"`
457460
}
458461

459462
// ClusterExtensionInstallStatus is a representation of the status of the identified bundle.

‎api/v1/zz_generated.deepcopy.go

+19
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cmd/operator-controller/main.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -415,9 +415,7 @@ func run() error {
415415
helmApplier := &applier.Helm{
416416
ActionClientGetter: acg,
417417
Preflights: preflights,
418-
Authorizer: authorization.NewRBACAuthorizer(mgr.GetClient()),
419-
RuleResolver: authorization.NewRBACRulesResolver(mgr.GetClient()),
420-
RestMapper: mgr.GetRESTMapper(),
418+
PreAuthorizer: authorization.NewRBACPreAuthorizer(mgr.GetClient()),
421419
}
422420

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

‎config/base/operator-controller/crd/bases/olm.operatorframework.io_clusterextensions.yaml

+52
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,58 @@ spec:
581581
required:
582582
- bundle
583583
type: object
584+
missingRules:
585+
additionalProperties:
586+
items:
587+
description: |-
588+
PolicyRule holds information that describes a policy rule, but does not contain information
589+
about who the rule applies to or which namespace the rule applies to.
590+
properties:
591+
apiGroups:
592+
description: |-
593+
APIGroups is the name of the APIGroup that contains the resources. If multiple API groups are specified, any action requested against one of
594+
the enumerated resources in any API group will be allowed. "" represents the core API group and "*" represents all API groups.
595+
items:
596+
type: string
597+
type: array
598+
x-kubernetes-list-type: atomic
599+
nonResourceURLs:
600+
description: |-
601+
NonResourceURLs is a set of partial urls that a user should have access to. *s are allowed, but only as the full, final step in the path
602+
Since non-resource URLs are not namespaced, this field is only applicable for ClusterRoles referenced from a ClusterRoleBinding.
603+
Rules can either apply to API resources (such as "pods" or "secrets") or non-resource URL paths (such as "/api"), but not both.
604+
items:
605+
type: string
606+
type: array
607+
x-kubernetes-list-type: atomic
608+
resourceNames:
609+
description: ResourceNames is an optional white list of names
610+
that the rule applies to. An empty set means that everything
611+
is allowed.
612+
items:
613+
type: string
614+
type: array
615+
x-kubernetes-list-type: atomic
616+
resources:
617+
description: Resources is a list of resources this rule applies
618+
to. '*' represents all resources.
619+
items:
620+
type: string
621+
type: array
622+
x-kubernetes-list-type: atomic
623+
verbs:
624+
description: Verbs is a list of Verbs that apply to ALL the
625+
ResourceKinds contained in this rule. '*' represents all
626+
verbs.
627+
items:
628+
type: string
629+
type: array
630+
x-kubernetes-list-type: atomic
631+
required:
632+
- verbs
633+
type: object
634+
type: array
635+
type: object
584636
type: object
585637
type: object
586638
served: true

‎config/samples/crb.yaml

-1
This file was deleted.

‎config/samples/crb2.yaml

-24
This file was deleted.

‎config/samples/olm_v1_clusterextension.yaml

-2
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,6 @@ apiVersion: olm.operatorframework.io/v1
276276
kind: ClusterExtension
277277
metadata:
278278
name: argocd
279-
annotations:
280-
rev: "1"
281279
spec:
282280
namespace: argocd
283281
serviceAccount:

‎config/samples/xx_olm_v1_clusterextension.yaml

-103
This file was deleted.

‎docs/api-reference/operator-controller-api-reference.md

+1
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ _Appears in:_
326326
| --- | --- | --- | --- |
327327
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.<br /><br />The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.<br />When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />When Installed is False and the Reason is Failed, the bundle has failed to install.<br /><br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br /><br />When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.<br />ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.<br />PackageDeprecated is set if the requested package is marked deprecated in the catalog.<br />Deprecated is a rollup condition that is present when any of the deprecated conditions are present. | | |
328328
| `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | |
329+
| `missingRules` _object (keys:string, values:[PolicyRule](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#policyrule-v1-rbac))_ | | | |
329330

330331

331332
#### ImageSource

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

+23-207
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"io"
99
"io/fs"
10-
"maps"
1110
"strings"
1211

1312
"helm.sh/helm/v3/pkg/action"
@@ -17,27 +16,20 @@ import (
1716
"helm.sh/helm/v3/pkg/release"
1817
"helm.sh/helm/v3/pkg/storage/driver"
1918
corev1 "k8s.io/api/core/v1"
20-
rbacv1 "k8s.io/api/rbac/v1"
21-
"k8s.io/apimachinery/pkg/api/meta"
2219
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
23-
"k8s.io/apimachinery/pkg/runtime"
24-
"k8s.io/apimachinery/pkg/runtime/schema"
25-
"k8s.io/apimachinery/pkg/util/sets"
2620
apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
2721
"k8s.io/apiserver/pkg/authentication/user"
28-
"k8s.io/apiserver/pkg/authorization/authorizer"
2922
"sigs.k8s.io/controller-runtime/pkg/client"
3023
"sigs.k8s.io/controller-runtime/pkg/log"
3124

3225
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
3326

3427
ocv1 "github.com/operator-framework/operator-controller/api/v1"
3528
"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
29+
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
3630
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
3731
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
3832
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
39-
"github.com/operator-framework/operator-controller/internal/shared/util/filter"
40-
"github.com/operator-framework/operator-controller/kubernetes/pkg/registry/rbac/validation"
4133
)
4234

4335
const (
@@ -66,9 +58,7 @@ type Preflight interface {
6658
type Helm struct {
6759
ActionClientGetter helmclient.ActionClientGetter
6860
Preflights []Preflight
69-
Authorizer authorizer.Authorizer
70-
RuleResolver validation.AuthorizationRuleResolver
71-
RestMapper meta.RESTMapper
61+
PreAuthorizer authorization.PreAuthorizer
7262
}
7363

7464
// shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
@@ -92,51 +82,41 @@ func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.Clu
9282
}
9383

9484
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) {
95-
l := log.FromContext(ctx)
9685
chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{corev1.NamespaceAll})
9786
if err != nil {
9887
return nil, "", err
9988
}
10089
values := chartutil.Values{}
10190

102-
ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext)
103-
if err != nil {
104-
return nil, "", err
105-
}
106-
10791
post := &postrenderer{
10892
labels: objectLabels,
10993
}
11094

111-
tmplRel, err := h.template(ctx, ext, chrt, values, post)
112-
if err != nil {
113-
return nil, "", fmt.Errorf("failed to get release state using client-only dry-run: %w", err)
114-
}
95+
if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) {
96+
tmplRel, err := h.template(ctx, ext, chrt, values, post)
97+
if err != nil {
98+
return nil, "", fmt.Errorf("failed to get release state using client-only dry-run: %w", err)
99+
}
115100

116-
collectionVerbs := []string{"list", "watch", "create"}
117-
objectVerbs := []string{"get", "patch", "update", "delete"}
118-
authzResults, escalationErrs, err := h.authorizeManifest(ctx, ext, strings.NewReader(tmplRel.Manifest), collectionVerbs, objectVerbs)
119-
if err != nil {
120-
return nil, "", fmt.Errorf("failed to authorize manifest: %w", err)
121-
}
122-
for _, r := range authzResults {
123-
if !isAuthzResultAllowed(r) {
124-
l.Info("authz result",
125-
"attrs", r.Attrs,
126-
"denied", r.Denied,
127-
"error", r.Err,
128-
)
101+
ceServiceAccount := user.DefaultInfo{Name: fmt.Sprintf("system:serviceaccount:%s:%s", ext.Spec.Namespace, ext.Spec.ServiceAccount.Name)}
102+
missingRules, err := h.PreAuthorizer.PreAuthorize(ctx, &ceServiceAccount, strings.NewReader(tmplRel.Manifest))
103+
104+
var preAuthErrors []error
105+
if len(missingRules) > 0 {
106+
ext.Status.MissingRules = missingRules
107+
preAuthErrors = append(preAuthErrors, errors.New("service account lacks permission to manage cluster extension"))
129108
}
130-
}
131-
for _, err := range escalationErrs {
132109
if err != nil {
133-
l.Info("escalation result failure",
134-
"error", err,
135-
)
110+
preAuthErrors = append(preAuthErrors, fmt.Errorf("authorization evaluation error: %w", err))
111+
}
112+
if len(preAuthErrors) > 0 {
113+
return nil, "", fmt.Errorf("pre-authorization failed: %v", preAuthErrors)
136114
}
137115
}
138-
if err := processAuthzResults(ctx, authzResults, escalationErrs); err != nil {
139-
return nil, "", fmt.Errorf("service account lacks the necessary permissions: %w", err)
116+
117+
ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext)
118+
if err != nil {
119+
return nil, "", err
140120
}
141121

142122
rel, desiredRel, state, err := h.getReleaseState(ac, ext, chrt, values, post)
@@ -164,7 +144,6 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
164144

165145
switch state {
166146
case StateNeedsInstall:
167-
l.Info("installing extension")
168147
rel, err = ac.Install(ext.GetName(), ext.Spec.Namespace, chrt, values, func(install *action.Install) error {
169148
install.CreateNamespace = false
170149
install.Labels = storageLabels
@@ -173,9 +152,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
173152
if err != nil {
174153
return nil, state, err
175154
}
176-
l.Info("install succeeded", "revision", rel.Version, "status", rel.Info.Status)
177155
case StateNeedsUpgrade:
178-
l.Info("upgrading extension")
179156
rel, err = ac.Upgrade(ext.GetName(), ext.Spec.Namespace, chrt, values, func(upgrade *action.Upgrade) error {
180157
upgrade.MaxHistory = maxHelmReleaseHistory
181158
upgrade.Labels = storageLabels
@@ -184,13 +161,10 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
184161
if err != nil {
185162
return nil, state, err
186163
}
187-
l.Info("upgrade succeeded", "revision", rel.Version, "status", rel.Info.Status)
188164
case StateUnchanged:
189-
l.Info("reconciling extension")
190165
if err := ac.Reconcile(rel); err != nil {
191166
return nil, state, err
192167
}
193-
l.Info("reconcile succeeded", "revision", rel.Version, "status", rel.Info.Status)
194168
default:
195169
return nil, state, fmt.Errorf("unexpected release state %q", state)
196170
}
@@ -240,7 +214,7 @@ func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterE
240214
return nil
241215
}, helmclient.AppendInstallPostRenderer(post))
242216
if err != nil {
243-
return nil, nil, StateError, fmt.Errorf("dry-run install failed: %w", err)
217+
return nil, nil, StateError, err
244218
}
245219
return nil, desiredRelease, StateNeedsInstall, nil
246220
}
@@ -295,161 +269,3 @@ func (p *postrenderer) Run(renderedManifests *bytes.Buffer) (*bytes.Buffer, erro
295269
}
296270
return &buf, nil
297271
}
298-
299-
type authorizationResult struct {
300-
Attrs authorizer.AttributesRecord
301-
Denied bool
302-
Reason string
303-
Err error
304-
}
305-
306-
func (h *Helm) authorizeManifest(ctx context.Context, ext *ocv1.ClusterExtension, manifestReader io.Reader, collectionVerbs, objectVerbs []string) ([]authorizationResult, []error, error) {
307-
var (
308-
user = user.DefaultInfo{Name: fmt.Sprintf("system:serviceaccount:%s:%s", ext.Spec.Namespace, ext.Spec.ServiceAccount.Name)}
309-
310-
authorizationResults []authorizationResult
311-
escalationErrs []error
312-
groupVersionResources = map[schema.GroupVersionResource][]client.ObjectKey{}
313-
314-
clusterRoles = map[client.ObjectKey]rbacv1.ClusterRole{}
315-
roles = map[client.ObjectKey]rbacv1.Role{}
316-
clusterRoleBindings = map[client.ObjectKey]rbacv1.ClusterRoleBinding{}
317-
roleBindings = map[client.ObjectKey]rbacv1.RoleBinding{}
318-
)
319-
320-
dec := apimachyaml.NewYAMLOrJSONDecoder(manifestReader, 1024)
321-
for {
322-
var uObj unstructured.Unstructured
323-
err := dec.Decode(&uObj)
324-
if errors.Is(err, io.EOF) {
325-
break
326-
}
327-
if err != nil {
328-
return nil, nil, err
329-
}
330-
gvk := uObj.GroupVersionKind()
331-
restMapping, err := h.RestMapper.RESTMapping(gvk.GroupKind(), gvk.Version)
332-
if err != nil {
333-
return nil, nil, err
334-
}
335-
336-
gvr := restMapping.Resource
337-
groupVersionResources[gvr] = append(groupVersionResources[gvr], client.ObjectKeyFromObject(&uObj))
338-
339-
switch restMapping.Resource.GroupResource() {
340-
case schema.GroupResource{Group: rbacv1.GroupName, Resource: "clusterroles"}:
341-
obj := &rbacv1.ClusterRole{}
342-
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil {
343-
return nil, nil, err
344-
}
345-
clusterRoles[client.ObjectKeyFromObject(obj)] = *obj
346-
case schema.GroupResource{Group: rbacv1.GroupName, Resource: "clusterrolebindings"}:
347-
obj := &rbacv1.ClusterRoleBinding{}
348-
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil {
349-
return nil, nil, err
350-
}
351-
clusterRoleBindings[client.ObjectKeyFromObject(obj)] = *obj
352-
case schema.GroupResource{Group: rbacv1.GroupName, Resource: "roles"}:
353-
obj := &rbacv1.Role{}
354-
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil {
355-
return nil, nil, err
356-
}
357-
roles[client.ObjectKeyFromObject(obj)] = *obj
358-
case schema.GroupResource{Group: rbacv1.GroupName, Resource: "rolebindings"}:
359-
obj := &rbacv1.RoleBinding{}
360-
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(uObj.UnstructuredContent(), obj); err != nil {
361-
return nil, nil, err
362-
}
363-
roleBindings[client.ObjectKeyFromObject(obj)] = *obj
364-
}
365-
}
366-
367-
var attributeRecords []authorizer.AttributesRecord
368-
for gvr, keys := range groupVersionResources {
369-
namespaces := sets.New[string]()
370-
for _, k := range keys {
371-
namespaces.Insert(k.Namespace)
372-
for _, v := range objectVerbs {
373-
attributeRecords = append(attributeRecords, authorizer.AttributesRecord{
374-
User: &user,
375-
Namespace: k.Namespace,
376-
Name: k.Name,
377-
APIGroup: gvr.Group,
378-
APIVersion: gvr.Version,
379-
Resource: gvr.Resource,
380-
ResourceRequest: true,
381-
Verb: v,
382-
})
383-
}
384-
}
385-
for _, ns := range sets.List(namespaces) {
386-
for _, v := range collectionVerbs {
387-
attributeRecords = append(attributeRecords, authorizer.AttributesRecord{
388-
User: &user,
389-
Namespace: ns,
390-
Name: "",
391-
APIGroup: gvr.Group,
392-
APIVersion: gvr.Version,
393-
Resource: gvr.Resource,
394-
ResourceRequest: true,
395-
Verb: v,
396-
})
397-
}
398-
}
399-
}
400-
401-
for _, attributeRecord := range attributeRecords {
402-
decision, reason, err := h.Authorizer.Authorize(ctx, attributeRecord)
403-
authorizationResults = append(authorizationResults, authorizationResult{
404-
Attrs: attributeRecord,
405-
Denied: decision == authorizer.DecisionDeny,
406-
Reason: reason,
407-
Err: err,
408-
})
409-
}
410-
411-
escalationChecker := authorization.EscalateChecker{
412-
Authorizer: h.Authorizer,
413-
AuthorizationRuleResolver: h.RuleResolver,
414-
ExtraClusterRoles: clusterRoles,
415-
ExtraRoles: roles,
416-
}
417-
418-
for obj := range maps.Values(clusterRoles) {
419-
escalationErrs = append(escalationErrs, escalationChecker.CheckEscalation(ctx, user, &obj))
420-
}
421-
for obj := range maps.Values(roles) {
422-
escalationErrs = append(escalationErrs, escalationChecker.CheckEscalation(ctx, user, &obj))
423-
}
424-
for obj := range maps.Values(clusterRoleBindings) {
425-
escalationErrs = append(escalationErrs, escalationChecker.CheckEscalation(ctx, user, &obj))
426-
}
427-
for obj := range maps.Values(roleBindings) {
428-
escalationErrs = append(escalationErrs, escalationChecker.CheckEscalation(ctx, user, &obj))
429-
}
430-
return authorizationResults, escalationErrs, nil
431-
}
432-
433-
func isAuthzResultAllowed(authzResult authorizationResult) bool {
434-
return authzResult.Err == nil && !authzResult.Denied
435-
}
436-
437-
func processAuthzResults(ctx context.Context, authzResults []authorizationResult, escalationErrs []error) error {
438-
unauthorizedAuthzResults := filter.Filter(authzResults, func(authzResult authorizationResult) bool {
439-
return !isAuthzResultAllowed(authzResult)
440-
})
441-
unauthorizedEscalations := filter.Filter(escalationErrs, func(escalationErr error) bool {
442-
return escalationErr != nil
443-
})
444-
445-
var errs []error
446-
if len(unauthorizedAuthzResults) > 0 {
447-
log.FromContext(ctx).Info("unauthorized authzs", "deniedCount", len(unauthorizedAuthzResults), "totalCount", len(authzResults))
448-
errs = append(errs, errors.New("unauthorized authzs"))
449-
}
450-
if len(unauthorizedEscalations) > 0 {
451-
log.FromContext(ctx).Info("unauthorized escalation", "deniedCount", len(unauthorizedEscalations), "totalCount", len(escalationErrs))
452-
errs = append(errs, errors.New("unauthorized escalations"))
453-
}
454-
return errors.Join(errs...)
455-
}

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

-67
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,12 @@ import (
1313
"helm.sh/helm/v3/pkg/chart"
1414
"helm.sh/helm/v3/pkg/release"
1515
"helm.sh/helm/v3/pkg/storage/driver"
16-
featuregatetesting "k8s.io/component-base/featuregate/testing"
1716
"sigs.k8s.io/controller-runtime/pkg/client"
1817

1918
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
2019

2120
ocv1 "github.com/operator-framework/operator-controller/api/v1"
2221
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
23-
"github.com/operator-framework/operator-controller/internal/operator-controller/features"
2422
)
2523

2624
type mockPreflight struct {
@@ -228,71 +226,6 @@ func TestApply_Installation(t *testing.T) {
228226
})
229227
}
230228

231-
func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
232-
featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.PreflightPermissions, true)
233-
234-
t.Run("fails during dry-run installation", func(t *testing.T) {
235-
mockAcg := &mockActionGetter{
236-
getClientErr: driver.ErrReleaseNotFound,
237-
dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
238-
}
239-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
240-
241-
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
242-
require.Error(t, err)
243-
require.ErrorContains(t, err, "attempting to dry-run install chart")
244-
require.Nil(t, objs)
245-
require.Empty(t, state)
246-
})
247-
248-
t.Run("fails during pre-flight installation", func(t *testing.T) {
249-
mockAcg := &mockActionGetter{
250-
getClientErr: driver.ErrReleaseNotFound,
251-
installErr: errors.New("failed installing chart"),
252-
}
253-
mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
254-
helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
255-
256-
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
257-
require.Error(t, err)
258-
require.ErrorContains(t, err, "install pre-flight check")
259-
require.Equal(t, applier.StateNeedsInstall, state)
260-
require.Nil(t, objs)
261-
})
262-
263-
t.Run("fails during installation", func(t *testing.T) {
264-
mockAcg := &mockActionGetter{
265-
getClientErr: driver.ErrReleaseNotFound,
266-
installErr: errors.New("failed installing chart"),
267-
}
268-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
269-
270-
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
271-
require.Error(t, err)
272-
require.ErrorContains(t, err, "installing chart")
273-
require.Equal(t, applier.StateNeedsInstall, state)
274-
require.Nil(t, objs)
275-
})
276-
277-
t.Run("successful installation", func(t *testing.T) {
278-
mockAcg := &mockActionGetter{
279-
getClientErr: driver.ErrReleaseNotFound,
280-
desiredRel: &release.Release{
281-
Info: &release.Info{Status: release.StatusDeployed},
282-
Manifest: validManifest,
283-
},
284-
}
285-
helmApplier := applier.Helm{ActionClientGetter: mockAcg}
286-
287-
objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
288-
require.NoError(t, err)
289-
require.Equal(t, applier.StateNeedsInstall, state)
290-
require.NotNil(t, objs)
291-
assert.Equal(t, "service-a", objs[0].GetName())
292-
assert.Equal(t, "service-b", objs[1].GetName())
293-
})
294-
}
295-
296229
func TestApply_Upgrade(t *testing.T) {
297230
testCurrentRelease := &release.Release{
298231
Info: &release.Info{Status: release.StatusDeployed},

‎kubernetes/pkg/apis/rbac/v1/zz_generated.conversion.go ‎internal/operator-controller/authorization/internal/kubernetes/pkg/apis/rbac/v1/zz_generated.conversion.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎kubernetes/pkg/registry/rbac/escalation_check.go ‎internal/operator-controller/authorization/internal/kubernetes/pkg/registry/rbac/escalation_check.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,13 @@ package rbac
1919
import (
2020
"context"
2121
"fmt"
22+
rbac2 "github.com/operator-framework/operator-controller/internal/operator-controller/authorization/internal/kubernetes/pkg/apis/rbac"
2223

2324
"k8s.io/apimachinery/pkg/runtime/schema"
2425
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2526
"k8s.io/apiserver/pkg/authentication/user"
2627
"k8s.io/apiserver/pkg/authorization/authorizer"
2728
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
28-
29-
"github.com/operator-framework/operator-controller/kubernetes/pkg/apis/rbac"
3029
)
3130

3231
// EscalationAllowed checks if the user associated with the context is a superuser
@@ -48,8 +47,8 @@ func EscalationAllowed(ctx context.Context) bool {
4847
}
4948

5049
var roleResources = map[schema.GroupResource]bool{
51-
rbac.SchemeGroupVersion.WithResource("clusterroles").GroupResource(): true,
52-
rbac.SchemeGroupVersion.WithResource("roles").GroupResource(): true,
50+
rbac2.SchemeGroupVersion.WithResource("clusterroles").GroupResource(): true,
51+
rbac2.SchemeGroupVersion.WithResource("roles").GroupResource(): true,
5352
}
5453

5554
// RoleEscalationAuthorized checks if the user associated with the context is explicitly authorized to escalate the role resource associated with the context
@@ -99,7 +98,7 @@ func RoleEscalationAuthorized(ctx context.Context, a authorizer.Authorizer) bool
9998
}
10099

101100
// BindingAuthorized returns true if the user associated with the context is explicitly authorized to bind the specified roleRef
102-
func BindingAuthorized(ctx context.Context, roleRef rbac.RoleRef, bindingNamespace string, a authorizer.Authorizer) bool {
101+
func BindingAuthorized(ctx context.Context, roleRef rbac2.RoleRef, bindingNamespace string, a authorizer.Authorizer) bool {
103102
if a == nil {
104103
return false
105104
}

‎kubernetes/pkg/registry/rbac/rbac.go ‎internal/operator-controller/authorization/internal/kubernetes/pkg/registry/rbac/rbac.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ import (
2121
"bytes"
2222
"context"
2323
"fmt"
24+
rbacv1helpers "github.com/operator-framework/operator-controller/internal/operator-controller/authorization/internal/kubernetes/pkg/apis/rbac/v1"
25+
rbacregistryvalidation "github.com/operator-framework/operator-controller/internal/operator-controller/authorization/internal/kubernetes/pkg/registry/rbac/validation"
2426

2527
"k8s.io/klog/v2"
2628

27-
rbacv1helpers "github.com/operator-framework/operator-controller/kubernetes/pkg/apis/rbac/v1"
28-
rbacregistryvalidation "github.com/operator-framework/operator-controller/kubernetes/pkg/registry/rbac/validation"
2929
rbacv1 "k8s.io/api/rbac/v1"
3030
"k8s.io/apimachinery/pkg/labels"
3131
utilerrors "k8s.io/apimachinery/pkg/util/errors"

‎kubernetes/pkg/registry/rbac/validation/policy_compact.go ‎internal/operator-controller/authorization/internal/kubernetes/pkg/registry/rbac/validation/policy_compact.go

+8
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"reflect"
2121

2222
rbacv1 "k8s.io/api/rbac/v1"
23+
"k8s.io/apimachinery/pkg/util/sets"
2324
)
2425

2526
type simpleResource struct {
@@ -54,6 +55,13 @@ func CompactRules(rules []rbacv1.PolicyRule) ([]rbacv1.PolicyRule, error) {
5455

5556
// Once we've consolidated the simple resource rules, add them to the compacted list
5657
for _, simpleRule := range simpleRules {
58+
verbSet := sets.New[string](simpleRule.Verbs...)
59+
if verbSet.Has("*") {
60+
simpleRule.Verbs = []string{"*"}
61+
} else {
62+
simpleRule.Verbs = sets.List(verbSet)
63+
}
64+
5765
compacted = append(compacted, *simpleRule)
5866
}
5967

‎kubernetes/pkg/registry/rbac/validation/rule.go ‎internal/operator-controller/authorization/internal/kubernetes/pkg/registry/rbac/validation/rule.go

+27-12
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
rbacv1helpers "github.com/operator-framework/operator-controller/internal/operator-controller/authorization/internal/kubernetes/pkg/apis/rbac/v1"
2324
"strings"
2425

2526
rbacv1 "k8s.io/api/rbac/v1"
@@ -30,8 +31,6 @@ import (
3031
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
3132
"k8s.io/component-helpers/auth/rbac/validation"
3233
"k8s.io/klog/v2"
33-
34-
rbacv1helpers "github.com/operator-framework/operator-controller/kubernetes/pkg/apis/rbac/v1"
3534
)
3635

3736
type AuthorizationRuleResolver interface {
@@ -49,6 +48,27 @@ type AuthorizationRuleResolver interface {
4948
VisitRulesFor(ctx context.Context, user user.Info, namespace string, visitor func(source fmt.Stringer, rule *rbacv1.PolicyRule, err error) bool)
5049
}
5150

51+
type PrivilegeEscalationError struct {
52+
User user.Info
53+
Namespace string
54+
MissingRules []rbacv1.PolicyRule
55+
RuleResolutionErrors []error
56+
}
57+
58+
func (e *PrivilegeEscalationError) Error() string {
59+
missingDescriptions := sets.NewString()
60+
for _, missing := range e.MissingRules {
61+
missingDescriptions.Insert(rbacv1helpers.CompactString(missing))
62+
}
63+
64+
msg := fmt.Sprintf("user %q (groups=%q) is attempting to grant RBAC permissions not currently held:\n%s", e.User.GetName(), e.User.GetGroups(), strings.Join(missingDescriptions.List(), "\n"))
65+
if len(e.RuleResolutionErrors) > 0 {
66+
msg = msg + fmt.Sprintf("; resolution errors: %v", e.RuleResolutionErrors)
67+
}
68+
69+
return msg
70+
}
71+
5272
// ConfirmNoEscalation determines if the roles for a given user in a given namespace encompass the provided role.
5373
func ConfirmNoEscalation(ctx context.Context, ruleResolver AuthorizationRuleResolver, rules []rbacv1.PolicyRule) error {
5474
ruleResolutionErrors := []error{}
@@ -73,17 +93,12 @@ func ConfirmNoEscalation(ctx context.Context, ruleResolver AuthorizationRuleReso
7393
compactMissingRights = compact
7494
}
7595

76-
missingDescriptions := sets.NewString()
77-
for _, missing := range compactMissingRights {
78-
missingDescriptions.Insert(rbacv1helpers.CompactString(missing))
96+
return &PrivilegeEscalationError{
97+
User: user,
98+
Namespace: namespace,
99+
MissingRules: compactMissingRights,
100+
RuleResolutionErrors: ruleResolutionErrors,
79101
}
80-
81-
msg := fmt.Sprintf("user %q (groups=%q) is attempting to grant RBAC permissions not currently held:\n%s", user.GetName(), user.GetGroups(), strings.Join(missingDescriptions.List(), "\n"))
82-
if len(ruleResolutionErrors) > 0 {
83-
msg = msg + fmt.Sprintf("; resolution errors: %v", ruleResolutionErrors)
84-
}
85-
86-
return errors.New(msg)
87102
}
88103
return nil
89104
}

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

+313-55
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)
Please sign in to comment.