From 30ed50d32e3557f9d40225e2a179a618ceb1161b Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Thu, 19 Oct 2023 20:35:40 +0300 Subject: [PATCH] kubeadm: make super-admin.conf changes in app/phases - Register the new file in /certs/renewal, so that the file is renewed if present. If not present the common message "MISSING" is shown. Same for other certs/kubeconfig files. - In /kubeconfig, update the spec for admin.conf to use the "kubeadm:cluster-admins" Group. A new spec is added for the "super-admin.conf" file that uses the "system:masters" Group. - Add a new function EnsureAdminClusterRoleBinding() that includes logic to ensure that admin.conf contains a User that is properly bound on the "cluster-admin" built-in ClusterRole. This requires bootstrapping using the "system:masters" containing "super-admin.conf". Add detailed unit tests for this new logic. - In /upgrade#PerformPostUpgradeTasks() add logic to create the "admin.conf" and "super-admin.conf" with the new, updated specs. Add detailed unit tests for this new logic. - In /upgrade#StaticPodControlPlane() ensure that renewal of "super-admin.conf" is performed if the file exists. Update unit tests. --- .../app/phases/certs/renewal/manager.go | 4 + .../app/phases/certs/renewal/manager_test.go | 4 +- .../app/phases/kubeconfig/kubeconfig.go | 154 ++++++++++++ .../app/phases/kubeconfig/kubeconfig_test.go | 219 +++++++++++++++++- cmd/kubeadm/app/phases/upgrade/postupgrade.go | 68 ++++++ .../app/phases/upgrade/postupgrade_test.go | 110 +++++++++ cmd/kubeadm/app/phases/upgrade/staticpods.go | 14 ++ .../app/phases/upgrade/staticpods_test.go | 48 ++-- 8 files changed, 598 insertions(+), 23 deletions(-) diff --git a/cmd/kubeadm/app/phases/certs/renewal/manager.go b/cmd/kubeadm/app/phases/certs/renewal/manager.go index d7c420c45fd..27b350694b1 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/manager.go +++ b/cmd/kubeadm/app/phases/certs/renewal/manager.go @@ -141,6 +141,10 @@ func NewManager(cfg *kubeadmapi.ClusterConfiguration, kubernetesDir string) (*Ma longName: "certificate embedded in the kubeconfig file for the admin to use and for kubeadm itself", fileName: kubeadmconstants.AdminKubeConfigFileName, }, + { + longName: "certificate embedded in the kubeconfig file for the super-admin", + fileName: kubeadmconstants.SuperAdminKubeConfigFileName, + }, { longName: "certificate embedded in the kubeconfig file for the controller manager to use", fileName: kubeadmconstants.ControllerManagerKubeConfigFileName, diff --git a/cmd/kubeadm/app/phases/certs/renewal/manager_test.go b/cmd/kubeadm/app/phases/certs/renewal/manager_test.go index ca9640049e5..9163d2d1517 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/manager_test.go +++ b/cmd/kubeadm/app/phases/certs/renewal/manager_test.go @@ -64,7 +64,7 @@ func TestNewManager(t *testing.T) { { name: "cluster with local etcd", cfg: &kubeadmapi.ClusterConfiguration{}, - expectedCertificates: 10, //[admin apiserver apiserver-etcd-client apiserver-kubelet-client controller-manager etcd/healthcheck-client etcd/peer etcd/server front-proxy-client scheduler] + expectedCertificates: 11, // [admin super-admin apiserver apiserver-etcd-client apiserver-kubelet-client controller-manager etcd/healthcheck-client etcd/peer etcd/server front-proxy-client scheduler] }, { name: "cluster with external etcd", @@ -73,7 +73,7 @@ func TestNewManager(t *testing.T) { External: &kubeadmapi.ExternalEtcd{}, }, }, - expectedCertificates: 6, // [admin apiserver apiserver-kubelet-client controller-manager front-proxy-client scheduler] + expectedCertificates: 7, // [admin super-admin apiserver apiserver-kubelet-client controller-manager front-proxy-client scheduler] }, } diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go index af25d7cac3f..0798befe929 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go @@ -18,6 +18,7 @@ package kubeconfig import ( "bytes" + "context" "crypto" "crypto/x509" "fmt" @@ -28,6 +29,11 @@ import ( "github.com/pkg/errors" + rbac "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" certutil "k8s.io/client-go/util/cert" @@ -97,6 +103,9 @@ func CreateJoinControlPlaneKubeConfigFiles(outDir string, cfg *kubeadmapi.InitCo return nil } +// CreateKubeConfigFileFunc defines a function type used for creating kubeconfig files. +type CreateKubeConfigFileFunc func(string, string, *kubeadmapi.InitConfiguration) error + // CreateKubeConfigFile creates a kubeconfig file. // If the kubeconfig file already exists, it is used only if evaluated equal; otherwise an error is returned. func CreateKubeConfigFile(kubeConfigFileName string, outDir string, cfg *kubeadmapi.InitConfiguration) error { @@ -407,6 +416,7 @@ func ValidateKubeconfigsForExternalCA(outDir string, cfg *kubeadmapi.InitConfigu validationConfigCPE := kubeconfigutil.CreateBasic(controlPlaneEndpoint, "dummy", "dummy", pkiutil.EncodeCertPEM(caCert)) kubeConfigFileNamesCPE := []string{ kubeadmconstants.AdminKubeConfigFileName, + kubeadmconstants.SuperAdminKubeConfigFileName, kubeadmconstants.KubeletKubeConfigFileName, } @@ -433,6 +443,13 @@ func getKubeConfigSpecsBase(cfg *kubeadmapi.InitConfiguration) (map[string]*kube kubeadmconstants.AdminKubeConfigFileName: { APIServer: controlPlaneEndpoint, ClientName: "kubernetes-admin", + ClientCertAuth: &clientCertAuth{ + Organizations: []string{kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding}, + }, + }, + kubeadmconstants.SuperAdminKubeConfigFileName: { + APIServer: controlPlaneEndpoint, + ClientName: "kubernetes-super-admin", ClientCertAuth: &clientCertAuth{ Organizations: []string{kubeadmconstants.SystemPrivilegedGroup}, }, @@ -541,3 +558,140 @@ func CreateDefaultKubeConfigsAndCSRFiles(out io.Writer, kubeConfigDir string, ku } return nil } + +// EnsureRBACFunc defines a function type that can be passed to EnsureAdminClusterRoleBinding(). +type EnsureRBACFunc func(context.Context, clientset.Interface, clientset.Interface, time.Duration, time.Duration) (clientset.Interface, error) + +// EnsureAdminClusterRoleBinding constructs a client from admin.conf and optionally +// constructs a client from super-admin.conf if the file exists. It then proceeds +// to pass the clients to EnsureAdminClusterRoleBindingImpl. The function returns a +// usable client from admin.conf with RBAC properly constructed or an error. +func EnsureAdminClusterRoleBinding(outDir string, ensureRBACFunc EnsureRBACFunc) (clientset.Interface, error) { + var ( + err error + adminClient, superAdminClient clientset.Interface + ) + + // Create a client from admin.conf. + adminClient, err = kubeconfigutil.ClientSetFromFile(filepath.Join(outDir, kubeadmconstants.AdminKubeConfigFileName)) + if err != nil { + return nil, err + } + + // Create a client from super-admin.conf. + superAdminPath := filepath.Join(outDir, kubeadmconstants.SuperAdminKubeConfigFileName) + if _, err := os.Stat(superAdminPath); err == nil { + superAdminClient, err = kubeconfigutil.ClientSetFromFile(superAdminPath) + if err != nil { + return nil, err + } + } + + if ensureRBACFunc == nil { + ensureRBACFunc = EnsureAdminClusterRoleBindingImpl + } + + ctx := context.Background() + return ensureRBACFunc( + ctx, adminClient, superAdminClient, kubeadmconstants.APICallRetryInterval, kubeadmconstants.APICallWithWriteTimeout) +} + +// EnsureAdminClusterRoleBindingImpl first attempts to see if the ClusterRoleBinding +// kubeadm:cluster-admins exists by using adminClient. If it already exists, +// it would mean the adminClient is usable. If it does not, attempt to create +// the ClusterRoleBinding by using superAdminClient. +func EnsureAdminClusterRoleBindingImpl(ctx context.Context, adminClient, superAdminClient clientset.Interface, + retryInterval, retryTimeout time.Duration) (clientset.Interface, error) { + + klog.V(1).Infof("ensuring that the ClusterRoleBinding for the %s Group exists", + kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding) + + var ( + err, lastError error + crbResult *rbac.ClusterRoleBinding + clusterRoleBinding = &rbac.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding, + }, + RoleRef: rbac.RoleRef{ + APIGroup: rbac.GroupName, + Kind: "ClusterRole", + Name: "cluster-admin", + }, + Subjects: []rbac.Subject{ + { + Kind: rbac.GroupKind, + Name: kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding, + }, + }, + } + ) + + // First try to create the CRB with the admin.conf client. If the admin.conf contains a User bound + // to the built-in super-user group, this will pass. In all other cases an error will be returned. + // The poll here is required to ensure the API server is reachable during "kubeadm init" workflows. + err = wait.PollUntilContextTimeout( + ctx, + retryInterval, + retryTimeout, + true, func(ctx context.Context) (bool, error) { + if crbResult, err = adminClient.RbacV1().ClusterRoleBindings().Create( + ctx, + clusterRoleBinding, + metav1.CreateOptions{}, + ); err != nil { + if apierrors.IsForbidden(err) { + // If it encounters a forbidden error this means that the API server was reached + // but the CRB is missing - i.e. the admin.conf user does not have permissions + // to create its own permission RBAC yet. + // + // When a "create" call is made, but the resource is forbidden, a non-nil + // CRB will still be returned. Return true here, but update "crbResult" to nil, + // to ensure that the process continues with super-admin.conf. + crbResult = nil + return true, nil + } else if apierrors.IsAlreadyExists(err) { + // If the CRB exists it means the admin.conf already has the right + // permissions; return. + return true, nil + } else { + // Retry on any other error type. + lastError = errors.Wrap(err, "unable to create ClusterRoleBinding") + return false, nil + } + } + return true, nil + }) + if err != nil { + return nil, lastError + } + + // The CRB exists; return the admin.conf client. + if crbResult != nil { + return adminClient, nil + } + + // If the superAdminClient is nil at this point we cannot proceed creating the CRB; return an error. + if superAdminClient == nil { + return nil, errors.Errorf("the ClusterRoleBinding for the %s Group is missing but there is no %s to create it", + kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding, + kubeadmconstants.SuperAdminKubeConfigFileName) + } + + // Create the ClusterRoleBinding with the super-admin.conf client. + klog.V(1).Infof("creating the ClusterRoleBinding for the %s Group by using %s", + kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding, + kubeadmconstants.SuperAdminKubeConfigFileName) + + if _, err := superAdminClient.RbacV1().ClusterRoleBindings().Create( + ctx, + clusterRoleBinding, + metav1.CreateOptions{}, + ); err != nil { + return nil, errors.Wrapf(err, "unable to create the %s ClusterRoleBinding", + kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding) + } + + // Once the CRB is in place, start using the admin.conf client. + return adminClient, nil +} diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go index 91d35f02ffd..cdbc4e51300 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go @@ -18,6 +18,7 @@ package kubeconfig import ( "bytes" + "context" "crypto" "crypto/x509" "fmt" @@ -26,14 +27,24 @@ import ( "path/filepath" "reflect" "testing" + "time" "github.com/lithammer/dedent" + "github.com/pkg/errors" + rbac "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + clientset "k8s.io/client-go/kubernetes" + clientsetfake "k8s.io/client-go/kubernetes/fake" + clientgotesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" certstestutil "k8s.io/kubernetes/cmd/kubeadm/app/util/certs" "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil" @@ -119,6 +130,11 @@ func TestGetKubeConfigSpecs(t *testing.T) { { kubeConfigFile: kubeadmconstants.AdminKubeConfigFileName, clientName: "kubernetes-admin", + organizations: []string{kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding}, + }, + { + kubeConfigFile: kubeadmconstants.SuperAdminKubeConfigFileName, + clientName: "kubernetes-super-admin", organizations: []string{kubeadmconstants.SystemPrivilegedGroup}, }, { @@ -174,7 +190,7 @@ func TestGetKubeConfigSpecs(t *testing.T) { } switch assertion.kubeConfigFile { - case kubeadmconstants.AdminKubeConfigFileName, kubeadmconstants.KubeletKubeConfigFileName: + case kubeadmconstants.AdminKubeConfigFileName, kubeadmconstants.SuperAdminKubeConfigFileName, kubeadmconstants.KubeletKubeConfigFileName: if spec.APIServer != controlPlaneEndpoint { t.Errorf("expected getKubeConfigSpecs for %s to set cfg.APIServer to %s, got %s", assertion.kubeConfigFile, controlPlaneEndpoint, spec.APIServer) @@ -615,8 +631,9 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { }, "some files don't exist": { filesToWrite: map[string]*clientcmdapi.Config{ - kubeadmconstants.AdminKubeConfigFileName: config, - kubeadmconstants.KubeletKubeConfigFileName: config, + kubeadmconstants.AdminKubeConfigFileName: config, + kubeadmconstants.SuperAdminKubeConfigFileName: config, + kubeadmconstants.KubeletKubeConfigFileName: config, }, initConfig: initConfig, expectedError: true, @@ -624,6 +641,7 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { "some files have invalid CA": { filesToWrite: map[string]*clientcmdapi.Config{ kubeadmconstants.AdminKubeConfigFileName: config, + kubeadmconstants.SuperAdminKubeConfigFileName: config, kubeadmconstants.KubeletKubeConfigFileName: config, kubeadmconstants.ControllerManagerKubeConfigFileName: configWithAnotherClusterCa, kubeadmconstants.SchedulerKubeConfigFileName: config, @@ -634,6 +652,7 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { "some files have a different Server URL": { filesToWrite: map[string]*clientcmdapi.Config{ kubeadmconstants.AdminKubeConfigFileName: config, + kubeadmconstants.SuperAdminKubeConfigFileName: config, kubeadmconstants.KubeletKubeConfigFileName: config, kubeadmconstants.ControllerManagerKubeConfigFileName: config, kubeadmconstants.SchedulerKubeConfigFileName: configWithAnotherServerURL, @@ -643,6 +662,7 @@ func TestValidateKubeconfigsForExternalCA(t *testing.T) { "all files are valid": { filesToWrite: map[string]*clientcmdapi.Config{ kubeadmconstants.AdminKubeConfigFileName: config, + kubeadmconstants.SuperAdminKubeConfigFileName: config, kubeadmconstants.KubeletKubeConfigFileName: config, kubeadmconstants.ControllerManagerKubeConfigFileName: config, kubeadmconstants.SchedulerKubeConfigFileName: config, @@ -715,3 +735,196 @@ func setupdKubeConfigWithTokenAuth(t *testing.T, caCert *x509.Certificate, APISe return config } + +func TestEnsureAdminClusterRoleBinding(t *testing.T) { + dir := testutil.SetupTempDir(t) + defer os.RemoveAll(dir) + + cfg := testutil.GetDefaultInternalConfig(t) + cfg.CertificatesDir = dir + + ca := certsphase.KubeadmCertRootCA() + _, _, err := ca.CreateAsCA(cfg) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + expectedRBACError bool + expectedError bool + missingAdminConf bool + missingSuperAdminConf bool + }{ + { + name: "no errors", + }, + { + name: "expect RBAC error", + expectedRBACError: true, + expectedError: true, + }, + { + name: "admin.conf is missing", + missingAdminConf: true, + expectedError: true, + }, + { + name: "super-admin.conf is missing", + missingSuperAdminConf: true, + expectedError: false, // The file is optional. + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ensureRBACFunc := func(_ context.Context, adminClient clientset.Interface, superAdminClient clientset.Interface, + _ time.Duration, _ time.Duration) (clientset.Interface, error) { + + if tc.expectedRBACError { + return nil, errors.New("ensureRBACFunc error") + } + return adminClient, nil + } + + // Create the admin.conf and super-admin.conf so that EnsureAdminClusterRoleBinding + // can create clients from the files. + os.Remove(filepath.Join(dir, kubeadmconstants.AdminKubeConfigFileName)) + if !tc.missingAdminConf { + if err := CreateKubeConfigFile(kubeadmconstants.AdminKubeConfigFileName, dir, cfg); err != nil { + t.Fatal(err) + } + } + os.Remove(filepath.Join(dir, kubeadmconstants.SuperAdminKubeConfigFileName)) + if !tc.missingSuperAdminConf { + if err := CreateKubeConfigFile(kubeadmconstants.SuperAdminKubeConfigFileName, dir, cfg); err != nil { + t.Fatal(err) + } + } + + client, err := EnsureAdminClusterRoleBinding(dir, ensureRBACFunc) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got: %v, error: %v", err != nil, tc.expectedError, err) + } + + if err == nil && client == nil { + t.Fatal("got nil client") + } + }) + } +} + +func TestEnsureAdminClusterRoleBindingImpl(t *testing.T) { + tests := []struct { + name string + setupAdminClient func(*clientsetfake.Clientset) + setupSuperAdminClient func(*clientsetfake.Clientset) + expectedError bool + }{ + { + name: "admin.conf: handle forbidden errors when the super-admin.conf client is nil", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewForbidden( + schema.GroupResource{}, "name", errors.New("")) + }) + }, + expectedError: true, + }, + { + // A "create" call against a real server can return a forbidden error and a non-nil CRB + name: "admin.conf: handle forbidden error and returned CRBs, when the super-admin.conf client is nil", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &rbac.ClusterRoleBinding{}, apierrors.NewForbidden( + schema.GroupResource{}, "name", errors.New("")) + }) + }, + expectedError: true, + }, + { + name: "admin.conf: CRB already exists, use the admin.conf client", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewAlreadyExists( + schema.GroupResource{}, "name") + }) + }, + expectedError: true, + }, + { + name: "admin.conf: handle other errors, such as a server timeout", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewServerTimeout( + schema.GroupResource{}, "create", 0) + }) + }, + expectedError: true, + }, + { + name: "admin.conf: CRB exists, return a client from admin.conf", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &rbac.ClusterRoleBinding{}, nil + }) + }, + expectedError: false, + }, + { + name: "super-admin.conf: error while creating CRB", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewForbidden( + schema.GroupResource{}, "name", errors.New("")) + }) + }, + setupSuperAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewServerTimeout( + schema.GroupResource{}, "create", 0) + }) + }, + expectedError: true, + }, + { + name: "super-admin.conf: admin.conf cannot create CRB, create CRB with super-admin.conf, return client from admin.conf", + setupAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewForbidden( + schema.GroupResource{}, "name", errors.New("")) + }) + }, + setupSuperAdminClient: func(client *clientsetfake.Clientset) { + client.PrependReactor("create", "clusterrolebindings", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &rbac.ClusterRoleBinding{}, nil + }) + }, + expectedError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + adminClient := clientsetfake.NewSimpleClientset() + tc.setupAdminClient(adminClient) + + var superAdminClient clientset.Interface // ensure superAdminClient is nil by default + if tc.setupSuperAdminClient != nil { + fakeSuperAdminClient := clientsetfake.NewSimpleClientset() + tc.setupSuperAdminClient(fakeSuperAdminClient) + superAdminClient = fakeSuperAdminClient + } + + client, err := EnsureAdminClusterRoleBindingImpl( + context.Background(), adminClient, superAdminClient, time.Millisecond*50, time.Millisecond*1000) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got %v, error: %v", tc.expectedError, err != nil, err) + } + + if err == nil && client == nil { + t.Fatal("got nil client") + } + }) + } +} diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index 09ad32e70d8..6d7c587c4da 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade.go @@ -40,6 +40,7 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy" "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/clusterinfo" nodebootstraptoken "k8s.io/kubernetes/cmd/kubeadm/app/phases/bootstraptoken/node" + kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig" kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet" patchnodephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/patchnode" "k8s.io/kubernetes/cmd/kubeadm/app/phases/uploadconfig" @@ -69,6 +70,12 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon errs = append(errs, err) } + // TODO: remove this in the 1.30 release cycle: + // https://github.com/kubernetes/kubeadm/issues/2414 + if err := createSuperAdminKubeConfig(cfg, kubeadmconstants.KubernetesDir, dryRun, nil, nil); err != nil { + errs = append(errs, err) + } + // Annotate the node with the crisocket information, sourced either from the InitConfiguration struct or // --cri-socket. // TODO: In the future we want to use something more official like NodeStatus or similar for detecting this properly @@ -296,3 +303,64 @@ func GetKubeletDir(dryRun bool) (string, error) { } return kubeadmconstants.KubeletRunDirectory, nil } + +// createSuperAdminKubeConfig creates new admin.conf and super-admin.conf and then +// ensures that the admin.conf client has RBAC permissions to be cluster-admin. +// TODO: this code must not be present in the 1.30 release, remove it during the 1.30 +// release cycle: +// https://github.com/kubernetes/kubeadm/issues/2414 +func createSuperAdminKubeConfig(cfg *kubeadmapi.InitConfiguration, outDir string, dryRun bool, + ensureRBACFunc kubeconfigphase.EnsureRBACFunc, + createKubeConfigFileFunc kubeconfigphase.CreateKubeConfigFileFunc) error { + + if dryRun { + fmt.Printf("[dryrun] Would create a separate %s and RBAC for %s", + kubeadmconstants.SuperAdminKubeConfigFileName, kubeadmconstants.AdminKubeConfigFileName) + return nil + } + + if ensureRBACFunc == nil { + ensureRBACFunc = kubeconfigphase.EnsureAdminClusterRoleBindingImpl + } + if createKubeConfigFileFunc == nil { + createKubeConfigFileFunc = kubeconfigphase.CreateKubeConfigFile + } + + var ( + err error + adminPath = filepath.Join(outDir, kubeadmconstants.AdminKubeConfigFileName) + adminBackupPath = adminPath + ".backup" + superAdminPath = filepath.Join(outDir, kubeadmconstants.SuperAdminKubeConfigFileName) + superAdminBackupPath = superAdminPath + ".backup" + ) + + // Create new admin.conf and super-admin.conf. + // If something goes wrong, old existing files will be restored from backup as a best effort. + + restoreBackup := func() { + _ = os.Rename(adminBackupPath, adminPath) + _ = os.Rename(superAdminBackupPath, superAdminPath) + } + + _ = os.Rename(adminPath, adminBackupPath) + if err = createKubeConfigFileFunc(kubeadmconstants.AdminKubeConfigFileName, outDir, cfg); err != nil { + restoreBackup() + return err + } + + _ = os.Rename(superAdminPath, superAdminBackupPath) + if err = createKubeConfigFileFunc(kubeadmconstants.SuperAdminKubeConfigFileName, outDir, cfg); err != nil { + restoreBackup() + return err + } + + // Ensure the RBAC for admin.conf exists. + if _, err = kubeconfigphase.EnsureAdminClusterRoleBinding(outDir, ensureRBACFunc); err != nil { + restoreBackup() + return err + } + + _ = os.Remove(adminBackupPath) + _ = os.Remove(superAdminBackupPath) + return nil +} diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go index f090889beb1..68595e43ba2 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go @@ -17,18 +17,25 @@ limitations under the License. package upgrade import ( + "context" "os" "path/filepath" + "reflect" "regexp" "strings" "testing" + "time" "github.com/pkg/errors" errorsutil "k8s.io/apimachinery/pkg/util/errors" + clientset "k8s.io/client-go/kubernetes" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" + certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" + kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig" "k8s.io/kubernetes/cmd/kubeadm/app/preflight" testutil "k8s.io/kubernetes/cmd/kubeadm/test" ) @@ -230,3 +237,106 @@ func rollbackFiles(files map[string]string, originalErr error) error { } return errors.Errorf("couldn't move these files: %v. Got errors: %v", files, errorsutil.NewAggregate(errs)) } + +// TODO: Remove this unit test during the 1.30 release cycle: +// https://github.com/kubernetes/kubeadm/issues/2414 +func TestCreateSuperAdminKubeConfig(t *testing.T) { + dir := testutil.SetupTempDir(t) + defer os.RemoveAll(dir) + + cfg := testutil.GetDefaultInternalConfig(t) + cfg.CertificatesDir = dir + + ca := certsphase.KubeadmCertRootCA() + _, _, err := ca.CreateAsCA(cfg) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + kubeConfigExist bool + expectRBACError bool + expectedError bool + expectKubeConfigError bool + }{ + { + name: "no error", + }, + { + name: "no error, kubeconfig files already exist", + kubeConfigExist: true, + }, + { + name: "return RBAC error", + expectRBACError: true, + expectedError: true, + }, + { + name: "return kubeconfig error", + expectKubeConfigError: true, + expectedError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + + // Define a custom RBAC test function, so that there is no test coverage overlap. + ensureRBACFunc := func(context.Context, clientset.Interface, clientset.Interface, + time.Duration, time.Duration) (clientset.Interface, error) { + + if tc.expectRBACError { + return nil, errors.New("ensureRBACFunc error") + } + return nil, nil + } + + // Define a custom kubeconfig function so that we can fail at least one call. + kubeConfigFunc := func(a string, b string, cfg *kubeadmapi.InitConfiguration) error { + if tc.expectKubeConfigError { + return errors.New("kubeConfigFunc error") + } + return kubeconfigphase.CreateKubeConfigFile(a, b, cfg) + } + + // If kubeConfigExist is true, pre-create the admin.conf and super-admin.conf files. + if tc.kubeConfigExist { + b := []byte("foo") + if err := os.WriteFile(filepath.Join(dir, constants.AdminKubeConfigFileName), b, 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, constants.SuperAdminKubeConfigFileName), b, 0644); err != nil { + t.Fatal(err) + } + } + + // Call createSuperAdminKubeConfig() with a custom ensureRBACFunc(). + err := createSuperAdminKubeConfig(cfg, dir, false, ensureRBACFunc, kubeConfigFunc) + if (err != nil) != tc.expectedError { + t.Fatalf("expected error: %v, got: %v, error: %v", err != nil, tc.expectedError, err) + } + + // Obtain the list of files in the directory after createSuperAdminKubeConfig() is done. + var files []string + fileInfo, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, file := range fileInfo { + files = append(files, file.Name()) + } + + // Verify the expected files. + expectedFiles := []string{ + constants.AdminKubeConfigFileName, + constants.CACertName, + constants.CAKeyName, + constants.SuperAdminKubeConfigFileName, + } + if !reflect.DeepEqual(expectedFiles, files) { + t.Fatalf("expected files: %v, got: %v", expectedFiles, files) + } + }) + } +} diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods.go b/cmd/kubeadm/app/phases/upgrade/staticpods.go index 4b63c23ce35..e5c88ac6f9e 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods.go @@ -497,6 +497,20 @@ func StaticPodControlPlane(client clientset.Interface, waiter apiclient.Waiter, // if not error, but not renewed because of external CA detected, inform the user fmt.Printf("[upgrade/staticpods] External CA detected, %s certificate can't be renewed\n", constants.AdminKubeConfigFileName) } + + // Do the same for super-admin.conf, but only if it exists + if _, err := os.Stat(filepath.Join(pathMgr.KubernetesDir(), constants.SuperAdminKubeConfigFileName)); err == nil { + // renew the certificate embedded in the super-admin.conf file + renewed, err := certsRenewMgr.RenewUsingLocalCA(constants.SuperAdminKubeConfigFileName) + if err != nil { + return rollbackOldManifests(recoverManifests, errors.Wrapf(err, "failed to upgrade the %s certificates", constants.SuperAdminKubeConfigFileName), pathMgr, false) + } + + if !renewed { + // if not error, but not renewed because of external CA detected, inform the user + fmt.Printf("[upgrade/staticpods] External CA detected, %s certificate can't be renewed\n", constants.SuperAdminKubeConfigFileName) + } + } } // Remove the temporary directories used on a best-effort (don't fail if the calls error out) diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go index 3d21e5e4cee..da7267f3c53 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go @@ -330,9 +330,7 @@ func TestStaticPodControlPlane(t *testing.T) { waitForHashChange: nil, waitForPodsWithLabel: nil, }, - moveFileFunc: func(oldPath, newPath string) error { - return os.Rename(oldPath, newPath) - }, + moveFileFunc: os.Rename, expectedErr: false, manifestShouldChange: true, }, @@ -343,9 +341,7 @@ func TestStaticPodControlPlane(t *testing.T) { waitForHashChange: nil, waitForPodsWithLabel: nil, }, - moveFileFunc: func(oldPath, newPath string) error { - return os.Rename(oldPath, newPath) - }, + moveFileFunc: os.Rename, expectedErr: true, manifestShouldChange: false, }, @@ -356,9 +352,7 @@ func TestStaticPodControlPlane(t *testing.T) { waitForHashChange: errors.New("boo! failed"), waitForPodsWithLabel: nil, }, - moveFileFunc: func(oldPath, newPath string) error { - return os.Rename(oldPath, newPath) - }, + moveFileFunc: os.Rename, expectedErr: true, manifestShouldChange: false, }, @@ -369,9 +363,7 @@ func TestStaticPodControlPlane(t *testing.T) { waitForHashChange: nil, waitForPodsWithLabel: errors.New("boo! failed"), }, - moveFileFunc: func(oldPath, newPath string) error { - return os.Rename(oldPath, newPath) - }, + moveFileFunc: os.Rename, expectedErr: true, manifestShouldChange: false, }, @@ -433,9 +425,7 @@ func TestStaticPodControlPlane(t *testing.T) { waitForHashChange: nil, waitForPodsWithLabel: nil, }, - moveFileFunc: func(oldPath, newPath string) error { - return os.Rename(oldPath, newPath) - }, + moveFileFunc: os.Rename, skipKubeConfig: constants.SchedulerKubeConfigFileName, expectedErr: true, manifestShouldChange: false, @@ -447,13 +437,34 @@ func TestStaticPodControlPlane(t *testing.T) { waitForHashChange: nil, waitForPodsWithLabel: nil, }, - moveFileFunc: func(oldPath, newPath string) error { - return os.Rename(oldPath, newPath) - }, + moveFileFunc: os.Rename, skipKubeConfig: constants.AdminKubeConfigFileName, expectedErr: true, manifestShouldChange: false, }, + { + description: "super-admin.conf is renewed if it exists", + waitErrsToReturn: map[string]error{ + waitForHashes: nil, + waitForHashChange: nil, + waitForPodsWithLabel: nil, + }, + moveFileFunc: os.Rename, + expectedErr: false, + manifestShouldChange: true, + }, + { + description: "no error is thrown if super-admin.conf does not exist", + waitErrsToReturn: map[string]error{ + waitForHashes: nil, + waitForHashChange: nil, + waitForPodsWithLabel: nil, + }, + moveFileFunc: os.Rename, + skipKubeConfig: constants.SuperAdminKubeConfigFileName, + expectedErr: false, + manifestShouldChange: true, + }, } for i := range tests { @@ -495,6 +506,7 @@ func TestStaticPodControlPlane(t *testing.T) { for _, kubeConfig := range []string{ constants.AdminKubeConfigFileName, + constants.SuperAdminKubeConfigFileName, constants.SchedulerKubeConfigFileName, constants.ControllerManagerKubeConfigFileName, } {