From 46c4249e3ec20692a70ad329402cb6b5e900f31d Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Mon, 15 Jan 2024 19:09:50 +0800 Subject: [PATCH 1/2] Remove code to be removed in 1.30 --- cmd/kubeadm/app/phases/upgrade/postupgrade.go | 68 ----------- .../app/phases/upgrade/postupgrade_test.go | 110 ------------------ 2 files changed, 178 deletions(-) diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index 336efbf2174..895d28ddc39 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade.go @@ -40,7 +40,6 @@ 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" @@ -70,12 +69,6 @@ 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 @@ -304,64 +297,3 @@ 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 68595e43ba2..f090889beb1 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go @@ -17,25 +17,18 @@ 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" ) @@ -237,106 +230,3 @@ 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) - } - }) - } -} From 6e5e1d03177c74b7041848adc664432338b8fabb Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Thu, 25 Jan 2024 21:49:14 +0800 Subject: [PATCH 2/2] Remove useless org mutate code --- .../app/phases/certs/renewal/manager.go | 43 ------------------- .../app/phases/certs/renewal/manager_test.go | 17 -------- 2 files changed, 60 deletions(-) diff --git a/cmd/kubeadm/app/phases/certs/renewal/manager.go b/cmd/kubeadm/app/phases/certs/renewal/manager.go index 61c2ccdb714..0cf272e1617 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/manager.go +++ b/cmd/kubeadm/app/phases/certs/renewal/manager.go @@ -423,48 +423,5 @@ func certToConfig(cert *x509.Certificate) certutil.Config { } func loadCertConfigMutators(certBaseName string) []certConfigMutatorFunc { - // TODO: Remove these mutators after the organization migration is complete in a future release - // https://github.com/kubernetes/kubeadm/issues/2414 - switch certBaseName { - case kubeadmconstants.EtcdHealthcheckClientCertAndKeyBaseName, - kubeadmconstants.APIServerEtcdClientCertAndKeyBaseName: - return []certConfigMutatorFunc{ - removeSystemPrivilegedGroupMutator(), - } - case kubeadmconstants.APIServerKubeletClientCertAndKeyBaseName: - return []certConfigMutatorFunc{ - removeSystemPrivilegedGroupMutator(), - addClusterAdminsGroupMutator(), - } - } return nil } - -func removeSystemPrivilegedGroupMutator() certConfigMutatorFunc { - return func(c *certutil.Config) error { - organizations := make([]string, 0, len(c.Organization)) - for _, org := range c.Organization { - if org != kubeadmconstants.SystemPrivilegedGroup { - organizations = append(organizations, org) - } - } - c.Organization = organizations - return nil - } -} - -func addClusterAdminsGroupMutator() certConfigMutatorFunc { - return func(c *certutil.Config) error { - found := false - for _, org := range c.Organization { - if org == kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding { - found = true - break - } - } - if !found { - c.Organization = append(c.Organization, kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding) - } - return nil - } -} diff --git a/cmd/kubeadm/app/phases/certs/renewal/manager_test.go b/cmd/kubeadm/app/phases/certs/renewal/manager_test.go index b06df5fe7cf..aea942c1483 100644 --- a/cmd/kubeadm/app/phases/certs/renewal/manager_test.go +++ b/cmd/kubeadm/app/phases/certs/renewal/manager_test.go @@ -30,7 +30,6 @@ import ( netutils "k8s.io/utils/net" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" - kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" certtestutil "k8s.io/kubernetes/cmd/kubeadm/app/util/certs" "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil" testutil "k8s.io/kubernetes/cmd/kubeadm/test" @@ -127,22 +126,6 @@ func TestRenewUsingLocalCA(t *testing.T) { }, expectedOrganization: testCertOrganization, }, - { - name: "apiserver-etcd-client cert should not contain SystemPrivilegedGroup after renewal", - certName: "apiserver-etcd-client", - createCertFunc: func() *x509.Certificate { - return writeTestCertificate(t, dir, "apiserver-etcd-client", testCACert, testCAKey, []string{kubeadmconstants.SystemPrivilegedGroup}) - }, - expectedOrganization: []string{}, - }, - { - name: "apiserver-kubelet-client cert should replace SystemPrivilegedGroup with ClusterAdminsGroup after renewal", - certName: "apiserver-kubelet-client", - createCertFunc: func() *x509.Certificate { - return writeTestCertificate(t, dir, "apiserver-kubelet-client", testCACert, testCAKey, []string{kubeadmconstants.SystemPrivilegedGroup}) - }, - expectedOrganization: []string{kubeadmconstants.ClusterAdminsGroupAndClusterRoleBinding}, - }, } for _, test := range tests {