From 24ba0e8ff17e7ea402a835d31ed14b35b45014d9 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Mon, 10 Apr 2023 14:15:37 +0200 Subject: [PATCH 1/3] OCPBUGS-11395: Remove stale kubeconfigs Remove any kubeconfig that is not part of the hostname or SAN values in configuration. --- .../microshift/pkg/config/kubeconfig.go | 6 +++- pkg/cmd/init.go | 33 +++++++++++++++++++ pkg/config/kubeconfig.go | 6 +++- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/etcd/vendor/github.com/openshift/microshift/pkg/config/kubeconfig.go b/etcd/vendor/github.com/openshift/microshift/pkg/config/kubeconfig.go index 72a85466f5..a4409a5304 100644 --- a/etcd/vendor/github.com/openshift/microshift/pkg/config/kubeconfig.go +++ b/etcd/vendor/github.com/openshift/microshift/pkg/config/kubeconfig.go @@ -20,5 +20,9 @@ func (cfg *Config) KubeConfigPath(id KubeConfigID) string { } func (cfg *Config) KubeConfigAdminPath(id string) string { - return filepath.Join(DataDir, "resources", string(KubeAdmin), id, "kubeconfig") + return filepath.Join(cfg.KubeConfigRootAdminPath(), id, "kubeconfig") +} + +func (cfg *Config) KubeConfigRootAdminPath() string { + return filepath.Join(DataDir, "resources", string(KubeAdmin)) } diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index abd3ccb890..4c77b80655 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -33,6 +33,8 @@ import ( "github.com/openshift/microshift/pkg/util" "github.com/openshift/microshift/pkg/util/cryptomaterial" "github.com/openshift/microshift/pkg/util/cryptomaterial/certchains" + + "k8s.io/klog/v2" ) func initCerts(cfg *config.Config) (*certchains.CertificateChains, error) { @@ -401,6 +403,10 @@ func initKubeconfigs( } } + if err := cleanupStaleKubeconfigs(cfg); err != nil { + klog.Warningf("Unable to remove stale kubeconfigs: %v", err) + } + if err := util.KubeConfigWithClientCerts( cfg.KubeConfigPath(config.KubeAdmin), cfg.ApiServer.URL, @@ -510,3 +516,30 @@ func certsToRegenerate(cs *certchains.CertificateChains) ([][]string, error) { return regenCerts, err } + +func cleanupStaleKubeconfigs(cfg *config.Config) error { + currentKubeconfigs := make(map[string]struct{}) + for _, name := range append(cfg.ApiServer.SubjectAltNames, cfg.Node.HostnameOverride) { + currentKubeconfigs[name] = struct{}{} + } + files, err := os.ReadDir(cfg.KubeConfigRootAdminPath()) + if err != nil { + return err + } + deleteDirs := make([]string, 0) + for _, file := range files { + if !file.IsDir() { + continue + } + if _, ok := currentKubeconfigs[file.Name()]; !ok { + deleteDirs = append(deleteDirs, filepath.Join(cfg.KubeConfigRootAdminPath(), file.Name())) + } + } + for _, path := range deleteDirs { + if err := os.RemoveAll(path); err != nil { + klog.Warningf("Unable to remove %s: %v", path, err) + } + klog.Infof("Removed stale kubeconfig %s", path) + } + return nil +} diff --git a/pkg/config/kubeconfig.go b/pkg/config/kubeconfig.go index 72a85466f5..a4409a5304 100644 --- a/pkg/config/kubeconfig.go +++ b/pkg/config/kubeconfig.go @@ -20,5 +20,9 @@ func (cfg *Config) KubeConfigPath(id KubeConfigID) string { } func (cfg *Config) KubeConfigAdminPath(id string) string { - return filepath.Join(DataDir, "resources", string(KubeAdmin), id, "kubeconfig") + return filepath.Join(cfg.KubeConfigRootAdminPath(), id, "kubeconfig") +} + +func (cfg *Config) KubeConfigRootAdminPath() string { + return filepath.Join(DataDir, "resources", string(KubeAdmin)) } From f26232632d8518f45ee67cf3faf959191cef961e Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Wed, 12 Apr 2023 10:04:22 +0200 Subject: [PATCH 2/3] OCPBUGS-11395: Add path parameter to cleanup function --- pkg/cmd/init.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index 4c77b80655..8e681b2b6b 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -403,7 +403,7 @@ func initKubeconfigs( } } - if err := cleanupStaleKubeconfigs(cfg); err != nil { + if err := cleanupStaleKubeconfigs(cfg, cfg.KubeConfigRootAdminPath()); err != nil { klog.Warningf("Unable to remove stale kubeconfigs: %v", err) } @@ -517,12 +517,12 @@ func certsToRegenerate(cs *certchains.CertificateChains) ([][]string, error) { return regenCerts, err } -func cleanupStaleKubeconfigs(cfg *config.Config) error { +func cleanupStaleKubeconfigs(cfg *config.Config, path string) error { currentKubeconfigs := make(map[string]struct{}) for _, name := range append(cfg.ApiServer.SubjectAltNames, cfg.Node.HostnameOverride) { currentKubeconfigs[name] = struct{}{} } - files, err := os.ReadDir(cfg.KubeConfigRootAdminPath()) + files, err := os.ReadDir(path) if err != nil { return err } @@ -532,14 +532,14 @@ func cleanupStaleKubeconfigs(cfg *config.Config) error { continue } if _, ok := currentKubeconfigs[file.Name()]; !ok { - deleteDirs = append(deleteDirs, filepath.Join(cfg.KubeConfigRootAdminPath(), file.Name())) + deleteDirs = append(deleteDirs, filepath.Join(path, file.Name())) } } - for _, path := range deleteDirs { - if err := os.RemoveAll(path); err != nil { - klog.Warningf("Unable to remove %s: %v", path, err) + for _, deletePath := range deleteDirs { + if err := os.RemoveAll(deletePath); err != nil { + klog.Warningf("Unable to remove %s: %v", deletePath, err) } - klog.Infof("Removed stale kubeconfig %s", path) + klog.Infof("Removed stale kubeconfig %s", deletePath) } return nil } From dba3f1f504131ca2585cf04b2ffdd53b6a7c3b49 Mon Sep 17 00:00:00 2001 From: Pablo Acevedo Montserrat Date: Wed, 12 Apr 2023 11:22:22 +0200 Subject: [PATCH 3/3] OCPBUGS-11395: Add unit tests --- pkg/cmd/init_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index 568114c5ea..80045ab695 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -16,9 +16,12 @@ limitations under the License. package cmd import ( + "os" + "path/filepath" "reflect" "testing" + "github.com/openshift/microshift/pkg/config" "github.com/openshift/microshift/pkg/util/cryptomaterial/certchains" "github.com/stretchr/testify/require" "k8s.io/apiserver/pkg/authentication/user" @@ -126,6 +129,45 @@ func Test_certsToRegenerate(t *testing.T) { } } +func Test_removeStaleKubeconfig(t *testing.T) { + rootDir, err := os.MkdirTemp("", "test") + if err != nil { + t.Fatalf("unable to create temporary dir: %v", err) + } + defer os.RemoveAll(rootDir) + + cfg := &config.Config{ + Node: config.Node{ + HostnameOverride: "hostname", + }, + ApiServer: config.ApiServer{ + SubjectAltNames: []string{"altname1", "altname2"}, + }, + } + for _, dir := range append(cfg.ApiServer.SubjectAltNames, cfg.Node.HostnameOverride) { + os.Mkdir(filepath.Join(rootDir, dir), 0600) + } + + staleDir, err := os.MkdirTemp(rootDir, "example") + if err != nil { + t.Fatalf("unable to create temporary dir: %v", err) + } + cleanupStaleKubeconfigs(cfg, rootDir) + _, err = os.Stat(staleDir) + if err == nil { + t.Fatalf("%s should have been deleted", staleDir) + } + if !os.IsNotExist(err) { + t.Fatalf("unable to check %s existence: %v", staleDir, err) + } + for _, dir := range append(cfg.ApiServer.SubjectAltNames, cfg.Node.HostnameOverride) { + d := filepath.Join(rootDir, dir) + if _, err = os.Stat(d); err != nil { + t.Fatalf("dir %s should remain: %v", d, err) + } + } +} + func mustComplete(t *testing.T, cs certchains.CertificateChainsBuilder) *certchains.CertificateChains { ret, err := cs.Complete() require.NoError(t, err)