kubeadm: Always enable RBAC, validate authz mode and improve the code slightly

cmd/kubeadm/app/apis/kubeadm/validation/BUILD

@@ -15,6 +15,7 @@ go_library(
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
+        "//pkg/kubeapiserver/authorizer/modes:go_default_library",
         "//pkg/registry/core/service/ipallocator:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/util/validation/field",
     ],

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -22,6 +22,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
 	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 )
 
@@ -43,8 +44,9 @@ var cloudproviders = []string{
 func ValidateMasterConfiguration(c *kubeadm.MasterConfiguration) field.ErrorList {
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, ValidateDiscovery(&c.Discovery, field.NewPath("discovery"))...)
-	allErrs = append(allErrs, ValidateDiscovery(&c.Discovery, field.NewPath("service subnet"))...)
+	allErrs = append(allErrs, ValidateServiceSubnet(c.Networking.ServiceSubnet, field.NewPath("service subnet"))...)
 	allErrs = append(allErrs, ValidateCloudProvider(c.CloudProvider, field.NewPath("cloudprovider"))...)
+	allErrs = append(allErrs, ValidateAuthorizationMode(c.AuthorizationMode, field.NewPath("authorization-mode"))...)
 	return allErrs
 }
 
@@ -96,6 +98,13 @@ func ValidateTokenDiscovery(c *kubeadm.TokenDiscovery, fldPath *field.Path) fiel
 	return allErrs
 }
 
