Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce the Crossplane RBAC Manager #1728

Merged
merged 5 commits into from
Sep 15, 2020
Merged

Introduce the Crossplane RBAC Manager #1728

merged 5 commits into from
Sep 15, 2020

Conversation

negz
Copy link
Member

@negz negz commented Sep 12, 2020

Description of your changes

Partial fix for #1637

This PR introduces the RBAC manager - a distinct deployment that generates opinionated RBAC roles that grant access to Crossplane resources as they are created. Currently the RBAC manager grants access to newly defined composite resources and their claims. Deploying the RBAC manager is optional. It is deployed by default. See the design document for further context.

Note that while the RBAC manager grants Crossplane itself access to reconcile composite resources, Crossplane must be manually granted access to manage any composed resources; i.e. a provider's managed resources. A future PR will introduce support for granting Crossplane access to manage the resources of any provider installed via the v2 package manager.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

In addition to the unit tests, this PR has been tested manually. See comments below for details. Reviewers, it's probably worth double-checking my Helm updates; I found our Helm chart architecture tough to understand.

Signed-off-by: Nic Cope <negz@rk0n.org>
Signed-off-by: Nic Cope <negz@rk0n.org>
@negz
Copy link
Member Author

negz commented Sep 12, 2020

Installing Crossplane installs the RBAC manager by default, using the --manage=All policy.

$ kubectl -n crossplane-system get po
NAME                                          READY   STATUS      RESTARTS   AGE
crossplane-6cc5bc8c86-84qcd                   1/1     Running     0          47m
crossplane-package-manager-55b8df6477-k6zf4   1/1     Running     0          47m
crossplane-rbac-manager-7dbb968798-l8rk4      1/1     Running     0          47m
provider-gcp-controller-ff88cd558-g7ntl       1/1     Running     0          39m
provider-gcp-jhmcd                            0/1     Completed   0          39m

The Helm chart creates several ClusterRoles:

$ kubectl get clusterrole|grep crossplane
crossplane                                                             48m
# The RBAC manager runs as a ClusterRole with 'escalate' privileges.
crossplane-rbac-manager                                                48m
# Hyphenated roles are 'user-facing' - they are intended to be bound
# at the cluster scope to human subjects.
crossplane-admin                                                       48m
crossplane-browse                                                      48m
crossplane-edit                                                        48m
crossplane-view                                                        48m
# Colon separated roles provide the 'base' rules for the above.
crossplane:aggregate-to-admin                                          48m
crossplane:aggregate-to-edit                                           48m
crossplane:aggregate-to-ns-admin                                       48m
crossplane:aggregate-to-ns-edit                                        48m
crossplane:aggregate-to-ns-view                                        48m
crossplane:aggregate-to-view                                           48m
crossplane:system:aggregate-to-crossplane                              48m
# Deprecated package manager roles.
package:crossplane-system:provider-gcp:master:admin                    39m
package:crossplane-system:provider-gcp:master:edit                     39m
package:crossplane-system:provider-gcp:master:system                   40m
package:crossplane-system:provider-gcp:master:view                     39m

Creating an XRD causes the RBAC manager to create ClusterRoles for it:

$ kubectl describe xrd                                                                                                                                   
Name:         compositepostgresqlinstances.database.example.org
Namespace:    
Labels:       <none>
Annotations:  API Version:  apiextensions.crossplane.io/v1alpha1
Kind:         CompositeResourceDefinition
# ...Truncated...
Events:
  Type     Reason              Age              From                                                             Message
  ----     ------              ----             ----                                                             -------
  Normal   EstablishComposite  7s               defined/compositeresourcedefinition.apiextensions.crossplane.io  waiting for composite resource CustomResourceDefinition to be established
  Normal   OfferClaim          7s               offered/compositeresourcedefinition.apiextensions.crossplane.io  waiting for composite resource claim CustomResourceDefinition to be established
  Normal   ApplyClusterRoles   6s (x3 over 7s)  rbac/compositeresourcedefinition.apiextensions.crossplane.io     Applied RBAC ClusterRoles
  Normal   RenderCRD           4s (x5 over 7s)  offered/compositeresourcedefinition.apiextensions.crossplane.io  Rendered composite resource claim CustomResourceDefinition
  Normal   OfferClaim          4s (x5 over 7s)  offered/compositeresourcedefinition.apiextensions.crossplane.io  Applied composite resource claim CustomResourceDefinition
  Normal   EstablishComposite  4s (x4 over 7s)  defined/compositeresourcedefinition.apiextensions.crossplane.io  Applied composite resource CustomResourceDefinition
  Normal   RenderCRD           4s (x7 over 7s)  defined/compositeresourcedefinition.apiextensions.crossplane.io  Rendered composite resource CustomResourceDefinition
  Normal   OfferClaim          4s (x4 over 7s)  offered/compositeresourcedefinition.apiextensions.crossplane.io  (Re)started composite resource claim controller
  Normal   EstablishComposite  4s (x3 over 7s)  defined/compositeresourcedefinition.apiextensions.crossplane.io  (Re)started composite resource controller

