From 28327e001250f5260a09d4c85e853a64e703213c Mon Sep 17 00:00:00 2001
From: Noel Georgi <git@frezbo.dev>
Date: Tue, 14 Jan 2025 10:22:33 +0530
Subject: [PATCH] fix: kube-apiserver authorizers order

Fixes handling of `kube-apiserver` authorization config authorizers.
order.

Fixes: #10110

Signed-off-by: Noel Georgi <git@frezbo.dev>
(cherry picked from commit e41a995253428dde437eecec52cabfb4c80f90ea)
---
 hack/release.toml                             | 30 +++++++
 .../pkg/controllers/k8s/control_plane.go      | 25 ++++--
 .../pkg/controllers/k8s/control_plane_test.go | 90 ++++++++++++++++++-
 3 files changed, 137 insertions(+), 8 deletions(-)

diff --git a/hack/release.toml b/hack/release.toml
index ceacf4560a..3ad472a1bc 100644
--- a/hack/release.toml
+++ b/hack/release.toml
@@ -22,6 +22,36 @@ preface = """
 * runc: 1.2.4
 
 Talos is built with Go 1.23.4.
+"""
+
+    [notes.kube-apiserver-authorization-config]
+        title = "kube-apiserver Authorization Config"
+        description = """\
+When using `.cluster.apiServer.authorizationConfig` the user provided order for the authorizers is honoured and `Node` and `RBAC` authorizers are always added to the end if not explicitly specified.
+
+Eg: If user provides only `Webhook` authorizer, the final order will be `Webhook`, `Node`, `RBAC`.
+
+To provide a specific order for `Node` or `RBAC` explicitly, user can provide the authorizer in the order they want.
+
+Eg:
+
+```yaml
+cluster:
+  apiServer:
+    authorizationConfig:
+      - type: Node
+        name: Node
+      - type: Webhook
+        name: Webhook
+        webhook:
+          connectionInfo:
+            type: InClusterConfig
+        ...
+      - type: RBAC
+        name: rbac
+```
+
+Usage of `authorization-mode` CLI argument will not support this form of customization.
 """
 
 [make_deps]
diff --git a/internal/app/machined/pkg/controllers/k8s/control_plane.go b/internal/app/machined/pkg/controllers/k8s/control_plane.go
index 595788ab1e..8d47a78f72 100644
--- a/internal/app/machined/pkg/controllers/k8s/control_plane.go
+++ b/internal/app/machined/pkg/controllers/k8s/control_plane.go
@@ -131,11 +131,6 @@ func NewControlPlaneAuthorizationController() *ControlPlaneAuthorizationControll
 				var authorizers []k8s.AuthorizationAuthorizersSpec
 
 				for _, authorizer := range cfgProvider.Cluster().APIServer().AuthorizationConfig() {
-					// skip Node and RBAC authorizers as we add them by default later on.
-					if authorizer.Type() == "Node" || authorizer.Type() == "RBAC" {
-						continue
-					}
-
 					authorizers = slices.Concat(authorizers, []k8s.AuthorizationAuthorizersSpec{
 						{
 							Type:    authorizer.Type(),
@@ -145,7 +140,25 @@ func NewControlPlaneAuthorizationController() *ControlPlaneAuthorizationControll
 					})
 				}
 
-				res.TypedSpec().Config = slices.Concat(v1alpha1.APIServerDefaultAuthorizationConfigAuthorizers, authorizers)
+				if !slices.ContainsFunc(authorizers, func(a k8s.AuthorizationAuthorizersSpec) bool {
+					return a.Type == "Node"
+				}) {
+					authorizers = slices.Insert(authorizers, 0, k8s.AuthorizationAuthorizersSpec{
+						Type: "Node",
+						Name: "node",
+					})
+				}
+
+				if !slices.ContainsFunc(authorizers, func(a k8s.AuthorizationAuthorizersSpec) bool {
+					return a.Type == "RBAC"
+				}) {
+					authorizers = slices.Insert(authorizers, 1, k8s.AuthorizationAuthorizersSpec{
+						Type: "RBAC",
+						Name: "rbac",
+					})
+				}
+
+				res.TypedSpec().Config = authorizers
 
 				return nil
 			},
