Introduce RBAC Toggle Enhancement

awgreene · awgreene · commit 81d7ec2d7049 · 2022-06-07T16:56:37.000-04:00
diff --git a/enhancements/rbac-toggle.md b/enhancements/rbac-toggle.md
@@ -0,0 +1,235 @@
+---
+title: rbac-toggle
+authors:
+  - "@awgreene"
+reviewers:
+  - TBD
+approvers:
+  - TBD
+creation-date: 2023-06-06
+last-updated: 2023-06-06
+status: implementable
+see-also:
+  - NA
+replaces:
+  - NA
+superseded-by:
+  - NA
+---
+
+# RBAC Toggle
+
+## Release Signoff Checklist
+
+- [ ] Enhancement is `implementable`
+- [ ] Design details are appropriately documented from clear requirements
+- [ ] Test plan is defined
+- [ ] Graduation criteria for dev preview, tech preview, GA
+
+## Open Questions [optional]
+
+NA
+
+## Summary
+
+The [operator-lifecycle-manager (OLM)](https://github.com/operator-framework/operator-lifecycle-manager) is responsible for installing, upgrading, and managing the lifecycle of operators. While managing the lifecycle of an operator, OLM creates the [Role-Based Access Control (RBAC)](https://kubernetes.io/docs/reference/access-authn-authz/rbac/) manifests requested by the operator in namespaces that the operator is [scoped](https://olm.operatorframework.io/docs/advanced-tasks/operator-scoping-with-operatorgroups/) to. When an operator is installed at the cluster scoped level, the requested RBAC is elevated to cluster wide levels.
+
+Recently, the Operator Framework has been advising operator authors to develope cluster scoped operators as:
+- The existing [CustomResourceDefinition (CRD)](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/) primitive does not provide a great user experience on multitenant environments.
+- It becomes increasingly complex when a user installs multiple versions of an operator scoped to different namespaces.
+
+As more operators move towards the cluster scoped model, it becomes increasingly problematic for cluster admins that are uncomfortable or unable to allow OLM to grant cluster wide RBAC to operators it installs.
+
+This enhancement proposes providing cluster admins greater control over the RBAC generated by OLM while managing an operator.
+
+## Motivation
+
+The primary motivation of this enhancement centers around using OLM on highly regulated clusters. Within these highly regulated environments, cluster admins are responsible for tightly limiting the permissions of users or services in an effort to adhere to specific regulations that are regularly audited. Today, it is difficult or impossible to use OLM in these environments as the cluster admins have no control over the RBAC generated by OLM.
+
+### Goals
+
+Allow cluster admins to disable:
+
+- The promotion of Roles and RoleBindings defined from the [ClusterServiceVersion.Spec.Install.Spec.Permissions field](https://github.com/operator-framework/api/blob/33310d6154f344da5494c788451bd34cdf54711c/pkg/operators/v1alpha1/clusterserviceversion_types.go#L79) to ClusterRoles and ClusterRoleBindings respectively when installing an operators into a namespace that includes an `AllNamespace` scoped OperatorGroup.
+- The generation of ClusterRoles aggregated to the view, edit, and admin ClusterRoles when installing a CRD. For example, when installing a CRD named foo.bar.io at v1, OLM should not generate the following ClusterRoles:
+-- foo.bar.io-v1-admin
+-- foo.bar.io-v1-crdview
+-- foo.bar.io-v1-edit
+-- foo.bar.io-v1-view
+
+> NOTE: The following Goals may not be required, but it seems reasonable that a cluster admin would not want every user to be able to view/edit CRDs introduced by OLM. This should be decided upon prior to merging the EP.
+
+- The generation of [aggregate-olm-edit ClusterRole](https://github.com/operator-framework/operator-lifecycle-manager/blob/943a726ab1a516bc231e2fe96d13fc2e47bf4448/deploy/upstream/quickstart/olm.yaml#L153-L167) RBAC.
+- The generation of [aggregate-olm-view ClusterRole](https://github.com/operator-framework/operator-lifecycle-manager/blob/943a726ab1a516bc231e2fe96d13fc2e47bf4448/deploy/upstream/quickstart/olm.yaml#L168-L184) RBAC.
+
+### Non-Goals
+
+- Provide means to disable generation of ClusterRoles and ClusterRoleBindings derived from the [ClusterServiceVersion.Spec.Install.Spec.ClusterPermissions field](https://github.com/operator-framework/api/blob/33310d6154f344da5494c788451bd34cdf54711c/pkg/operators/v1alpha1/clusterserviceversion_types.go#L80). These cluster scoped permissions are required for the operator to run so OLM cannot remove them.
+
+## Proposal 
+
+OLM recently introduced the [olmConfig resource](https://github.com/operator-framework/api/blob/33310d6154f344da5494c788451bd34cdf54711c/pkg/operators/v1/olmconfig_types.go#L40-L47) to provide users with the means to configure OLM behavior. This enhancement proposes three additional toggles to the [spec.features struct](https://github.com/operator-framework/api/blob/33310d6154f344da5494c788451bd34cdf54711c/pkg/operators/v1/olmconfig_types.go#L16-L26):
+```yaml
+// Features contains the list of configurable OLM features.
+type Features struct {
+
+  // DisableOLMAggregateCRDRoles allows users to disable the aggregate-olm-edit and aggregate-olm-view clusterroles that ship with OLM.
+  //
+  // Again, the jury is still out on whether or not this field is needed.
+	DisableOLMAggregateCRDRoles *bool `json:"disableOLMAggregateCRDRoles,omitempty"`
+
+	// DisableAggregateCRDRoles is used to prevent OLM from generating
+  // clusterRoles for CRDs it installs that are aggregated to the default
+  // view, edit, and admins roles that are shipped with Kubernetes. OLM
+  // creates these roles so users may immediately begin interacting with APIs
+  // introduced by operators installed with OLM.
+	DisableAggregateCRDRoles *bool `json:"disableAggregateCRDRoles,omitempty"`
+
+  // DisableAllNamespacePermissionPromotion prevents OLM from escalating
+  // roles and roleBindings requested by an operator to clusterRole and
+  // clusterRoleBindings respectively when OLM installs an operators into
+  // a namespace that includes an operatorGroup with the AllNamespace
+  // installMode.
+	DisableAllNamespacePermissionPromotion *bool `json:"disableAllNamespacePermissionPromotion,omitempty"`
+}
+```
+
+Let's discuss the design of each of these toggles:
+
+### DisableOLMAggregateCRDRoles Toggle
+
+The goal of this toggle is to provide cluster admins with the means to prevent OLM from creating aggregated `edit` and `view` roles for the CRDs and APIServices introduced by OLM.
+
+To support this toggle, the `aggregate-olm-edit` and `aggregate-olm-view` will be removed from the manifests directory and instead the [olmConfig controller](https://github.com/operator-framework/operator-lifecycle-manager/blob/3002cf79ce867e32d30d2190df642eaa7f449294/pkg/controller/operators/olm/operator.go#L1249-L1314) will be updated to inspect the `olmConfig.Spec.DisableOLMAggregateCRDRoles` field and:
+- Ensure that the `aggregate-olm-edit` and `aggregate-olm-view` clusterRoles exist if the `DisableOLMAggregateCRDRoles` field in  is `nil` or `false`.
+- Ensure that the `aggregate-olm-edit` and `aggregate-olm-view` clusterRoles do not exist if the field is `true`.
+
+### DisableAggregateCRDRoles Toggle
+
+The goal of this toggle is to provide cluster admins with the means to prevent OLM from creating aggregated `edit`, `view`, `crdview,` and `admin` for CRDs and APIServices introduced by managed operators.
+
+To support this toggle, the [ensureClusterRolesForCRD](https://github.com/operator-framework/operator-lifecycle-manager/blob/3002cf79ce867e32d30d2190df642eaa7f449294/pkg/controller/operators/olm/operatorgroup.go#L418-L457) function should be updated to inspect the `olmConfig.Spec.DisableAggregateCRDRoles` field and:
+- Ensure that the `edit`, `view`, `crdview,` and `admin` roleBindings exist if the field is `nil` or `false`.
+- Ensure that the `edit`, `view`, `crdview`, and `admin` roleBindings do not exist if the field is `true`.
+
+### DisableAllNamespacePermissionPromotion Toggle
+
+The goal of this toggle is to provide cluster admins with the means to prevent OLM from promotion roles and roleBindings defined in a ClusterServiceVersion (CSV) to clusterRoles and clusterRoleBindings.
+
+To support this toggle, the [ensureRBACInTargetNamespace](https://github.com/operator-framework/operator-lifecycle-manager/blob/3002cf79ce867e32d30d2190df642eaa7f449294/pkg/controller/operators/olm/operatorgroup.go#L483-L484) function should be updated to inspect the `olmConfig.Spec.DisableAllNamespacePermissionPromotion` field and:
+- Ensure that the `permissions` are promoted to `clusterPermissions` when the field is set to `nil` or `false`.
+- Ensure that the `permissions` are not promoted to `clusterPermissions` when the field is set to `true`.
+
+### User Stories [optional]
+#### Story 1
+
+As a cluster admin, I would like to be able to remove the view and edit aggregate roles that OLM creates for the APIs it introduces so only a select number of users can interact with OLM.
+
+#### Story 2
+
+As a cluster admin, I would like to be able to configure OLM so it does not create the  `edit`, `view`, `crdview,` and `admin` aggregate roleBindings for the CRDs of operators it installs.
+
+#### Story 2
+
+As a cluster admin, I would like to be able to configure OLM so it does not promote roles and roleBindings defined in a CSV's `spec.permissions` field to clusterRole and clusterRoleBindings respectively so I may have greater control over the RBAC generated for each operator.
+
+### Implementation Details/Notes/Constraints [optional]
+
+
+After disabling OLM's promotion of a CSV's `permissions` to `clusterPermissions` through the `DisableAllNamespacePermissionPromotion` toggle, the cluster admin will need to complete a few manual steps for each operator installed ath the cluster level.
+
+
+#### Namespaced RBAC Generation
+
+The cluster admin will need to manually create the roles and roleBindings in each of the namespaces they want to "scope" the operator to.
+
+While this could be done manually, it may be helpful to provide support for packaging [combo](https://github.com/operator-framework/combo) or providing the steps to using the combo cli to generate the roles and roleBindings for a list of namespaces from a CSV.
+
+#### Handling events from Informers
+
+Operator clients are created today with informers that are configured to watch for events in specific or all namespaces at startup, there is no dynamic informer that exists. Typically, informers used by operators are appropriately scoped to the correct namespaces using the `WATCH_NAMESPACE` environment variable. Operators managed by OLM set the value of this environment variable by referencing the 'olm.targetNamespaces` annotation on the CSV, which in turn is set by OLM based on the targetNamespaces defined by the operatorGroup in the namespace.
+
+The cluster admin will need to override the CSV's `WATCH_NAMESPACE` environment variable through the `Subscription's spec.config.env` field, to include the set of namespaces they want the operator to operator in. Failure to address this concern will result in the operator receiving events from namespaces in which the operator does not have the appropriate RBAC permissions.
+
+### Risks and Mitigations
+
+### Impact on Copied CSVs
+
+Historically, OLM has relied on [copied CSVs](https://olm.operatorframework.io/docs/concepts/crds/operatorgroup/#copied-csvs) to communicate to users which operators are available in a given namespace. At a high level, whenever OLM installs an operator in a namespace, a lightweight copy of the CSV is created in each of the targetNamespaces defined by the operatorGroup in the namespace.
+
+By allowing cluster admins to disable OLM's promotion of RBAC requested by an operator [Copied CSVs](https://olm.operatorframework.io/docs/concepts/crds/operatorgroup/#copied-csvs) there is a chance that the operator will not have the RBAC required to reconcile events in a given namespace, providing users a confusing experience.
+
+It may be useful to recommend that users [disable copied CSVs](https://olm.operatorframework.io/docs/advanced-tasks/configuring-olm/#disabling-copied-csvs) when disabling RBAC generation.
+
+### Impact on OpenShift Console
+
+The [OpenShift Console](https://github.com/openshift/console) project makes a few assumptions on Operator scope based on the OperatorGroup in its namespace. By allowing cluster admins to disable OLM's promotion of RBAC requested by an operator, the UI will incorrectly inform users that an operator is available in a given namespace, yielding a bad user experience. As such, this feature must make it clear to users that it renders parts of the UI invalid.
+
+## Design Details
+
+### Test Plan
+
+The following testcases should be implemented as a part of this enhancement:
+- Setting the `cluster olmConfig` resource's `spec.features.disableOLMAggregateCRDRoles` to `true` removes the `aggregate-olm-edit` and `aggregate-olm-view` clusterRoles from the cluster.
+- Setting the `cluster olmConfig` resource's `spec.features.disableAggregateCRDRoles` to `true` prevents OLM from creating `edit`, `view`, `crdview,` and `admin` roleBindings for CRDs and APIServices introduced by the CSVs.
+- Setting the `cluster olmConfig` resource's `spec.features.disableAllNamespacePermissionPromotion` to `true` prevents OLM from converting roles and roleBindings defined in a CSV's `spec.permissions` field to clusterRoles and clusterRoleBindings respectively.
+
+### Prototype Demo [optional]
+
+A proof of concept implementation highlighting the `disableAggregateCRDRoles` and `disableAllNamespacePermissionPromotion` toggles can be found [here](https://github.com/awgreene/operator-lifecycle-manager/tree/rbac-toggle-demo).
+
+### Graduation Criteria
+
+Straight to GA.
+
+#### Examples
+
+Not applicable.
+
+##### Dev Preview -> Tech Preview
+
+Straight to GA.
+
+##### Tech Preview -> GA
+
+Straight to GA.
+
+##### Removing a deprecated feature
+
+Not applicable.
+
+### Upgrade / Downgrade Strategy
+
+If OLM is downgraded the proposed changes to the API will be lost and OLM may begin generating RBAC for the operators that were created. As such, the OLM team should investigate:
+
+- How OLM can prevent cluster downgrades.
+- How we could communicate the potential damage to customers if they initiate a downgrade after utilizing this feature.
+
+### Version Skew Strategy
+
+The is feature does not depend on other projects and uses existing APIs, as such there is little concern regarding the version of other components.
+
+## Implementation History
+
+Not applicable.
+
+## Drawbacks
+
+The goal of this enhancement is to provide cluster admins with greater control over the RBAC that OLM generates on cluster. Providing cluster admin's with the means to disable OLM's RBAC generation naturally places a greater burden on cluster admins to manage RBAC as they install new operators. There are a number of existing processes that become more tedious when RBAC generation is enabled, to name a few:
+
+- The cluster admin must keep track of the namespaces that include the roles and roleBindings required by the operator. If the cluster admin wants to extend the scope of the operator to additional namespaces, they would need to create additional RBAC in said namespaces.
+- The cluster admin must keep the `WATCHED_NAMESPACES` environment variable up to date, otherwise the informer will either:
+-- Receive events from namespaces that it lacks the appropriate RBAC.
+-- Not Receive events from namespaces that it has the appropriate RBAC.
+## Alternatives
+
+### Multi Namespace Cluster Singletons
+
+A core requirements of this enhancement is to prevent OLM from promoting roles/roleBindings defined in a CSV's permissions to clusterRoles and clusterRoleBindings respectively when installing an operator in AllNamespace mode. Instead of disabling RBAC promotion by OLM, we could instead add a feature that treats MultiNamespace operatorGroups as cluster singletons, which would remove the need to modify the list of namespaces that an operator is configured to watch.
+
+Unfortunately, OLM has been directing operator authors to use the AllNamespace installMode, which means that some subset of operator may not even support the MultiNamespace installMode.
+
+## Infrastructure Needed [optional]
+
+Not applicable.
+