$ kubectl get clusterrole|grep crossplane:composite
crossplane:composite:compositepostgresqlinstances.database.example.org:aggregate-to-browse   2m41s
crossplane:composite:compositepostgresqlinstances.database.example.org:aggregate-to-edit     2m41s
crossplane:composite:compositepostgresqlinstances.database.example.org:aggregate-to-view     2m41s

$ kubectl describe clusterrole crossplane:composite:compositepostgresqlinstances.database.example.org:aggregate-to-edit
Name:         crossplane:composite:compositepostgresqlinstances.database.example.org:aggregate-to-edit
Labels:       rbac.crossplane.io/aggregate-to-admin=true
              rbac.crossplane.io/aggregate-to-crossplane=true
              rbac.crossplane.io/aggregate-to-edit=true
              rbac.crossplane.io/aggregate-to-ns-admin=true
              rbac.crossplane.io/aggregate-to-ns-edit=true
              rbac.crossplane.io/xrd=compositepostgresqlinstances.database.example.org
Annotations:  <none>
PolicyRule:
  Resources                                          Non-Resource URLs  Resource Names  Verbs
  ---------                                          -----------------  --------------  -----
  compositepostgresqlinstances.database.example.org  []                 []              [*]
  postgresqlinstances.database.example.org           []                 []              [*]

Creating a namespace causes Crossplane to create managed RBAC roles. The managed roles aggregate rules from only the crossplane:aggregate-to-ns-* cluster roles by default.

$ kubectl create namespace rbacls
namespace/rbacls created

$ kubectl -n rbacls get roles
NAME               AGE
crossplane-admin   5s
crossplane-edit    5s
crossplane-view    5s

$ kubectl -n rbacls describe role crossplane-admin
Name:         crossplane-admin
Labels:       <none>
Annotations:  rbac.crossplane.io/aggregated-by-crossplane: true
PolicyRule:
  Resources                               Non-Resource URLs  Resource Names  Verbs
  ---------                               -----------------  --------------  -----
  secrets                                 []                 []              [*]
  rolebindings.rbac.authorization.k8s.io  []                 []              [*]
  events                                  []                 []              [get list watch]
  roles.rbac.authorization.k8s.io         []                 []              [get list watch]

Annotating the namespace to indicate the claim an XRD offers is accepted in that namespace will cause its RBAC roles to be updated. It now also aggregates from the accepted crossplane:composite:*:aggregate-to-* cluster roles.

$ kubectl annotate namespace rbacls rbac.crossplane.io/compositepostgresqlinstances.database.example.org=xrd-claim-accepted
namespace/rbacls annotated

$ kubectl -n rbacls describe role crossplane-admin
Name:         crossplane-admin
Labels:       <none>
Annotations:  rbac.crossplane.io/aggregated-by-crossplane: true
PolicyRule:
  Resources                                          Non-Resource URLs  Resource Names  Verbs
  ---------                                          -----------------  --------------  -----
  secrets                                            []                 []              [*]
  compositepostgresqlinstances.database.example.org  []                 []              [*]
  postgresqlinstances.database.example.org           []                 []              [*]
  rolebindings.rbac.authorization.k8s.io             []                 []              [*]
  events                                             []                 []              [get list watch]
  roles.rbac.authorization.k8s.io                    []                 []              [get list watch]

Removing the annotation (or setting its value to anything other than xrd-claim-accepted) causes the rules that were aggregated from the crossplane:composite:*:aggregate-to-* cluster role to be removed. Deleting the accepted XRD has the same effect.

$ kubectl annotate --overwrite namespace rbacls rbac.crossplane.io/compositepostgresqlinstances.database.example.org=nope
namespace/rbacls annotated

$ kubectl -n rbacls describe role crossplane-admin
Name:         crossplane-admin
Labels:       <none>
Annotations:  rbac.crossplane.io/aggregated-by-crossplane: true
PolicyRule:
  Resources                               Non-Resource URLs  Resource Names  Verbs
  ---------                               -----------------  --------------  -----
  secrets                                 []                 []              [*]
  rolebindings.rbac.authorization.k8s.io  []                 []              [*]
  events                                  []                 []              [get list watch]
  roles.rbac.authorization.k8s.io         []                 []              [get list watch]

Signed-off-by: Nic Cope <negz@rk0n.org>
Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking pretty good @negz! I left a few comments and questions. The Helm chart changes have a few things that I think could be fixed, but in terms of placement (i.e. in crossplane-controllers vs `crossplane-types) they look correct. The new package manager and my subsequent follow-up for enabling hosted mode will affect the charts again so I am not too worried about their state in this PR 👍

cmd/crossplane/main.go Show resolved Hide resolved
pkg/controller/rbac/definition/roles.go Outdated Show resolved Hide resolved
pkg/controller/rbac/namespace/reconciler.go Outdated Show resolved Hide resolved
pkg/controller/rbac/namespace/reconciler_test.go Outdated Show resolved Hide resolved
pkg/controller/rbac/namespace/reconciler.go Outdated Show resolved Hide resolved
Signed-off-by: Nic Cope <negz@rk0n.org>
Signed-off-by: Nic Cope <negz@rk0n.org>
Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @negz :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants