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

Setting authorization mode to RBAC by default #1904

Closed
wants to merge 3 commits into from

Conversation

arschles
Copy link
Contributor

@arschles arschles commented Aug 31, 2017

Fixes #1722

Before this change, kubectl get clusterrole and kubectl get clusterrolebinding -- both indicators that RBAC auth is enabled in the cluster -- returned nothing. After this change, the ./out/minikube start command creates a cluster in which the following commands indicate that RBAC is turned on:

$ k get clusterrole
NAME                                           AGE
admin                                          4m
cluster-admin                                  4m
edit                                           4m
system:auth-delegator                          4m
system:basic-user                              4m
system:controller:attachdetach-controller      4m
system:controller:certificate-controller       4m
system:controller:cronjob-controller           4m
system:controller:daemon-set-controller        4m
system:controller:deployment-controller        4m
system:controller:disruption-controller        4m
system:controller:endpoint-controller          4m
system:controller:generic-garbage-collector    4m
system:controller:horizontal-pod-autoscaler    4m
system:controller:job-controller               4m
system:controller:namespace-controller         4m
system:controller:node-controller              4m
system:controller:persistent-volume-binder     4m
system:controller:pod-garbage-collector        4m
system:controller:replicaset-controller        4m
system:controller:replication-controller       4m
system:controller:resourcequota-controller     4m
system:controller:route-controller             4m
system:controller:service-account-controller   4m
system:controller:service-controller           4m
system:controller:statefulset-controller       4m
system:controller:ttl-controller               4m
system:discovery                               4m
system:heapster                                4m
system:kube-aggregator                         4m
system:kube-controller-manager                 4m
system:kube-dns                                4m
system:kube-scheduler                          4m
system:node                                    4m
system:node-bootstrapper                       4m
system:node-problem-detector                   4m
system:node-proxier                            4m
system:persistent-volume-provisioner           4m
view                                           4m
$ kubectl get clusterrolebinding
NAME                                           AGE
cluster-admin                                  6m
system:basic-user                              6m
system:controller:attachdetach-controller      6m
system:controller:certificate-controller       6m
system:controller:cronjob-controller           6m
system:controller:daemon-set-controller        6m
system:controller:deployment-controller        6m
system:controller:disruption-controller        6m
system:controller:endpoint-controller          6m
system:controller:generic-garbage-collector    6m
system:controller:horizontal-pod-autoscaler    6m
system:controller:job-controller               6m
system:controller:namespace-controller         6m
system:controller:node-controller              6m
system:controller:persistent-volume-binder     6m
system:controller:pod-garbage-collector        6m
system:controller:replicaset-controller        6m
system:controller:replication-controller       6m
system:controller:resourcequota-controller     6m
system:controller:route-controller             6m
system:controller:service-account-controller   6m
system:controller:service-controller           6m
system:controller:statefulset-controller       6m
system:controller:ttl-controller               6m
system:discovery                               6m
system:kube-controller-manager                 6m
system:kube-dns                                6m
system:kube-scheduler                          6m
system:node                                    6m
system:node-proxier                            6m

This patch would slightly improve the service-catalog installation process which currently has to instruct users to pass --extra-config=apiserver.Authorization.Mode=RBAC to minikube installations. If minikube turned on RBAC by default, we could remove the minikube-specific section altogether, and the first-time installation process would be smoother for minikube users.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 31, 2017
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@dlorenc
Copy link
Contributor

dlorenc commented Aug 31, 2017

@minikube-bot OK to test

@r2d4
Copy link
Contributor

r2d4 commented Aug 31, 2017

I think this is going to need a few more changes to our addons and some service account permissions (e.g. the dashboard won't have enough permissions to query) . #1903 adds kubeadm as a cluster bootstrapper, which enables RBAC. I needed to add something like this https://github.com/r2d4/minikube/blob/93e9ba58b3936e353b1d5e0d1908989463693f21/pkg/minikube/bootstrapper/kubeadm/util.go#L84-L109 to make the dashboard usable out of the box.

@arschles
Copy link
Contributor Author

thanks @r2d4 - I'll add perms elevation to accommodate the dashboard. do you know where I can look to see these addons?

@codecov-io
Copy link

codecov-io commented Aug 31, 2017

Codecov Report

Merging #1904 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1904      +/-   ##
==========================================
- Coverage   31.03%   31.01%   -0.02%     
==========================================
  Files          74       74              
  Lines        4302     4304       +2     
==========================================
  Hits         1335     1335              
- Misses       2792     2794       +2     
  Partials      175      175
Impacted Files Coverage Δ
pkg/localkube/apiserver.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81ec77a...42596c1. Read the comment docs.

@r2d4
Copy link
Contributor

r2d4 commented Aug 31, 2017

@arschles
Copy link
Contributor Author

got them, thanks @r2d4

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 31, 2017
@dlorenc
Copy link
Contributor

dlorenc commented Sep 6, 2017

Looks like there are some build failures:

pkg/minikube/bootstrapper/localkube/privileges.go:5: imported and not used: "k8s.io/minikube/vendor/k8s.io/apimachinery/pkg/api/errors" as apierrs
pkg/minikube/bootstrapper/localkube/privileges.go:7: imported and not used: "k8s.io/minikube/vendor/k8s.io/apimachinery/pkg/types"
pkg/minikube/bootstrapper/localkube/privileges.go:8: imported and not used: "k8s.io/minikube/vendor/k8s.io/apimachinery/pkg/util/strategicpatch"
pkg/minikube/bootstrapper/localkube/privileges.go:9: imported and not used: "k8s.io/minikube/vendor/k8s.io/client-go/pkg/api/v1" as clientv1

@r2d4 r2d4 mentioned this pull request Sep 11, 2017
wallrj added a commit to wallrj/navigator that referenced this pull request Nov 9, 2017
This should prevent intermittent E2E test failures in case Minikube API server is
not yet ready to accept configuration changes.

Inspired by: kubernetes/minikube#1904
munnerz pushed a commit to wallrj/navigator that referenced this pull request Nov 13, 2017
This should prevent intermittent E2E test failures in case Minikube API server is
not yet ready to accept configuration changes.

Inspired by: kubernetes/minikube#1904
jetstack-bot added a commit to jetstack/navigator that referenced this pull request Nov 13, 2017
Automatic merge from submit-queue.

Retry the kube-system RBAC cluserrolebinding fix

This should prevent intermittent E2E test failures in case Minikube API server is
not yet ready to accept configuration changes.

Inspired by: kubernetes/minikube#1904

Fixes: #117
**Release note**:
```release-note
NONE
```
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 4, 2018
},
RoleRef: rbacv1beta1.RoleRef{
Kind: "ClusterRole",
Name: "cluster-admin",
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if i'm wrong, but I believe most RBAC-enabled clusters are not setup to give the kube-system:default service account cluster-admin privileges. Is it maybe safer to grant these privileges in the addons and services that need them, or perhaps create a new minikube-default service account to use in pods that would only exist in minikube?

@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 28, 2018
@kfox1111
Copy link

/remove-lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 28, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 29, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 28, 2018
@kenden
Copy link
Contributor

kenden commented Jun 29, 2018

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 29, 2018
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@balopat
Copy link
Contributor

balopat commented Nov 6, 2018

@kenden Based on the discussion in #1722 it seems that RBAC is the default. Is this PR still required?

@tstromberg tstromberg closed this Jan 16, 2019
@tstromberg
Copy link
Contributor

Closing because I think this PR is obsolete. Please re-open if this is incorrect.

@arschles arschles deleted the rbac-default branch March 26, 2019 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.