From 735d2e71558ddde16248adfdfd31bd4e53be18f3 Mon Sep 17 00:00:00 2001 From: Steve Leon Date: Tue, 14 Nov 2017 15:33:33 -0800 Subject: [PATCH] Not create rbac roles on k8s cluster that have rbac disabled. Fixes https://github.com/rook/rook/issues/1221 --- cluster/charts/rook/templates/deployment.yaml | 4 + .../examples/kubernetes/rook-operator.yaml | 3 + pkg/operator/k8sutil/rbac.go | 21 +++++ pkg/operator/k8sutil/rbac_test.go | 82 +++++++++++++++++++ 4 files changed, 110 insertions(+) diff --git a/cluster/charts/rook/templates/deployment.yaml b/cluster/charts/rook/templates/deployment.yaml index cb42464d2..fb542a015 100644 --- a/cluster/charts/rook/templates/deployment.yaml +++ b/cluster/charts/rook/templates/deployment.yaml @@ -23,6 +23,10 @@ spec: env: - name: ROOK_REPO_PREFIX value: {{ .Values.image.prefix }} +{{- if not .Values.rbacEnable }} + - name: RBAC_ENABLED + value: "false" +{{- end }} - name: ROOK_LOG_LEVEL value: {{ .Values.logLevel }} - name: NODE_NAME diff --git a/cluster/examples/kubernetes/rook-operator.yaml b/cluster/examples/kubernetes/rook-operator.yaml index cb0d56e58..a80af838a 100644 --- a/cluster/examples/kubernetes/rook-operator.yaml +++ b/cluster/examples/kubernetes/rook-operator.yaml @@ -123,6 +123,9 @@ spec: env: - name: ROOK_REPO_PREFIX value: rook + # To disable RBAC, uncomment the following: + # - name: RBAC_ENABLED + # value: "false" # The interval to check if every mon is in the quorum. - name: ROOK_MON_HEALTHCHECK_INTERVAL value: "45s" diff --git a/pkg/operator/k8sutil/rbac.go b/pkg/operator/k8sutil/rbac.go index 425a99bc9..15dedb8b1 100644 --- a/pkg/operator/k8sutil/rbac.go +++ b/pkg/operator/k8sutil/rbac.go @@ -22,6 +22,7 @@ package k8sutil import ( "fmt" + "os" "k8s.io/api/core/v1" "k8s.io/api/rbac/v1beta1" @@ -30,6 +31,10 @@ import ( "k8s.io/client-go/kubernetes" ) +const ( + enableRBACEnv = "RBAC_ENABLED" +) + func MakeRole(clientset kubernetes.Interface, namespace, name string, rules []v1beta1.PolicyRule) error { err := makeServiceAccount(clientset, namespace, name) @@ -37,6 +42,10 @@ func MakeRole(clientset kubernetes.Interface, namespace, name string, rules []v1 return err } + if !isRBACEnabled() { + return nil + } + // Create the role if it doesn't yet exist. // If the role already exists we have to update it. Otherwise if the permissions change during an upgrade, // the create will fail with an error that we're changing the permissions. @@ -72,6 +81,10 @@ func MakeClusterRole(clientset kubernetes.Interface, namespace, name string, rul return err } + if !isRBACEnabled() { + return nil + } + // Create the cluster scoped role if it doesn't yet exist. // If the role already exists we have to update it. Otherwise if the permissions change during an upgrade, // the create will fail with an error that we're changing the permissions. @@ -110,3 +123,11 @@ func makeServiceAccount(clientset kubernetes.Interface, namespace, name string) } return nil } + +func isRBACEnabled() bool { + r := os.Getenv(enableRBACEnv) + if r == "false" { + return false + } + return true +} diff --git a/pkg/operator/k8sutil/rbac_test.go b/pkg/operator/k8sutil/rbac_test.go index 8ed1151d1..ca4e528cd 100644 --- a/pkg/operator/k8sutil/rbac_test.go +++ b/pkg/operator/k8sutil/rbac_test.go @@ -21,11 +21,13 @@ which also has the apache 2.0 license. package k8sutil import ( + "os" "testing" "github.com/rook/rook/pkg/operator/test" "github.com/stretchr/testify/assert" "k8s.io/api/rbac/v1beta1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -146,3 +148,83 @@ func TestMakeClusterRole(t *testing.T) { assert.Equal(t, 1, len(role.Rules[0].Resources)) assert.Equal(t, 2, len(role.Rules[0].Verbs)) } + +func TestMakeRoleRBACDisabled(t *testing.T) { + os.Setenv(enableRBACEnv, "false") + defer os.Unsetenv(enableRBACEnv) + clientset := test.New(1) + namespace := "myns" + name := "myapp" + + rules := []v1beta1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"namespaces", "secrets", "pods", "services", "nodes", "configmaps", "events"}, + Verbs: []string{"get", "list", "watch", "create", "update"}, + }, + { + APIGroups: []string{"extensions"}, + Resources: []string{"thirdpartyresources", "deployments", "daemonsets", "replicasets"}, + Verbs: []string{"get", "list", "create", "delete"}, + }, + { + APIGroups: []string{"apiextensions.k8s.io"}, + Resources: []string{"customresourcedefinitions"}, + Verbs: []string{"get", "list", "create"}, + }, + { + APIGroups: []string{"storage.k8s.io"}, + Resources: []string{"storageclasses"}, + Verbs: []string{"get", "list"}, + }, + } + + err := MakeRole(clientset, namespace, name, rules) + + _, err = clientset.RbacV1beta1().Roles(namespace).Get(name, metav1.GetOptions{}) + assert.NotNil(t, err) + assert.True(t, errors.IsNotFound(err)) + + account, err := clientset.CoreV1().ServiceAccounts(namespace).Get(name, metav1.GetOptions{}) + assert.Nil(t, err) + assert.Equal(t, namespace, account.Namespace) + + _, err = clientset.RbacV1beta1().RoleBindings(namespace).Get(name, metav1.GetOptions{}) + assert.NotNil(t, err) + assert.True(t, errors.IsNotFound(err)) +} + +func TestMakeClusterRoleRBACDisabled(t *testing.T) { + os.Setenv(enableRBACEnv, "false") + defer os.Unsetenv(enableRBACEnv) + clientset := test.New(1) + namespace := "myns" + name := "myapp" + + rules := []v1beta1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "secrets", "configmaps", "persistentvolumes", "nodes/proxy"}, + Verbs: []string{"get", "list"}, + }, + { + APIGroups: []string{"storage.k8s.io"}, + Resources: []string{"storageclasses"}, + Verbs: []string{"get"}, + }, + } + + err := MakeClusterRole(clientset, namespace, name, rules) + assert.Nil(t, err) + _, err = clientset.RbacV1beta1().ClusterRoles().Get(name, metav1.GetOptions{}) + assert.NotNil(t, err) + assert.True(t, errors.IsNotFound(err)) + + account, err := clientset.CoreV1().ServiceAccounts(namespace).Get(name, metav1.GetOptions{}) + assert.Nil(t, err) + assert.Equal(t, namespace, account.Namespace) + + _, err = clientset.RbacV1beta1().ClusterRoleBindings().Get(name, metav1.GetOptions{}) + assert.NotNil(t, err) + assert.True(t, errors.IsNotFound(err)) +}