From 73aeed8766c2c42a6cb4fc8632b1b974f4508dde Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:35:16 +0800 Subject: [PATCH] fix fd leaks and failed file removing for pkg client-go --- .../src/k8s.io/client-go/rest/request_test.go | 2 +- .../tools/clientcmd/api/helpers_test.go | 30 +++++--------- .../tools/clientcmd/client_config_test.go | 13 +++--- .../client-go/tools/clientcmd/loader_test.go | 40 ++++++++----------- .../tools/clientcmd/validation_test.go | 8 ++-- 5 files changed, 39 insertions(+), 54 deletions(-) diff --git a/staging/src/k8s.io/client-go/rest/request_test.go b/staging/src/k8s.io/client-go/rest/request_test.go index b4d6ea09af7..e26562bb065 100644 --- a/staging/src/k8s.io/client-go/rest/request_test.go +++ b/staging/src/k8s.io/client-go/rest/request_test.go @@ -322,7 +322,7 @@ func TestRequestBody(t *testing.T) { } // test error set when failing to read file - f, err := os.CreateTemp("", "test") + f, err := os.CreateTemp("", "") if err != nil { t.Fatalf("unable to create temp file") } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go index 9cab27b1453..f823ef0f899 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/helpers_test.go @@ -23,6 +23,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "sigs.k8s.io/yaml" ) @@ -53,11 +55,9 @@ func newMergedConfig(certFile, certContent, keyFile, keyContent, caFile, caConte func TestMinifySuccess(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) @@ -89,11 +89,9 @@ func TestMinifySuccess(t *testing.T) { func TestMinifyMissingContext(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) mutatingConfig.CurrentContext = "missing" @@ -107,11 +105,9 @@ func TestMinifyMissingContext(t *testing.T) { func TestMinifyMissingCluster(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) delete(mutatingConfig.Clusters, mutatingConfig.Contexts[mutatingConfig.CurrentContext].Cluster) @@ -125,11 +121,9 @@ func TestMinifyMissingCluster(t *testing.T) { func TestMinifyMissingAuthInfo(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) mutatingConfig := newMergedConfig(certFile.Name(), "cert", keyFile.Name(), "key", caFile.Name(), "ca", t) delete(mutatingConfig.AuthInfos, mutatingConfig.Contexts[mutatingConfig.CurrentContext].AuthInfo) @@ -143,11 +137,9 @@ func TestMinifyMissingAuthInfo(t *testing.T) { func TestFlattenSuccess(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) certData := "cert" keyData := "key" @@ -208,11 +200,9 @@ func TestFlattenSuccess(t *testing.T) { func Example_minifyAndShorten() { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, certFile, keyFile, caFile) certData := "cert" keyData := "key" @@ -245,11 +235,9 @@ func Example_minifyAndShorten() { func TestShortenSuccess(t *testing.T) { certFile, _ := os.CreateTemp("", "") - defer os.Remove(certFile.Name()) keyFile, _ := os.CreateTemp("", "") - defer os.Remove(keyFile.Name()) caFile, _ := os.CreateTemp("", "") - defer os.Remove(caFile.Name()) + defer utiltesting.CloseAndRemove(t, certFile, keyFile, caFile) certData := "cert" keyData := "key" diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go index ee78d8e6b32..64efc901623 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go @@ -22,6 +22,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/imdario/mergo" "k8s.io/apimachinery/pkg/runtime" @@ -177,7 +179,7 @@ func TestCAOverridesCAData(t *testing.T) { if err != nil { t.Fatalf("could not create tempfile: %v", err) } - defer os.Remove(file.Name()) + defer utiltesting.CloseAndRemove(t, file) config := createCAValidTestConfig() clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{ @@ -312,8 +314,7 @@ func TestModifyContext(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - defer os.Remove(tempPath.Name()) - + defer utiltesting.CloseAndRemove(t, tempPath) pathOptions := NewDefaultPathOptions() config := createValidTestConfig() @@ -498,7 +499,7 @@ func TestBasicTokenFile(t *testing.T) { t.Errorf("Unexpected error: %v", err) return } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) if err := os.WriteFile(f.Name(), []byte(token), 0644); err != nil { t.Errorf("Unexpected error: %v", err) return @@ -534,7 +535,7 @@ func TestPrecedenceTokenFile(t *testing.T) { t.Errorf("Unexpected error: %v", err) return } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) if err := os.WriteFile(f.Name(), []byte(token), 0644); err != nil { t.Errorf("Unexpected error: %v", err) return @@ -923,7 +924,7 @@ users: if err != nil { t.Error(err) } - defer os.Remove(tmpfile.Name()) + defer utiltesting.CloseAndRemove(t, tmpfile) if err := os.WriteFile(tmpfile.Name(), []byte(content), 0666); err != nil { t.Error(err) } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go index aa6741b8b5f..30e6161f5bc 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/loader_test.go @@ -26,6 +26,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" "sigs.k8s.io/yaml" @@ -132,7 +134,7 @@ func TestToleratingMissingFiles(t *testing.T) { func TestErrorReadingFile(t *testing.T) { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) + defer utiltesting.CloseAndRemove(t, commandLineFile) if err := os.WriteFile(commandLineFile.Name(), []byte("bogus value"), 0644); err != nil { t.Fatalf("Error creating tempfile: %v", err) @@ -173,9 +175,8 @@ func TestErrorReadingNonFile(t *testing.T) { func TestConflictingCurrentContext(t *testing.T) { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) + defer utiltesting.CloseAndRemove(t, commandLineFile, envVarFile) mockCommandLineConfig := clientcmdapi.Config{ CurrentContext: "any-context-value", @@ -254,7 +255,7 @@ users: null func TestLoadingEmptyMaps(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) mockConfig := clientcmdapi.Config{ CurrentContext: "any-context-value", @@ -280,7 +281,7 @@ func TestLoadingEmptyMaps(t *testing.T) { func TestDuplicateClusterName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -322,7 +323,7 @@ users: func TestDuplicateContextName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -364,7 +365,7 @@ users: func TestDuplicateUserName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -404,7 +405,7 @@ users: func TestDuplicateExtensionName(t *testing.T) { configFile, _ := os.CreateTemp("", "") - defer os.Remove(configFile.Name()) + defer utiltesting.CloseAndRemove(t, configFile) err := os.WriteFile(configFile.Name(), []byte(` kind: Config @@ -560,7 +561,7 @@ func TestResolveRelativePaths(t *testing.T) { func TestMigratingFile(t *testing.T) { sourceFile, _ := os.CreateTemp("", "") - defer os.Remove(sourceFile.Name()) + defer utiltesting.CloseAndRemove(t, sourceFile) destinationFile, _ := os.CreateTemp("", "") // delete the file so that we'll write to it os.Remove(destinationFile.Name()) @@ -574,9 +575,8 @@ func TestMigratingFile(t *testing.T) { if _, err := loadingRules.Load(); err != nil { t.Errorf("unexpected error %v", err) } - // the load should have recreated this file - defer os.Remove(destinationFile.Name()) + defer utiltesting.CloseAndRemove(t, destinationFile) sourceContent, err := os.ReadFile(sourceFile.Name()) if err != nil { @@ -594,9 +594,8 @@ func TestMigratingFile(t *testing.T) { func TestMigratingFileLeaveExistingFileAlone(t *testing.T) { sourceFile, _ := os.CreateTemp("", "") - defer os.Remove(sourceFile.Name()) destinationFile, _ := os.CreateTemp("", "") - defer os.Remove(destinationFile.Name()) + defer utiltesting.CloseAndRemove(t, sourceFile, destinationFile) WriteToFile(testConfigAlfa, sourceFile.Name()) @@ -622,7 +621,7 @@ func TestMigratingFileSourceMissingSkip(t *testing.T) { sourceFilename := "some-missing-file" destinationFile, _ := os.CreateTemp("", "") // delete the file so that we'll write to it - os.Remove(destinationFile.Name()) + utiltesting.CloseAndRemove(t, destinationFile) loadingRules := ClientConfigLoadingRules{ MigrationRules: map[string]string{destinationFile.Name(): sourceFilename}, @@ -639,7 +638,7 @@ func TestMigratingFileSourceMissingSkip(t *testing.T) { func TestFileLocking(t *testing.T) { f, _ := os.CreateTemp("", "") - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) err := lockFile(f.Name()) if err != nil { @@ -655,9 +654,8 @@ func TestFileLocking(t *testing.T) { func Example_noMergingOnExplicitPaths() { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, commandLineFile, envVarFile) WriteToFile(testConfigAlfa, commandLineFile.Name()) WriteToFile(testConfigConflictAlfa, envVarFile.Name()) @@ -704,9 +702,8 @@ func Example_noMergingOnExplicitPaths() { func Example_mergingSomeWithConflict() { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, commandLineFile, envVarFile) WriteToFile(testConfigAlfa, commandLineFile.Name()) WriteToFile(testConfigConflictAlfa, envVarFile.Name()) @@ -760,13 +757,10 @@ func Example_mergingSomeWithConflict() { func Example_mergingEverythingNoConflicts() { commandLineFile, _ := os.CreateTemp("", "") - defer os.Remove(commandLineFile.Name()) envVarFile, _ := os.CreateTemp("", "") - defer os.Remove(envVarFile.Name()) currentDirFile, _ := os.CreateTemp("", "") - defer os.Remove(currentDirFile.Name()) homeDirFile, _ := os.CreateTemp("", "") - defer os.Remove(homeDirFile.Name()) + defer utiltesting.CloseAndRemove(&testing.T{}, commandLineFile, envVarFile, currentDirFile, homeDirFile) WriteToFile(testConfigAlfa, commandLineFile.Name()) WriteToFile(testConfigBravo, envVarFile.Name()) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go index 3c56498a44f..13296637099 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/validation_test.go @@ -23,6 +23,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + utilerrors "k8s.io/apimachinery/pkg/util/errors" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -296,7 +298,7 @@ func TestValidateCleanClusterInfo(t *testing.T) { func TestValidateCleanWithCAClusterInfo(t *testing.T) { tempFile, _ := os.CreateTemp("", "") - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) config := clientcmdapi.NewConfig() config.Clusters["clean"] = &clientcmdapi.Cluster{ @@ -339,7 +341,7 @@ func TestValidateCertFilesNotFoundAuthInfo(t *testing.T) { func TestValidateCertDataOverridesFiles(t *testing.T) { tempFile, _ := os.CreateTemp("", "") - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) config := clientcmdapi.NewConfig() config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ @@ -359,7 +361,7 @@ func TestValidateCertDataOverridesFiles(t *testing.T) { func TestValidateCleanCertFilesAuthInfo(t *testing.T) { tempFile, _ := os.CreateTemp("", "") - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) config := clientcmdapi.NewConfig() config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{