From d95c7779de88c08cc4c1db334233121c1ee9142a Mon Sep 17 00:00:00 2001 From: lalyos Date: Tue, 15 May 2018 19:20:04 +0200 Subject: [PATCH] kubeadm: APIServerExtraArgs should override defaultArguments --- cmd/kubeadm/app/phases/controlplane/BUILD | 2 + .../app/phases/controlplane/manifests.go | 46 ++-- .../app/phases/controlplane/manifests_test.go | 255 ++++++++++++++---- 3 files changed, 227 insertions(+), 76 deletions(-) diff --git a/cmd/kubeadm/app/phases/controlplane/BUILD b/cmd/kubeadm/app/phases/controlplane/BUILD index 3b6d5a95f34..6e826f4fd59 100644 --- a/cmd/kubeadm/app/phases/controlplane/BUILD +++ b/cmd/kubeadm/app/phases/controlplane/BUILD @@ -19,9 +19,11 @@ go_test( "//cmd/kubeadm/app/features:go_default_library", "//cmd/kubeadm/app/phases/certs:go_default_library", "//cmd/kubeadm/test:go_default_library", + "//pkg/kubeapiserver/authorizer/modes:go_default_library", "//pkg/util/pointer:go_default_library", "//pkg/util/version:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], ) diff --git a/cmd/kubeadm/app/phases/controlplane/manifests.go b/cmd/kubeadm/app/phases/controlplane/manifests.go index 3711b7797cd..5fd0aaea091 100644 --- a/cmd/kubeadm/app/phases/controlplane/manifests.go +++ b/cmd/kubeadm/app/phases/controlplane/manifests.go @@ -27,7 +27,6 @@ import ( "github.com/golang/glog" "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/sets" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapiv1alpha2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha2" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" @@ -217,13 +216,32 @@ func getAPIServerCommand(cfg *kubeadmapi.MasterConfiguration) []string { defaultArguments["audit-log-maxage"] = fmt.Sprintf("%d", *cfg.AuditPolicyConfiguration.LogMaxAge) } } - + if cfg.APIServerExtraArgs == nil { + cfg.APIServerExtraArgs = map[string]string{} + } + cfg.APIServerExtraArgs["authorization-mode"] = getAuthzModes(cfg.APIServerExtraArgs["authorization-mode"]) command = append(command, kubeadmutil.BuildArgumentListFromMap(defaultArguments, cfg.APIServerExtraArgs)...) - command = append(command, getAuthzParameters(cfg.AuthorizationModes)...) return command } +// getAuthzModes gets the authorization-related parameters to the api server +// Node,RBAC should be fixed in this order at the beginning +// AlwaysAllow and AlwaysDeny is ignored as they are only for testing +func getAuthzModes(authzModeExtraArgs string) string { + modes := []string{ + authzmodes.ModeNode, + authzmodes.ModeRBAC, + } + if strings.Contains(authzModeExtraArgs, authzmodes.ModeABAC) { + modes = append(modes, authzmodes.ModeABAC) + } + if strings.Contains(authzModeExtraArgs, authzmodes.ModeWebhook) { + modes = append(modes, authzmodes.ModeWebhook) + } + return strings.Join(modes, ",") +} + // calcNodeCidrSize determines the size of the subnets used on each node, based // on the pod subnet provided. For IPv4, we assume that the pod subnet will // be /16 and use /24. If the pod subnet cannot be parsed, the IPv4 value will @@ -335,25 +353,3 @@ func getProxyEnvVars() []v1.EnvVar { } return envs } - -// getAuthzParameters gets the authorization-related parameters to the api server -// At this point, we can assume the list of authorization modes is valid (due to that it has been validated in the API machinery code already) -// If the list is empty; it's defaulted (mostly for unit testing) -func getAuthzParameters(modes []string) []string { - command := []string{} - strset := sets.NewString(modes...) - - if len(modes) == 0 { - return []string{fmt.Sprintf("--authorization-mode=%s", kubeadmapiv1alpha2.DefaultAuthorizationModes)} - } - - if strset.Has(authzmodes.ModeABAC) { - command = append(command, "--authorization-policy-file="+kubeadmconstants.AuthorizationPolicyPath) - } - if strset.Has(authzmodes.ModeWebhook) { - command = append(command, "--authorization-webhook-config-file="+kubeadmconstants.AuthorizationWebhookConfigPath) - } - - command = append(command, "--authorization-mode="+strings.Join(modes, ",")) - return command -} diff --git a/cmd/kubeadm/app/phases/controlplane/manifests_test.go b/cmd/kubeadm/app/phases/controlplane/manifests_test.go index 16eddda0225..ea26780ff50 100644 --- a/cmd/kubeadm/app/phases/controlplane/manifests_test.go +++ b/cmd/kubeadm/app/phases/controlplane/manifests_test.go @@ -22,12 +22,15 @@ import ( "path/filepath" "reflect" "sort" + "strings" "testing" + "k8s.io/apimachinery/pkg/util/sets" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/features" "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" + authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes" "k8s.io/kubernetes/pkg/util/version" testutil "k8s.io/kubernetes/cmd/kubeadm/test" @@ -504,6 +507,126 @@ func TestGetAPIServerCommand(t *testing.T) { "--audit-log-maxage=2", }, }, + { + name: "authorization-mode extra-args ABAC", + cfg: &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{BindPort: 123, AdvertiseAddress: "1.2.3.4"}, + Networking: kubeadmapi.Networking{ServiceSubnet: "bar"}, + CertificatesDir: testCertsDir, + APIServerExtraArgs: map[string]string{ + "authorization-mode": authzmodes.ModeABAC, + }, + }, + expected: []string{ + "kube-apiserver", + "--insecure-port=0", + "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,NodeRestriction,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota", + "--service-cluster-ip-range=bar", + "--service-account-key-file=" + testCertsDir + "/sa.pub", + "--client-ca-file=" + testCertsDir + "/ca.crt", + "--tls-cert-file=" + testCertsDir + "/apiserver.crt", + "--tls-private-key-file=" + testCertsDir + "/apiserver.key", + "--kubelet-client-certificate=" + testCertsDir + "/apiserver-kubelet-client.crt", + "--kubelet-client-key=" + testCertsDir + "/apiserver-kubelet-client.key", + "--enable-bootstrap-token-auth=true", + "--secure-port=123", + "--allow-privileged=true", + "--kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname", + "--proxy-client-cert-file=/var/lib/certs/front-proxy-client.crt", + "--proxy-client-key-file=/var/lib/certs/front-proxy-client.key", + "--requestheader-username-headers=X-Remote-User", + "--requestheader-group-headers=X-Remote-Group", + "--requestheader-extra-headers-prefix=X-Remote-Extra-", + "--requestheader-client-ca-file=" + testCertsDir + "/front-proxy-ca.crt", + "--requestheader-allowed-names=front-proxy-client", + "--authorization-mode=Node,RBAC,ABAC", + "--advertise-address=1.2.3.4", + "--etcd-servers=https://127.0.0.1:2379", + "--etcd-cafile=" + testCertsDir + "/etcd/ca.crt", + "--etcd-certfile=" + testCertsDir + "/apiserver-etcd-client.crt", + "--etcd-keyfile=" + testCertsDir + "/apiserver-etcd-client.key", + }, + }, + { + name: "insecure-port extra-args", + cfg: &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{BindPort: 123, AdvertiseAddress: "1.2.3.4"}, + Networking: kubeadmapi.Networking{ServiceSubnet: "bar"}, + CertificatesDir: testCertsDir, + APIServerExtraArgs: map[string]string{ + "insecure-port": "1234", + }, + }, + expected: []string{ + "kube-apiserver", + "--insecure-port=1234", + "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,NodeRestriction,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota", + "--service-cluster-ip-range=bar", + "--service-account-key-file=" + testCertsDir + "/sa.pub", + "--client-ca-file=" + testCertsDir + "/ca.crt", + "--tls-cert-file=" + testCertsDir + "/apiserver.crt", + "--tls-private-key-file=" + testCertsDir + "/apiserver.key", + "--kubelet-client-certificate=" + testCertsDir + "/apiserver-kubelet-client.crt", + "--kubelet-client-key=" + testCertsDir + "/apiserver-kubelet-client.key", + "--enable-bootstrap-token-auth=true", + "--secure-port=123", + "--allow-privileged=true", + "--kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname", + "--proxy-client-cert-file=/var/lib/certs/front-proxy-client.crt", + "--proxy-client-key-file=/var/lib/certs/front-proxy-client.key", + "--requestheader-username-headers=X-Remote-User", + "--requestheader-group-headers=X-Remote-Group", + "--requestheader-extra-headers-prefix=X-Remote-Extra-", + "--requestheader-client-ca-file=" + testCertsDir + "/front-proxy-ca.crt", + "--requestheader-allowed-names=front-proxy-client", + "--authorization-mode=Node,RBAC", + "--advertise-address=1.2.3.4", + "--etcd-servers=https://127.0.0.1:2379", + "--etcd-cafile=" + testCertsDir + "/etcd/ca.crt", + "--etcd-certfile=" + testCertsDir + "/apiserver-etcd-client.crt", + "--etcd-keyfile=" + testCertsDir + "/apiserver-etcd-client.key", + }, + }, + { + name: "authorization-mode extra-args Webhook", + cfg: &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{BindPort: 123, AdvertiseAddress: "1.2.3.4"}, + Networking: kubeadmapi.Networking{ServiceSubnet: "bar"}, + CertificatesDir: testCertsDir, + APIServerExtraArgs: map[string]string{ + "authorization-mode": authzmodes.ModeWebhook, + }, + }, + expected: []string{ + "kube-apiserver", + "--insecure-port=0", + "--admission-control=NamespaceLifecycle,LimitRanger,ServiceAccount,DefaultStorageClass,DefaultTolerationSeconds,NodeRestriction,MutatingAdmissionWebhook,ValidatingAdmissionWebhook,ResourceQuota", + "--service-cluster-ip-range=bar", + "--service-account-key-file=" + testCertsDir + "/sa.pub", + "--client-ca-file=" + testCertsDir + "/ca.crt", + "--tls-cert-file=" + testCertsDir + "/apiserver.crt", + "--tls-private-key-file=" + testCertsDir + "/apiserver.key", + "--kubelet-client-certificate=" + testCertsDir + "/apiserver-kubelet-client.crt", + "--kubelet-client-key=" + testCertsDir + "/apiserver-kubelet-client.key", + "--enable-bootstrap-token-auth=true", + "--secure-port=123", + "--allow-privileged=true", + "--kubelet-preferred-address-types=InternalIP,ExternalIP,Hostname", + "--proxy-client-cert-file=/var/lib/certs/front-proxy-client.crt", + "--proxy-client-key-file=/var/lib/certs/front-proxy-client.key", + "--requestheader-username-headers=X-Remote-User", + "--requestheader-group-headers=X-Remote-Group", + "--requestheader-extra-headers-prefix=X-Remote-Extra-", + "--requestheader-client-ca-file=" + testCertsDir + "/front-proxy-ca.crt", + "--requestheader-allowed-names=front-proxy-client", + "--authorization-mode=Node,RBAC,Webhook", + "--advertise-address=1.2.3.4", + "--etcd-servers=https://127.0.0.1:2379", + "--etcd-cafile=" + testCertsDir + "/etcd/ca.crt", + "--etcd-certfile=" + testCertsDir + "/apiserver-etcd-client.crt", + "--etcd-keyfile=" + testCertsDir + "/apiserver-etcd-client.key", + }, + }, } for _, rt := range tests { @@ -512,18 +635,38 @@ func TestGetAPIServerCommand(t *testing.T) { sort.Strings(actual) sort.Strings(rt.expected) if !reflect.DeepEqual(actual, rt.expected) { - t.Errorf("failed getAPIServerCommand:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) + errorDiffArguments(t, rt.name, actual, rt.expected) } }) } } +func errorDiffArguments(t *testing.T, name string, actual, expected []string) { + expectedShort := removeCommon(expected, actual) + actualShort := removeCommon(actual, expected) + t.Errorf( + "[%s] failed getAPIServerCommand:\nexpected:\n%v\nsaw:\n%v"+ + "\nexpectedShort:\n%v\nsawShort:\n%v\n", + name, expected, actual, + expectedShort, actualShort) +} + +// removeCommon removes common items from left list +// makes compairing two cmdline (with lots of arguments) easier +func removeCommon(left, right []string) []string { + origSet := sets.NewString(left...) + origSet.Delete(right...) + return origSet.List() +} + func TestGetControllerManagerCommand(t *testing.T) { var tests = []struct { + name string cfg *kubeadmapi.MasterConfiguration expected []string }{ { + name: "custom certs dir", cfg: &kubeadmapi.MasterConfiguration{ CertificatesDir: testCertsDir, KubernetesVersion: "v1.7.0", @@ -542,6 +685,7 @@ func TestGetControllerManagerCommand(t *testing.T) { }, }, { + name: "custom cloudprovider", cfg: &kubeadmapi.MasterConfiguration{ Networking: kubeadmapi.Networking{PodSubnet: "10.0.1.15/16"}, CertificatesDir: testCertsDir, @@ -564,6 +708,7 @@ func TestGetControllerManagerCommand(t *testing.T) { }, }, { + name: "custom extra-args", cfg: &kubeadmapi.MasterConfiguration{ Networking: kubeadmapi.Networking{PodSubnet: "10.0.1.15/16"}, ControllerManagerExtraArgs: map[string]string{"node-cidr-mask-size": "20"}, @@ -587,6 +732,7 @@ func TestGetControllerManagerCommand(t *testing.T) { }, }, { + name: "custom IPv6 networking", cfg: &kubeadmapi.MasterConfiguration{ Networking: kubeadmapi.Networking{PodSubnet: "2001:db8::/64"}, CertificatesDir: testCertsDir, @@ -615,7 +761,7 @@ func TestGetControllerManagerCommand(t *testing.T) { sort.Strings(actual) sort.Strings(rt.expected) if !reflect.DeepEqual(actual, rt.expected) { - t.Errorf("failed getControllerManagerCommand:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) + errorDiffArguments(t, rt.name, actual, rt.expected) } } } @@ -699,11 +845,13 @@ func TestCalcNodeCidrSize(t *testing.T) { func TestGetControllerManagerCommandExternalCA(t *testing.T) { tests := []struct { + name string cfg *kubeadmapi.MasterConfiguration caKeyPresent bool expectedArgFunc func(dir string) []string }{ { + name: "caKeyPresent-false", cfg: &kubeadmapi.MasterConfiguration{ KubernetesVersion: "v1.7.0", API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4"}, @@ -727,6 +875,7 @@ func TestGetControllerManagerCommandExternalCA(t *testing.T) { }, }, { + name: "caKeyPresent true", cfg: &kubeadmapi.MasterConfiguration{ KubernetesVersion: "v1.7.0", API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4"}, @@ -776,18 +925,20 @@ func TestGetControllerManagerCommandExternalCA(t *testing.T) { sort.Strings(actual) sort.Strings(expected) if !reflect.DeepEqual(actual, expected) { - t.Errorf("failed getControllerManagerCommand:\nexpected:\n%v\nsaw:\n%v", expected, actual) + errorDiffArguments(t, test.name, actual, expected) } } } func TestGetSchedulerCommand(t *testing.T) { var tests = []struct { + name string cfg *kubeadmapi.MasterConfiguration expected []string }{ { - cfg: &kubeadmapi.MasterConfiguration{}, + name: "scheduler defaults", + cfg: &kubeadmapi.MasterConfiguration{}, expected: []string{ "kube-scheduler", "--address=127.0.0.1", @@ -802,79 +953,81 @@ func TestGetSchedulerCommand(t *testing.T) { sort.Strings(actual) sort.Strings(rt.expected) if !reflect.DeepEqual(actual, rt.expected) { - t.Errorf("failed getSchedulerCommand:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) + errorDiffArguments(t, rt.name, actual, rt.expected) } } } -func TestGetAuthzParameters(t *testing.T) { +func TestGetAuthzModes(t *testing.T) { var tests = []struct { + name string authMode []string - expected []string + expected string }{ { + name: "default if empty", authMode: []string{}, - expected: []string{ - "--authorization-mode=Node,RBAC", - }, + expected: "Node,RBAC", }, { - authMode: []string{"RBAC"}, - expected: []string{ - "--authorization-mode=RBAC", - }, + name: "add missing Node", + authMode: []string{authzmodes.ModeRBAC}, + expected: "Node,RBAC", }, { - authMode: []string{"AlwaysAllow"}, - expected: []string{ - "--authorization-mode=AlwaysAllow", - }, + name: "add missing RBAC", + authMode: []string{authzmodes.ModeNode}, + expected: "Node,RBAC", }, { - authMode: []string{"AlwaysDeny"}, - expected: []string{ - "--authorization-mode=AlwaysDeny", - }, + name: "add defaults to ABAC", + authMode: []string{authzmodes.ModeABAC}, + expected: "Node,RBAC,ABAC", }, { - authMode: []string{"ABAC"}, - expected: []string{ - "--authorization-mode=ABAC", - "--authorization-policy-file=/etc/kubernetes/abac_policy.json", - }, + name: "add defaults to RBAC+Webhook", + authMode: []string{authzmodes.ModeRBAC, authzmodes.ModeWebhook}, + expected: "Node,RBAC,Webhook", }, { - authMode: []string{"ABAC", "Webhook"}, - expected: []string{ - "--authorization-mode=ABAC,Webhook", - "--authorization-policy-file=/etc/kubernetes/abac_policy.json", - "--authorization-webhook-config-file=/etc/kubernetes/webhook_authz.conf", - }, + name: "add default to Webhook", + authMode: []string{authzmodes.ModeWebhook}, + expected: "Node,RBAC,Webhook", }, { - authMode: []string{"ABAC", "RBAC", "Webhook"}, - expected: []string{ - "--authorization-mode=ABAC,RBAC,Webhook", - "--authorization-policy-file=/etc/kubernetes/abac_policy.json", - "--authorization-webhook-config-file=/etc/kubernetes/webhook_authz.conf", - }, + name: "AlwaysAllow ignored", + authMode: []string{authzmodes.ModeAlwaysAllow}, + expected: "Node,RBAC", }, { - authMode: []string{"Node", "RBAC", "Webhook", "ABAC"}, - expected: []string{ - "--authorization-mode=Node,RBAC,Webhook,ABAC", - "--authorization-policy-file=/etc/kubernetes/abac_policy.json", - "--authorization-webhook-config-file=/etc/kubernetes/webhook_authz.conf", - }, + name: "AlwaysDeny ignored", + authMode: []string{authzmodes.ModeAlwaysDeny}, + expected: "Node,RBAC", + }, + { + name: "Unspecified ignored", + authMode: []string{"FooAuthzMode"}, + expected: "Node,RBAC", + }, + { + name: "Multiple ignored", + authMode: []string{authzmodes.ModeAlwaysAllow, authzmodes.ModeAlwaysDeny, "foo"}, + expected: "Node,RBAC", + }, + { + name: "all", + authMode: []string{authzmodes.ModeNode, authzmodes.ModeRBAC, authzmodes.ModeWebhook, authzmodes.ModeABAC}, + expected: "Node,RBAC,ABAC,Webhook", }, } for _, rt := range tests { - actual := getAuthzParameters(rt.authMode) - sort.Strings(actual) - sort.Strings(rt.expected) - if !reflect.DeepEqual(actual, rt.expected) { - t.Errorf("failed getAuthzParameters:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) - } + + t.Run(rt.name, func(t *testing.T) { + actual := getAuthzModes(strings.Join(rt.authMode, ",")) + if actual != rt.expected { + t.Errorf("failed getAuthzParameters:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual) + } + }) } }