+func ValidateAuthorizationMode(authzMode string, fldPath *field.Path) field.ErrorList {
+	if !authzmodes.IsValidAuthorizationMode(authzMode) {
+		return field.ErrorList{field.Invalid(fldPath, nil, "invalid authorization mode")}
+	}
+	return field.ErrorList{}
+}
+
 func ValidateServiceSubnet(subnet string, fldPath *field.Path) field.ErrorList {
 	_, svcSubnet, err := net.ParseCIDR(subnet)
 	if err != nil {

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -46,6 +46,30 @@ func TestValidateTokenDiscovery(t *testing.T) {
 	}
 }
 
+func TestValidateAuthorizationMode(t *testing.T) {
+	var tests = []struct {
+		s        string
+		f        *field.Path
+		expected bool
+	}{
+		{"", nil, false},
+		{"rBAC", nil, false},      // not supported
+		{"not valid", nil, false}, // not supported
+		{"RBAC", nil, true},       // supported
+		{"Webhook", nil, true},    // supported
+	}
+	for _, rt := range tests {
+		actual := ValidateAuthorizationMode(rt.s, rt.f)
+		if (len(actual) == 0) != rt.expected {
+			t.Errorf(
+				"failed ValidateAuthorizationMode:\n\texpected: %t\n\t  actual: %t",
+				rt.expected,
+				(len(actual) == 0),
+			)
+		}
+	}
+}
+
 func TestValidateServiceSubnet(t *testing.T) {
 	var tests = []struct {
 		s        string
@@ -111,16 +135,28 @@ func TestValidateMasterConfiguration(t *testing.T) {
 					Addresses: []string{"foobar"},
 				},
 			},
+			AuthorizationMode: "RBAC",
+			Networking: kubeadm.Networking{
+				ServiceSubnet: "10.96.0.1/12",
+			},
 		}, false},
 		{&kubeadm.MasterConfiguration{
 			Discovery: kubeadm.Discovery{
 				HTTPS: &kubeadm.HTTPSDiscovery{URL: "foo"},
 			},
+			AuthorizationMode: "RBAC",
+			Networking: kubeadm.Networking{
+				ServiceSubnet: "10.96.0.1/12",
+			},
 		}, true},
 		{&kubeadm.MasterConfiguration{
 			Discovery: kubeadm.Discovery{
 				File: &kubeadm.FileDiscovery{Path: "foo"},
 			},
+			AuthorizationMode: "RBAC",
+			Networking: kubeadm.Networking{
+				ServiceSubnet: "10.96.0.1/12",
+			},
 		}, true},
 		{&kubeadm.MasterConfiguration{
 			Discovery: kubeadm.Discovery{
@@ -130,6 +166,10 @@ func TestValidateMasterConfiguration(t *testing.T) {
 					Addresses: []string{"foobar"},
 				},
 			},
+			AuthorizationMode: "RBAC",
+			Networking: kubeadm.Networking{
+				ServiceSubnet: "10.96.0.1/12",
+			},
 		}, true},
 	}
 	for _, rt := range tests {

cmd/kubeadm/app/cmd/init.go

@@ -253,11 +253,9 @@ func (i *Init) Run(out io.Writer) error {
 		return err
 	}
 
-	if i.cfg.AuthorizationMode == kubeadmconstants.AuthzModeRBAC {
+	err = apiconfigphase.CreateRBACRules(client)
-		err = apiconfigphase.CreateRBACRules(client)
+	if err != nil {
-		if err != nil {
+		return err
-			return err
-		}
 	}
 
 	if err := addonsphase.CreateEssentialAddons(i.cfg, client); err != nil {

cmd/kubeadm/app/constants/constants.go

@@ -16,11 +16,14 @@ limitations under the License.
 
 package constants
 
-import "time"
+import (
+	"path"
+	"time"
+)
 
 const (
-	AuthorizationPolicyFile        = "abac_policy.json"
+	// KubernetesDir is the directory kubernetes owns for storing various configuration files
-	AuthorizationWebhookConfigFile = "webhook_authz.conf"
+	KubernetesDir = "/etc/kubernetes"
 
 	CACertAndKeyBaseName = "ca"
 	CACertName           = "ca.crt"
@@ -49,14 +52,6 @@ const (
 	AdminKubeConfigFileName   = "admin.conf"
 	KubeletKubeConfigFileName = "kubelet.conf"
 
-	// TODO: These constants should actually come from pkg/kubeapiserver/authorizer, but we can't vendor that package in now
-	// because of all the other sub-packages that would get vendored. To fix this, a pkg/kubeapiserver/authorizer/modes package
-	// or similar should exist that only has these constants; then we can vendor it.
-	AuthzModeAlwaysAllow = "AlwaysAllow"
-	AuthzModeABAC        = "ABAC"
-	AuthzModeRBAC        = "RBAC"
-	AuthzModeWebhook     = "Webhook"
-
 	// Important: a "v"-prefix shouldn't exist here; semver doesn't allow that
 	MinimumControlPlaneVersion = "1.6.0-alpha.2"
 
@@ -83,3 +78,8 @@ const (
 	// The file name of the tokens file that can be used for bootstrapping
 	CSVTokenFileName = "tokens.csv"
 )
+
+var (
+	AuthorizationPolicyPath        = path.Join(KubernetesDir, "abac_policy.json")
+	AuthorizationWebhookConfigPath = path.Join(KubernetesDir, "webhook_authz.conf")
+)

cmd/kubeadm/app/master/BUILD

@@ -24,6 +24,7 @@ go_library(
         "//cmd/kubeadm/app/images:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",
         "//cmd/kubeadm/app/util/kubeconfig:go_default_library",
+        "//pkg/kubeapiserver/authorizer/modes:go_default_library",
         "//pkg/kubectl/cmd/util:go_default_library",
         "//vendor:github.com/ghodss/yaml",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",

cmd/kubeadm/app/master/manifests.go

@@ -32,6 +32,7 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/images"
+	authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 )
 
@@ -326,15 +327,7 @@ func getAPIServerCommand(cfg *kubeadmapi.MasterConfiguration, selfHosted bool) [
 		"--requestheader-allowed-names=front-proxy-client",
 	)
 
-	if cfg.AuthorizationMode != "" {
+	command = append(command, getAuthzParameters(cfg.AuthorizationMode)...)
-		command = append(command, "--authorization-mode="+cfg.AuthorizationMode)
-		switch cfg.AuthorizationMode {
-		case kubeadmconstants.AuthzModeABAC:
-			command = append(command, "--authorization-policy-file="+path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationPolicyFile))
-		case kubeadmconstants.AuthzModeWebhook:
-			command = append(command, "--authorization-webhook-config-file="+path.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationWebhookConfigFile))
-		}
-	}
 
 	// Use first address we are given
 	if len(cfg.API.AdvertiseAddresses) > 0 {
@@ -459,3 +452,22 @@ func getSelfHostedAPIServerEnv() []api.EnvVar {
 
 	return append(getProxyEnvVars(), podIPEnvVar)
 }
+
+func getAuthzParameters(authzMode string) []string {
+	command := []string{}
+	// RBAC is always on. If the user specified
+	authzModes := []string{authzmodes.ModeRBAC}
+	if len(authzMode) != 0 && authzMode != authzmodes.ModeRBAC {
+		authzModes = append(authzModes, authzMode)
+	}
+
+	command = append(command, "--authorization-mode="+strings.Join(authzModes, ","))
+
+	switch authzMode {
+	case authzmodes.ModeABAC:
+		command = append(command, "--authorization-policy-file="+kubeadmconstants.AuthorizationPolicyPath)
+	case authzmodes.ModeWebhook:
+		command = append(command, "--authorization-webhook-config-file="+kubeadmconstants.AuthorizationWebhookConfigPath)
+	}
+	return command
+}

cmd/kubeadm/app/master/manifests_test.go

@@ -388,6 +388,7 @@ func TestGetAPIServerCommand(t *testing.T) {
 				"--requestheader-extra-headers-prefix=X-Remote-Extra-",
 				"--requestheader-client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/front-proxy-ca.crt",
 				"--requestheader-allowed-names=front-proxy-client",
+				"--authorization-mode=RBAC",
 				"--etcd-servers=http://127.0.0.1:2379",
 			},
 		},
@@ -417,6 +418,7 @@ func TestGetAPIServerCommand(t *testing.T) {
 				"--requestheader-extra-headers-prefix=X-Remote-Extra-",
 				"--requestheader-client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/front-proxy-ca.crt",
 				"--requestheader-allowed-names=front-proxy-client",
+				"--authorization-mode=RBAC",
 				"--advertise-address=foo",
 				"--etcd-servers=http://127.0.0.1:2379",
 			},
@@ -448,6 +450,7 @@ func TestGetAPIServerCommand(t *testing.T) {
 				"--requestheader-extra-headers-prefix=X-Remote-Extra-",
 				"--requestheader-client-ca-file=" + kubeadmapi.GlobalEnvParams.HostPKIPath + "/front-proxy-ca.crt",
 				"--requestheader-allowed-names=front-proxy-client",
+				"--authorization-mode=RBAC",
 				"--etcd-servers=http://127.0.0.1:2379",
 				"--etcd-certfile=fiz",
 				"--etcd-keyfile=faz",
@@ -567,3 +570,62 @@ func TestGetSchedulerCommand(t *testing.T) {
 		}
 	}
 }
+
+func TestGetAuthzParameters(t *testing.T) {
+	var tests = []struct {
+		authMode string
+		expected []string
+	}{
+		{
+			authMode: "",
+			expected: []string{
+				"--authorization-mode=RBAC",
+			},
+		},
+		{
+			authMode: "RBAC",
+			expected: []string{
+				"--authorization-mode=RBAC",
+			},
+		},
+		{
+			authMode: "AlwaysAllow",
+			expected: []string{
+				"--authorization-mode=RBAC,AlwaysAllow",
+			},
+		},
+		{
+			authMode: "AlwaysDeny",
+			expected: []string{
+				"--authorization-mode=RBAC,AlwaysDeny",
+			},
+		},
+		{
+			authMode: "ABAC",
+			expected: []string{
+				"--authorization-mode=RBAC,ABAC",
+				"--authorization-policy-file=/etc/kubernetes/abac_policy.json",
+			},
+		},
+		{
+			authMode: "Webhook",
+			expected: []string{
+				"--authorization-mode=RBAC,Webhook",
+				"--authorization-webhook-config-file=/etc/kubernetes/webhook_authz.conf",
+			},
+		},
+	}
+
+	for _, rt := range tests {
+		actual := getAuthzParameters(rt.authMode)
+		for i := range actual {
+			if actual[i] != rt.expected[i] {
+				t.Errorf(
+					"failed getAuthzParameters:\n\texpected: %s\n\t  actual: %s",
+					rt.expected[i],
+					actual[i],
+				)
+			}
+		}
+	}
+}

cmd/kubeadm/app/preflight/BUILD

@@ -16,6 +16,7 @@ go_library(
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//pkg/api/validation:go_default_library",
+        "//pkg/kubeapiserver/authorizer/modes:go_default_library",
         "//pkg/util/initsystem:go_default_library",
         "//pkg/util/node:go_default_library",
         "//test/e2e_node/system:go_default_library",

cmd/kubeadm/app/preflight/checks.go

@@ -31,6 +31,7 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/pkg/api/validation"
+	authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
 	"k8s.io/kubernetes/pkg/util/initsystem"
 	"k8s.io/kubernetes/pkg/util/node"
 	"k8s.io/kubernetes/test/e2e_node/system"
@@ -363,12 +364,10 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 
 	// Check the config for authorization mode
 	switch cfg.AuthorizationMode {
-	case kubeadmconstants.AuthzModeABAC:
+	case authzmodes.ModeABAC:
-		authorizationPolicyPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationPolicyFile)
+		checks = append(checks, FileExistingCheck{Path: kubeadmconstants.AuthorizationPolicyPath})
-		checks = append(checks, FileExistingCheck{Path: authorizationPolicyPath})
+	case authzmodes.ModeWebhook:
-	case kubeadmconstants.AuthzModeWebhook:
+		checks = append(checks, FileExistingCheck{Path: kubeadmconstants.AuthorizationWebhookConfigPath})
-		authorizationWebhookConfigPath := filepath.Join(kubeadmapi.GlobalEnvParams.KubernetesDir, kubeadmconstants.AuthorizationWebhookConfigFile)
-		checks = append(checks, FileExistingCheck{Path: authorizationWebhookConfigPath})
 	}
 
 	return RunChecks(checks, os.Stderr)