validate authorization flags in BuiltInAuthorizationOptions.Validate

cmd/kube-apiserver/app/options/options_test.go

@@ -73,7 +73,7 @@ func TestAddFlags(t *testing.T) {
 		"--audit-webhook-initial-backoff=2s",
 		"--authentication-token-webhook-cache-ttl=3m",
 		"--authentication-token-webhook-config-file=/token-webhook-config",
-		"--authorization-mode=AlwaysDeny",
+		"--authorization-mode=AlwaysDeny,RBAC",
 		"--authorization-policy-file=/policy",
 		"--authorization-webhook-cache-authorized-ttl=3m",
 		"--authorization-webhook-cache-unauthorized-ttl=1m",
@@ -248,7 +248,7 @@ func TestAddFlags(t *testing.T) {
 			TokenFailureCacheTTL: 0,
 		},
 		Authorization: &kubeoptions.BuiltInAuthorizationOptions{
-			Mode:                        "AlwaysDeny",
+			Modes:                       []string{"AlwaysDeny", "RBAC"},
 			PolicyFile:                  "/policy",
 			WebhookConfigFile:           "/webhook-config",
 			WebhookCacheAuthorizedTTL:   180000000000,

cmd/kube-apiserver/app/options/validation.go

@@ -67,6 +67,9 @@ func (s *ServerRunOptions) Validate() []error {
 	if errs := s.Authentication.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}
+	if errs := s.Authorization.Validate(); len(errs) > 0 {
+		errors = append(errors, errs...)
+	}
 	if errs := s.Audit.Validate(); len(errs) > 0 {
 		errors = append(errors, errs...)
 	}

cmd/kube-apiserver/app/server.go

@@ -485,7 +485,7 @@ func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transp
 	if err != nil {
 		return nil, nil, nil, nil, nil, fmt.Errorf("invalid authorization config: %v", err)
 	}
-	if !sets.NewString(s.Authorization.Modes()...).Has(modes.ModeRBAC) {
+	if !sets.NewString(s.Authorization.Modes...).Has(modes.ModeRBAC) {
 		genericConfig.DisabledPostStartHooks.Insert(rbacrest.PostStartHookName)
 	}
 

pkg/kubeapiserver/authorizer/BUILD

@@ -3,17 +3,6 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
-    "go_test",
-)
-
-go_test(
-    name = "go_default_test",
-    srcs = ["config_test.go"],
-    data = [
-        "//pkg/auth/authorizer/abac:example_policy",
-    ],
-    embed = [":go_default_library"],
-    deps = ["//pkg/kubeapiserver/authorizer/modes:go_default_library"],
 )
 
 go_library(

pkg/kubeapiserver/authorizer/config.go

@@ -17,7 +17,6 @@ limitations under the License.
 package authorizer
 
 import (
-	"errors"
 	"fmt"
 	"time"
 
@@ -60,20 +59,15 @@ type AuthorizationConfig struct {
 // based on the authorizationMode or an error.
 func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleResolver, error) {
 	if len(config.AuthorizationModes) == 0 {
-		return nil, nil, errors.New("At least one authorization mode should be passed")
+		return nil, nil, fmt.Errorf("at least one authorization mode must be passed")
 	}
 
 	var (
 		authorizers   []authorizer.Authorizer
 		ruleResolvers []authorizer.RuleResolver
 	)
-	authorizerMap := make(map[string]bool)
 
 	for _, authorizationMode := range config.AuthorizationModes {
-		if authorizerMap[authorizationMode] {
-			return nil, nil, fmt.Errorf("Authorization mode %s specified more than once", authorizationMode)
-		}
-
 		// Keep cases in sync with constant list above.
 		switch authorizationMode {
 		case modes.ModeNode:
@@ -96,9 +90,6 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR
 			authorizers = append(authorizers, alwaysDenyAuthorizer)
 			ruleResolvers = append(ruleResolvers, alwaysDenyAuthorizer)
 		case modes.ModeABAC:
-			if config.PolicyFile == "" {
-				return nil, nil, errors.New("ABAC's authorization policy file not passed")
-			}
 			abacAuthorizer, err := abac.NewFromFile(config.PolicyFile)
 			if err != nil {
 				return nil, nil, err
@@ -106,9 +97,6 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR
 			authorizers = append(authorizers, abacAuthorizer)
 			ruleResolvers = append(ruleResolvers, abacAuthorizer)
 		case modes.ModeWebhook:
-			if config.WebhookConfigFile == "" {
-				return nil, nil, errors.New("Webhook's configuration file not passed")
-			}
 			webhookAuthorizer, err := webhook.New(config.WebhookConfigFile,
 				config.WebhookCacheAuthorizedTTL,
 				config.WebhookCacheUnauthorizedTTL)
@@ -127,16 +115,8 @@ func (config AuthorizationConfig) New() (authorizer.Authorizer, authorizer.RuleR
 			authorizers = append(authorizers, rbacAuthorizer)
 			ruleResolvers = append(ruleResolvers, rbacAuthorizer)
 		default:
-			return nil, nil, fmt.Errorf("Unknown authorization mode %s specified", authorizationMode)
+			return nil, nil, fmt.Errorf("unknown authorization mode %s specified", authorizationMode)
 		}
-		authorizerMap[authorizationMode] = true
-	}
-
-	if !authorizerMap[modes.ModeABAC] && config.PolicyFile != "" {
-		return nil, nil, errors.New("Cannot specify --authorization-policy-file without mode ABAC")
-	}
-	if !authorizerMap[modes.ModeWebhook] && config.WebhookConfigFile != "" {
-		return nil, nil, errors.New("Cannot specify --authorization-webhook-config-file without mode Webhook")
 	}
 
 	return union.New(authorizers...), union.NewRuleResolvers(ruleResolvers...), nil

pkg/kubeapiserver/authorizer/config_test.go

@@ -1,101 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package authorizer
-
-import (
-	"k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
-	"testing"
-)
-
-// New has multiple return possibilities. This test
-// validates that errors are returned only when proper.
-func TestNew(t *testing.T) {
-	examplePolicyFile := "../../auth/authorizer/abac/example_policy_file.jsonl"
-
-	tests := []struct {
-		config  AuthorizationConfig
-		wantErr bool
-		msg     string
-	}{
-		{
-			// Unknown modes should return errors
-			config:  AuthorizationConfig{AuthorizationModes: []string{"DoesNotExist"}},
-			wantErr: true,
-			msg:     "using a fake mode should have returned an error",
-		},
-		{
-			// ModeAlwaysAllow and ModeAlwaysDeny should return without authorizationPolicyFile
-			// but error if one is given
-			config: AuthorizationConfig{AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}},
-			msg:    "returned an error for valid config",
-		},
-		{
-			// ModeABAC requires a policy file
-			config:  AuthorizationConfig{AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}},
-			wantErr: true,
-			msg:     "specifying ABAC with no policy file should return an error",
-		},
-		{
-			// ModeABAC should not error if a valid policy path is provided
-			config: AuthorizationConfig{
-				AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC},
-				PolicyFile:         examplePolicyFile,
-			},
-			msg: "errored while using a valid policy file",
-		},
-		{
-
-			// Authorization Policy file cannot be used without ModeABAC
-			config: AuthorizationConfig{
-				AuthorizationModes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny},
-				PolicyFile:         examplePolicyFile,
-			},
-			wantErr: true,
-			msg:     "should have errored when Authorization Policy File is used without ModeABAC",
-		},
-		{
-			// At least one authorizationMode is necessary
-			config:  AuthorizationConfig{PolicyFile: examplePolicyFile},
-			wantErr: true,
-			msg:     "should have errored when no authorization modes are passed",
-		},
-		{
-			// ModeWebhook requires at minimum a target.
-			config:  AuthorizationConfig{AuthorizationModes: []string{modes.ModeWebhook}},
-			wantErr: true,
-			msg:     "should have errored when config was empty with ModeWebhook",
-		},
-		{
-			// Cannot provide webhook flags without ModeWebhook
-			config: AuthorizationConfig{
-				AuthorizationModes: []string{modes.ModeAlwaysAllow},
-				WebhookConfigFile:  "authz_webhook_config.yml",
-			},
-			wantErr: true,
-			msg:     "should have errored when Webhook config file is used without ModeWebhook",
-		},
-	}
-
-	for _, tt := range tests {
-		_, _, err := tt.config.New()
-		if tt.wantErr && (err == nil) {
-			t.Errorf("New %s", tt.msg)
-		} else if !tt.wantErr && (err != nil) {
-			t.Errorf("New %s: %v", tt.msg, err)
-		}
-	}
-}

pkg/kubeapiserver/options/BUILD

@@ -90,8 +90,12 @@ go_test(
     name = "go_default_test",
     srcs = [
         "admission_test.go",
+        "authorization_test.go",
         "storage_versions_test.go",
     ],
     embed = [":go_default_library"],
-    deps = ["//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library"],
+    deps = [
+        "//pkg/kubeapiserver/authorizer/modes:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+    ],
 )

pkg/kubeapiserver/options/authentication.go

@@ -25,6 +25,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/spf13/pflag"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	genericoptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/kubernetes/pkg/kubeapiserver/authenticator"
@@ -365,17 +366,8 @@ func (o *BuiltInAuthenticationOptions) ApplyAuthorization(authorization *BuiltIn
 
 	// authorization ModeAlwaysAllow cannot be combined with AnonymousAuth.
 	// in such a case the AnonymousAuth is stomped to false and you get a message
-	if o.Anonymous.Allow {
+	if o.Anonymous.Allow && sets.NewString(authorization.Modes...).Has(authzmodes.ModeAlwaysAllow) {
-		found := false
+		glog.Warningf("AnonymousAuth is not allowed with the AlwaysAllow authorizer. Resetting AnonymousAuth to false. You should use a different authorizer")
-		for _, mode := range strings.Split(authorization.Mode, ",") {
+		o.Anonymous.Allow = false
-			if mode == authzmodes.ModeAlwaysAllow {
-				found = true
-				break
-			}
-		}
-		if found {
-			glog.Warningf("AnonymousAuth is not allowed with the AllowAll authorizer.  Resetting AnonymousAuth to false. You should use a different authorizer")
-			o.Anonymous.Allow = false
-		}
 	}
 }

pkg/kubeapiserver/options/authorization.go

@@ -17,11 +17,13 @@ limitations under the License.
 package options
 
 import (
+	"fmt"
 	"strings"
 	"time"
 
 	"github.com/spf13/pflag"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	versionedinformers "k8s.io/client-go/informers"
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	"k8s.io/kubernetes/pkg/kubeapiserver/authorizer"
@@ -29,7 +31,7 @@ import (
 )
 
 type BuiltInAuthorizationOptions struct {
-	Mode                        string
+	Modes                       []string
 	PolicyFile                  string
 	WebhookConfigFile           string
 	WebhookCacheAuthorizedTTL   time.Duration
@@ -38,19 +40,57 @@ type BuiltInAuthorizationOptions struct {
 
 func NewBuiltInAuthorizationOptions() *BuiltInAuthorizationOptions {
 	return &BuiltInAuthorizationOptions{
-		Mode: authzmodes.ModeAlwaysAllow,
+		Modes: []string{authzmodes.ModeAlwaysAllow},
 		WebhookCacheAuthorizedTTL:   5 * time.Minute,
 		WebhookCacheUnauthorizedTTL: 30 * time.Second,
 	}
 }
 
 func (s *BuiltInAuthorizationOptions) Validate() []error {
+	if s == nil {
+		return nil
+	}
 	allErrors := []error{}
+
+	if len(s.Modes) == 0 {
+		allErrors = append(allErrors, fmt.Errorf("at least one authorization-mode must be passed"))
+	}
+
+	allowedModes := sets.NewString(authzmodes.AuthorizationModeChoices...)
+	modes := sets.NewString(s.Modes...)
+	for _, mode := range s.Modes {
+		if !allowedModes.Has(mode) {
+			allErrors = append(allErrors, fmt.Errorf("authorization-mode %q is not a valid mode", mode))
+		}
+		if mode == authzmodes.ModeABAC {
+			if s.PolicyFile == "" {
+				allErrors = append(allErrors, fmt.Errorf("authorization-mode ABAC's authorization policy file not passed"))
+			}
+		}
+		if mode == authzmodes.ModeWebhook {
+			if s.WebhookConfigFile == "" {
+				allErrors = append(allErrors, fmt.Errorf("authorization-mode Webhook's authorization config file not passed"))
+			}
+		}
+	}
+
+	if s.PolicyFile != "" && !modes.Has(authzmodes.ModeABAC) {
+		allErrors = append(allErrors, fmt.Errorf("cannot specify --authorization-policy-file without mode ABAC"))
+	}
+
+	if s.WebhookConfigFile != "" && !modes.Has(authzmodes.ModeWebhook) {
+		allErrors = append(allErrors, fmt.Errorf("cannot specify --authorization-webhook-config-file without mode Webhook"))
+	}
+
+	if len(s.Modes) != len(modes.List()) {
+		allErrors = append(allErrors, fmt.Errorf("authorization-mode %q has mode specified more than once", s.Modes))
+	}
+
 	return allErrors
 }
 
 func (s *BuiltInAuthorizationOptions) AddFlags(fs *pflag.FlagSet) {
-	fs.StringVar(&s.Mode, "authorization-mode", s.Mode, ""+
+	fs.StringSliceVar(&s.Modes, "authorization-mode", s.Modes, ""+
 		"Ordered list of plug-ins to do authorization on secure port. Comma-delimited list of: "+
 		strings.Join(authzmodes.AuthorizationModeChoices, ",")+".")
 
@@ -70,17 +110,9 @@ func (s *BuiltInAuthorizationOptions) AddFlags(fs *pflag.FlagSet) {
 		"The duration to cache 'unauthorized' responses from the webhook authorizer.")
 }
 
-func (s *BuiltInAuthorizationOptions) Modes() []string {
-	modes := []string{}
-	if len(s.Mode) > 0 {
-		modes = strings.Split(s.Mode, ",")
-	}
-	return modes
-}
-
 func (s *BuiltInAuthorizationOptions) ToAuthorizationConfig(informerFactory informers.SharedInformerFactory, versionedInformerFactory versionedinformers.SharedInformerFactory) authorizer.AuthorizationConfig {
 	return authorizer.AuthorizationConfig{
-		AuthorizationModes:          s.Modes(),
+		AuthorizationModes:          s.Modes,
 		PolicyFile:                  s.PolicyFile,
 		WebhookConfigFile:           s.WebhookConfigFile,
 		WebhookCacheAuthorizedTTL:   s.WebhookCacheAuthorizedTTL,

pkg/kubeapiserver/options/authorization_test.go

@@ -0,0 +1,104 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package options
+
+import (
+	"testing"
+
+	"k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
+)
+
+func TestAuthzValidate(t *testing.T) {
+	examplePolicyFile := "../../auth/authorizer/abac/example_policy_file.jsonl"
+
+	testCases := []struct {
+		name              string
+		modes             []string
+		policyFile        string
+		webhookConfigFile string
+		expectErr         bool
+	}{
+		{
+			name:      "Unknown modes should return errors",
+			modes:     []string{"DoesNotExist"},
+			expectErr: true,
+		},
+		{
+			name:      "At least one authorizationMode is necessary",
+			modes:     []string{},
+			expectErr: true,
+		},
+		{
+			name:      "ModeAlwaysAllow and ModeAlwaysDeny should return without authorizationPolicyFile",
+			modes:     []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny},
+			expectErr: false,
+		},
+		{
+			name:      "ModeABAC requires a policy file",
+			modes:     []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC},
+			expectErr: true,
+		},
+		{
+			name:              "Authorization Policy file cannot be used without ModeABAC",
+			modes:             []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny},
+			policyFile:        examplePolicyFile,
+			webhookConfigFile: "",
+			expectErr:         true,
+		},
+		{
+			name:              "ModeABAC should not error if a valid policy path is provided",
+			modes:             []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC},
+			policyFile:        examplePolicyFile,
+			webhookConfigFile: "",
+			expectErr:         false,
+		},
+		{
+			name:      "ModeWebhook requires a config file",
+			modes:     []string{modes.ModeWebhook},
+			expectErr: true,
+		},
+		{
+			name:              "Cannot provide webhook config file without ModeWebhook",
+			modes:             []string{modes.ModeAlwaysAllow},
+			webhookConfigFile: "authz_webhook_config.yaml",
+			expectErr:         true,
+		},
+		{
+			name:              "ModeWebhook should not error if a valid config file is provided",
+			modes:             []string{modes.ModeWebhook},
+			webhookConfigFile: "authz_webhook_config.yaml",
+			expectErr:         false,
+		},
+	}
+
+	for _, testcase := range testCases {
+		t.Run(testcase.name, func(t *testing.T) {
+			options := NewBuiltInAuthorizationOptions()
+			options.Modes = testcase.modes
+			options.WebhookConfigFile = testcase.webhookConfigFile
+			options.PolicyFile = testcase.policyFile
+
+			errs := options.Validate()
+			if len(errs) > 0 && !testcase.expectErr {
+				t.Errorf("got unexpected err %v", errs)
+			}
+			if testcase.expectErr && len(errs) == 0 {
+				t.Errorf("should return an error")
+			}
+		})
+	}
+}

test/integration/etcd/etcd_storage_path_test.go

@@ -733,7 +733,7 @@ func startRealMasterOrDie(t *testing.T, certDir string) (*allClient, clientv3.KV
 		kubeAPIServerOptions.Etcd.StorageConfig.ServerList = []string{framework.GetEtcdURL()}
 		kubeAPIServerOptions.Etcd.DefaultStorageMediaType = runtime.ContentTypeJSON // TODO use protobuf?
 		kubeAPIServerOptions.ServiceClusterIPRange = *defaultServiceClusterIPRange
-		kubeAPIServerOptions.Authorization.Mode = "RBAC"
+		kubeAPIServerOptions.Authorization.Modes = []string{"RBAC"}
 		kubeAPIServerOptions.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount"}
 
 		tunneler, proxyTransport, err := app.CreateNodeDialer(kubeAPIServerOptions)

test/integration/examples/apiserver_test.go

@@ -109,7 +109,7 @@ func TestAggregatedAPIServer(t *testing.T) {
 		kubeAPIServerOptions.Authentication.RequestHeader.AllowedNames = []string{"kube-aggregator"}
 		kubeAPIServerOptions.Authentication.RequestHeader.ClientCAFile = proxyCACertFile.Name()
 		kubeAPIServerOptions.Authentication.ClientCert.ClientCA = clientCACertFile.Name()
-		kubeAPIServerOptions.Authorization.Mode = "RBAC"
+		kubeAPIServerOptions.Authorization.Modes = []string{"RBAC"}
 
 		tunneler, proxyTransport, err := app.CreateNodeDialer(kubeAPIServerOptions)
 		if err != nil {