Merge pull request #63879 from lalyos/kubeadm-authz-extra-args-override

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: Fix how kubeadm handles `.AuthorizationModes` and `.APIServerExtraArgs`

**What this PR does / why we need it**:

If _authorization-mode_ is configured as `--apiserver-extra-args` for kubeadm, than 
_authorization-mode_ argument gets duplicated in the static pod manifest file.

```
$ kubeadm alpha phase controlplane apiserver --apiserver-extra-args authorization-mode=AlwaysAllow
$ grep authorization-mode /etc/kubernetes/manifests/kube-apiserver.yaml
     - --authorization-mode=AlwaysAllow
    - --authorization-mode=Node,RBAC
```

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Release note**:
```release-note
NONE
```

/sig cluster-lifecycle
/assign @fabriziopandini
pull/8/head
Kubernetes Submit Queue 2018-05-20 13:05:10 -07:00 committed by GitHub
commit 8ea1d92d73
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 227 additions and 76 deletions

View File

@ -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",
],
)

View File

@ -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
}

View File

@ -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)
}
})
}
}