Merge pull request #82616 from ghouscht/kubeadm-issue-1787

kubeadm: preserve order of user specified apiserver authorization-mode

cmd/kubeadm/app/phases/controlplane/manifests.go

@@ -192,20 +192,52 @@ func getAPIServerCommand(cfg *kubeadmapi.ClusterConfiguration, localAPIEndpoint
 }
 
 // getAuthzModes gets the authorization-related parameters to the api server
-// Node,RBAC should be fixed in this order at the beginning
+// Node,RBAC is the default mode if nothing is passed to kubeadm. User provided modes override
-// AlwaysAllow and AlwaysDeny is ignored as they are only for testing
+// the default.
 func getAuthzModes(authzModeExtraArgs string) string {
-	modes := []string{
+	defaultMode := []string{
 		kubeadmconstants.ModeNode,
 		kubeadmconstants.ModeRBAC,
 	}
-	if strings.Contains(authzModeExtraArgs, kubeadmconstants.ModeABAC) {
+
-		modes = append(modes, kubeadmconstants.ModeABAC)
+	if len(authzModeExtraArgs) > 0 {
+		mode := []string{}
+		for _, requested := range strings.Split(authzModeExtraArgs, ",") {
+			if isValidAuthzMode(requested) {
+				mode = append(mode, requested)
+			} else {
+				klog.Warningf("ignoring unknown kube-apiserver authorization-mode %q", requested)
 			}
-	if strings.Contains(authzModeExtraArgs, kubeadmconstants.ModeWebhook) {
-		modes = append(modes, kubeadmconstants.ModeWebhook)
 		}
-	return strings.Join(modes, ",")
+
+		// only return the user provided mode if at least one was valid
+		if len(mode) > 0 {
+			klog.Warningf("the default kube-apiserver authorization-mode is %q; using %q",
+				strings.Join(defaultMode, ","),
+				strings.Join(mode, ","),
+			)
+			return strings.Join(mode, ",")
+		}
+	}
+	return strings.Join(defaultMode, ",")
+}
+
+func isValidAuthzMode(authzMode string) bool {
+	allModes := []string{
+		kubeadmconstants.ModeNode,
+		kubeadmconstants.ModeRBAC,
+		kubeadmconstants.ModeWebhook,
+		kubeadmconstants.ModeABAC,
+		kubeadmconstants.ModeAlwaysAllow,
+		kubeadmconstants.ModeAlwaysDeny,
+	}
+
+	for _, mode := range allModes {
+		if authzMode == mode {
+			return true
+		}
+	}
+	return false
 }
 
 // calcNodeCidrSize determines the size of the subnets used on each node, based

cmd/kubeadm/app/phases/controlplane/manifests_test.go

@@ -440,7 +440,7 @@ func TestGetAPIServerCommand(t *testing.T) {
 				"--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",
+				"--authorization-mode=ABAC",
 				"--advertise-address=1.2.3.4",
 				fmt.Sprintf("--etcd-servers=https://127.0.0.1:%d", kubeadmconstants.EtcdListenClientPort),
 				"--etcd-cafile=" + testCertsDir + "/etcd/ca.crt",
@@ -500,7 +500,11 @@ func TestGetAPIServerCommand(t *testing.T) {
 				APIServer: kubeadmapi.APIServer{
 					ControlPlaneComponent: kubeadmapi.ControlPlaneComponent{
 						ExtraArgs: map[string]string{
-							"authorization-mode": kubeadmconstants.ModeWebhook,
+							"authorization-mode": strings.Join([]string{
+								kubeadmconstants.ModeNode,
+								kubeadmconstants.ModeRBAC,
+								kubeadmconstants.ModeWebhook,
+							}, ","),
 						},
 					},
 				},
@@ -950,54 +954,29 @@ func TestGetAuthzModes(t *testing.T) {
 			expected: "Node,RBAC",
 		},
 		{
-			name:     "add missing Node",
+			name:     "default non empty",
-			authMode: []string{kubeadmconstants.ModeRBAC},
+			authMode: []string{kubeadmconstants.ModeNode, kubeadmconstants.ModeRBAC},
 			expected: "Node,RBAC",
 		},
 		{
-			name:     "add missing RBAC",
+			name:     "single unspecified returning default",
-			authMode: []string{kubeadmconstants.ModeNode},
-			expected: "Node,RBAC",
-		},
-		{
-			name:     "add defaults to ABAC",
-			authMode: []string{kubeadmconstants.ModeABAC},
-			expected: "Node,RBAC,ABAC",
-		},
-		{
-			name:     "add defaults to RBAC+Webhook",
-			authMode: []string{kubeadmconstants.ModeRBAC, kubeadmconstants.ModeWebhook},
-			expected: "Node,RBAC,Webhook",
-		},
-		{
-			name:     "add default to Webhook",
-			authMode: []string{kubeadmconstants.ModeWebhook},
-			expected: "Node,RBAC,Webhook",
-		},
-		{
-			name:     "AlwaysAllow ignored",
-			authMode: []string{kubeadmconstants.ModeAlwaysAllow},
-			expected: "Node,RBAC",
-		},
-		{
-			name:     "AlwaysDeny ignored",
-			authMode: []string{kubeadmconstants.ModeAlwaysDeny},
-			expected: "Node,RBAC",
-		},
-		{
-			name:     "Unspecified ignored",
 			authMode: []string{"FooAuthzMode"},
 			expected: "Node,RBAC",
 		},
 		{
-			name:     "Multiple ignored",
+			name:     "multiple ignored",
-			authMode: []string{kubeadmconstants.ModeAlwaysAllow, kubeadmconstants.ModeAlwaysDeny, "foo"},
+			authMode: []string{kubeadmconstants.ModeNode, "foo", kubeadmconstants.ModeRBAC, "bar"},
 			expected: "Node,RBAC",
 		},
 		{
-			name:     "all",
+			name:     "single mode",
-			authMode: []string{kubeadmconstants.ModeNode, kubeadmconstants.ModeRBAC, kubeadmconstants.ModeWebhook, kubeadmconstants.ModeABAC},
+			authMode: []string{kubeadmconstants.ModeAlwaysDeny},
-			expected: "Node,RBAC,ABAC,Webhook",
+			expected: "AlwaysDeny",
+		},
+		{
+			name:     "multiple special order",
+			authMode: []string{kubeadmconstants.ModeNode, kubeadmconstants.ModeWebhook, kubeadmconstants.ModeRBAC, kubeadmconstants.ModeABAC},
+			expected: "Node,Webhook,RBAC,ABAC",
 		},
 	}
 
@@ -1005,7 +984,52 @@ func TestGetAuthzModes(t *testing.T) {
 		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)
+				t.Errorf("failed getAuthzModes:\nexpected:\n%v\nsaw:\n%v", rt.expected, actual)
+			}
+		})
+	}
+}
+
+func TestIsValidAuthzMode(t *testing.T) {
+	var tests = []struct {
+		mode  string
+		valid bool
+	}{
+		{
+			mode:  "Node",
+			valid: true,
+		},
+		{
+			mode:  "RBAC",
+			valid: true,
+		},
+		{
+			mode:  "ABAC",
+			valid: true,
+		},
+		{
+			mode:  "AlwaysAllow",
+			valid: true,
+		},
+		{
+			mode:  "Webhook",
+			valid: true,
+		},
+		{
+			mode:  "AlwaysDeny",
+			valid: true,
+		},
+		{
+			mode:  "Foo",
+			valid: false,
+		},
+	}
+
+	for _, rt := range tests {
+		t.Run(rt.mode, func(t *testing.T) {
+			isValid := isValidAuthzMode(rt.mode)
+			if isValid != rt.valid {
+				t.Errorf("failed isValidAuthzMode:\nexpected:\n%v\nsaw:\n%v", rt.valid, isValid)
 			}
 		})
 	}