diff --git a/internal/app/machined/pkg/controllers/k8s/control_plane_test.go b/internal/app/machined/pkg/controllers/k8s/control_plane_test.go
index 0d9a11b232..c870c70c9e 100644
--- a/internal/app/machined/pkg/controllers/k8s/control_plane_test.go
+++ b/internal/app/machined/pkg/controllers/k8s/control_plane_test.go
@@ -270,10 +270,88 @@ func (suite *K8sControlPlaneSuite) TestReconcileAdditionalAuthorizationConfigAut
 									},
 								},
 							},
+							{
+								AuthorizerType: "Node",
+								AuthorizerName: "bar",
+							},
+						},
+					},
+				},
+			},
+		),
+	)
+
+	suite.setupMachine(cfg)
+
+	expectedAuthorizers := []k8s.AuthorizationAuthorizersSpec{
+		{
+			Type: "RBAC",
+			Name: "foo",
+		},
+		{
+			Type: "Webhook",
+			Name: "webhook",
+			Webhook: map[string]any{
+				"timeout":                    "3s",
+				"subjectAccessReviewVersion": "v1",
+				"matchConditionSubjectAccessReviewVersion": "v1",
+				"failurePolicy": "NoOpinion",
+				"connectionInfo": map[string]any{
+					"type": "InClusterConfig",
+				},
+			},
+		},
+		{
+			Type: "Node",
+			Name: "bar",
+		},
+	}
+
+	rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []resource.ID{k8s.AuthorizationConfigID},
+		func(authorizationConfig *k8s.AuthorizationConfig, assert *assert.Assertions) {
+			assert.Equal(expectedAuthorizers, authorizationConfig.TypedSpec().Config)
+		},
+	)
+}
+
+func (suite *K8sControlPlaneSuite) TestReconcileAdditionalAuthorizationConfigAuthorizersWithOnlyNodeSet() {
+	u, err := url.Parse("https://foo:6443")
+	suite.Require().NoError(err)
+
+	cfg := config.NewMachineConfig(
+		container.NewV1Alpha1(
+			&v1alpha1.Config{
+				ConfigVersion: "v1alpha1",
+				MachineConfig: &v1alpha1.MachineConfig{
+					MachineType: "controlplane",
+				},
+				ClusterConfig: &v1alpha1.ClusterConfig{
+					ControlPlane: &v1alpha1.ControlPlaneConfig{
+						Endpoint: &v1alpha1.Endpoint{
+							URL: u,
+						},
+					},
+					APIServerConfig: &v1alpha1.APIServerConfig{
+						AuthorizationConfigConfig: []*v1alpha1.AuthorizationConfigAuthorizerConfig{
 							{
 								AuthorizerType: "Node",
 								AuthorizerName: "foo",
 							},
+							{
+								AuthorizerType: "Webhook",
+								AuthorizerName: "webhook",
+								AuthorizerWebhook: v1alpha1.Unstructured{
+									Object: map[string]any{
+										"timeout":                    "3s",
+										"subjectAccessReviewVersion": "v1",
+										"matchConditionSubjectAccessReviewVersion": "v1",
+										"failurePolicy": "NoOpinion",
+										"connectionInfo": map[string]any{
+											"type": "InClusterConfig",
+										},
+									},
+								},
+							},
 						},
 					},
 				},
@@ -283,7 +361,15 @@ func (suite *K8sControlPlaneSuite) TestReconcileAdditionalAuthorizationConfigAut
 
 	suite.setupMachine(cfg)
 
-	expectedAuthorizers := slices.Concat(v1alpha1.APIServerDefaultAuthorizationConfigAuthorizers, []k8s.AuthorizationAuthorizersSpec{
+	expectedAuthorizers := []k8s.AuthorizationAuthorizersSpec{
+		{
+			Type: "Node",
+			Name: "foo",
+		},
+		{
+			Type: "RBAC",
+			Name: "rbac",
+		},
 		{
 			Type: "Webhook",
 			Name: "webhook",
@@ -297,7 +383,7 @@ func (suite *K8sControlPlaneSuite) TestReconcileAdditionalAuthorizationConfigAut
 				},
 			},
 		},
-	})
+	}
 
 	rtestutils.AssertResources(suite.Ctx(), suite.T(), suite.State(), []resource.ID{k8s.AuthorizationConfigID},
 		func(authorizationConfig *k8s.AuthorizationConfig, assert *assert.Assertions) {