From 82e3fa0930eaf88e9c8fb639d96523c8d91d97cf Mon Sep 17 00:00:00 2001 From: HirazawaUi <695097494plus@gmail.com> Date: Wed, 3 May 2023 01:37:53 +0800 Subject: [PATCH] fix fd leaks and failed file removing for main pkg and cmd --- cmd/kubeadm/app/preflight/checks_test.go | 18 ++++++----- cmd/kubeadm/app/util/patches/patches_test.go | 4 ++- pkg/credentialprovider/plugin/config_test.go | 4 ++- .../certificate/bootstrap/bootstrap_test.go | 6 ++-- pkg/kubelet/kuberuntime/logs/logs_test.go | 4 ++- .../admission/imagepolicy/admission_test.go | 32 +++++++++---------- 6 files changed, 39 insertions(+), 29 deletions(-) diff --git a/cmd/kubeadm/app/preflight/checks_test.go b/cmd/kubeadm/app/preflight/checks_test.go index 52c1a125f6d..b5927fa0f7c 100644 --- a/cmd/kubeadm/app/preflight/checks_test.go +++ b/cmd/kubeadm/app/preflight/checks_test.go @@ -26,6 +26,8 @@ import ( "strings" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" "github.com/lithammer/dedent" "github.com/pkg/errors" @@ -195,7 +197,7 @@ func TestFileExistingCheck(t *testing.T) { if err != nil { t.Fatalf("Failed to create file: %v", err) } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) var tests = []struct { name string check FileExistingCheck @@ -234,7 +236,7 @@ func TestFileAvailableCheck(t *testing.T) { if err != nil { t.Fatalf("Failed to create file: %v", err) } - defer os.Remove(f.Name()) + defer utiltesting.CloseAndRemove(t, f) var tests = []struct { name string check FileAvailableCheck @@ -461,8 +463,8 @@ func TestConfigRootCAs(t *testing.T) { if err != nil { t.Errorf("failed configRootCAs:\n\texpected: succeed creating temp CA file\n\tactual:%v", err) } - defer os.Remove(f.Name()) - if err := os.WriteFile(f.Name(), []byte(externalEtcdRootCAFileContent), 0644); err != nil { + defer utiltesting.CloseAndRemove(t, f) + if _, err := f.Write([]byte(externalEtcdRootCAFileContent)); err != nil { t.Errorf("failed configRootCAs:\n\texpected: succeed writing contents to temp CA file %s\n\tactual:%v", f.Name(), err) } @@ -490,8 +492,8 @@ func TestConfigCertAndKey(t *testing.T) { err, ) } - defer os.Remove(certFile.Name()) - if err := os.WriteFile(certFile.Name(), []byte(externalEtcdCertFileContent), 0644); err != nil { + defer utiltesting.CloseAndRemove(t, certFile) + if _, err := certFile.Write([]byte(externalEtcdCertFileContent)); err != nil { t.Errorf( "failed configCertAndKey:\n\texpected: succeed writing contents to temp CertFile file %s\n\tactual:%v", certFile.Name(), @@ -506,8 +508,8 @@ func TestConfigCertAndKey(t *testing.T) { err, ) } - defer os.Remove(keyFile.Name()) - if err := os.WriteFile(keyFile.Name(), []byte(externalEtcdKeyFileContent), 0644); err != nil { + defer utiltesting.CloseAndRemove(t, keyFile) + if _, err := keyFile.Write([]byte(externalEtcdKeyFileContent)); err != nil { t.Errorf( "failed configCertAndKey:\n\texpected: succeed writing contents to temp KeyFile file %s\n\tactual:%v", keyFile.Name(), diff --git a/cmd/kubeadm/app/util/patches/patches_test.go b/cmd/kubeadm/app/util/patches/patches_test.go index b704c820928..5213c5f7f70 100644 --- a/cmd/kubeadm/app/util/patches/patches_test.go +++ b/cmd/kubeadm/app/util/patches/patches_test.go @@ -24,6 +24,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/pkg/errors" v1 "k8s.io/api/core/v1" @@ -172,7 +174,7 @@ func TestGetPatchSetsForPathMustBeDirectory(t *testing.T) { if err != nil { t.Errorf("error creating temporary file: %v", err) } - defer os.Remove(tempFile.Name()) + defer utiltesting.CloseAndRemove(t, tempFile) _, _, _, err = getPatchSetsFromPath(tempFile.Name(), testKnownTargets, io.Discard) var pathErr *os.PathError diff --git a/pkg/credentialprovider/plugin/config_test.go b/pkg/credentialprovider/plugin/config_test.go index 2d1e10bf966..3ca2104ea67 100644 --- a/pkg/credentialprovider/plugin/config_test.go +++ b/pkg/credentialprovider/plugin/config_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" ) @@ -320,7 +322,7 @@ providers: if err != nil { t.Fatal(err) } - defer os.Remove(file.Name()) + defer utiltesting.CloseAndRemove(t, file) _, err = file.WriteString(testcase.configData) if err != nil { diff --git a/pkg/kubelet/certificate/bootstrap/bootstrap_test.go b/pkg/kubelet/certificate/bootstrap/bootstrap_test.go index e5dee33dae0..71fa56efdfb 100644 --- a/pkg/kubelet/certificate/bootstrap/bootstrap_test.go +++ b/pkg/kubelet/certificate/bootstrap/bootstrap_test.go @@ -25,6 +25,8 @@ import ( "reflect" "testing" + utiltesting "k8s.io/client-go/util/testing" + "github.com/google/go-cmp/cmp" certificatesv1 "k8s.io/api/certificates/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -324,8 +326,8 @@ users: if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) - os.WriteFile(f.Name(), testData, os.FileMode(0755)) + defer utiltesting.CloseAndRemove(t, f) + f.Write(testData) config, err := loadRESTClientConfig(f.Name()) if err != nil { diff --git a/pkg/kubelet/kuberuntime/logs/logs_test.go b/pkg/kubelet/kuberuntime/logs/logs_test.go index c7d2ed5e396..f2c17cb8161 100644 --- a/pkg/kubelet/kuberuntime/logs/logs_test.go +++ b/pkg/kubelet/kuberuntime/logs/logs_test.go @@ -26,6 +26,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + v1 "k8s.io/api/core/v1" apitesting "k8s.io/cri-api/pkg/apis/testing" "k8s.io/utils/pointer" @@ -79,7 +81,7 @@ func TestReadLogs(t *testing.T) { if err != nil { t.Fatalf("unable to create temp file") } - defer os.Remove(file.Name()) + defer utiltesting.CloseAndRemove(t, file) file.WriteString(`{"log":"line1\n","stream":"stdout","time":"2020-09-27T11:18:01.00000000Z"}` + "\n") file.WriteString(`{"log":"line2\n","stream":"stdout","time":"2020-09-27T11:18:02.00000000Z"}` + "\n") file.WriteString(`{"log":"line3\n","stream":"stdout","time":"2020-09-27T11:18:03.00000000Z"}` + "\n") diff --git a/plugin/pkg/admission/imagepolicy/admission_test.go b/plugin/pkg/admission/imagepolicy/admission_test.go index d1f81d51950..267b163eeda 100644 --- a/plugin/pkg/admission/imagepolicy/admission_test.go +++ b/plugin/pkg/admission/imagepolicy/admission_test.go @@ -29,6 +29,8 @@ import ( "testing" "time" + utiltesting "k8s.io/client-go/util/testing" + "k8s.io/api/imagepolicy/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/admission" @@ -67,7 +69,7 @@ imagePolicy: ` func TestNewFromConfig(t *testing.T) { - dir, err := ioutil.TempDir("", "") + dir, err := os.MkdirTemp("", "") if err != nil { t.Fatal(err) } @@ -200,8 +202,7 @@ current-context: default if err != nil { return err } - p := tempfile.Name() - defer os.Remove(p) + defer utiltesting.CloseAndRemove(t, tempfile) tmpl, err := template.New("test").Parse(tt.kubeConfigTmpl) if err != nil { @@ -215,8 +216,7 @@ current-context: default if err != nil { return err } - pc := tempconfigfile.Name() - defer os.Remove(pc) + defer os.Remove(tempconfigfile.Name()) configTmpl, err := template.New("testconfig").Parse(defaultConfigTmplJSON) if err != nil { @@ -229,7 +229,7 @@ current-context: default RetryBackoff int DefaultAllow bool }{ - KubeConfig: p, + KubeConfig: tempfile.Name(), AllowTTL: 500, DenyTTL: 500, RetryBackoff: 500, @@ -240,7 +240,7 @@ current-context: default } // Create a new admission controller - configFile, err := os.Open(pc) + configFile, err := os.Open(tempconfigfile.Name()) if err != nil { return fmt.Errorf("failed to read test config: %v", err) } @@ -358,13 +358,13 @@ func (m *mockService) HTTPStatusCode() int { return m.statusCode } // newImagePolicyWebhook creates a temporary kubeconfig file from the provided arguments and attempts to load // a new newImagePolicyWebhook from it. -func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) { +func newImagePolicyWebhook(t *testing.T, callbackURL string, clientCert, clientKey, ca []byte, cacheTime time.Duration, defaultAllow bool) (*Plugin, error) { tempfile, err := ioutil.TempFile("", "") if err != nil { return nil, err } p := tempfile.Name() - defer os.Remove(p) + defer utiltesting.CloseAndRemove(t, tempfile) config := v1.Config{ Clusters: []v1.NamedCluster{ { @@ -386,7 +386,7 @@ func newImagePolicyWebhook(callbackURL string, clientCert, clientKey, ca []byte, return nil, err } pc := tempconfigfile.Name() - defer os.Remove(pc) + defer utiltesting.CloseAndRemove(t, tempconfigfile) configTmpl, err := template.New("testconfig").Parse(defaultConfigTmplYAML) if err != nil { @@ -478,7 +478,7 @@ func TestTLSConfig(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, tt.clientCert, tt.clientKey, tt.clientCA, -1, false) + wh, err := newImagePolicyWebhook(t, server.URL, tt.clientCert, tt.clientKey, tt.clientCA, -1, false) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -559,7 +559,7 @@ func TestWebhookCache(t *testing.T) { defer s.Close() // Create an admission controller that caches successful responses. - wh, err := newImagePolicyWebhook(s.URL, clientCert, clientKey, caCert, 200, false) + wh, err := newImagePolicyWebhook(t, s.URL, clientCert, clientKey, caCert, 200, false) if err != nil { t.Fatal(err) } @@ -753,7 +753,7 @@ func TestContainerCombinations(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, false) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, false) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -847,7 +847,7 @@ func TestDefaultAllow(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, tt.defaultAllow) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, tt.defaultAllow) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -954,7 +954,7 @@ func TestAnnotationFiltering(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, true) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return @@ -1047,7 +1047,7 @@ func TestReturnedAnnotationAdd(t *testing.T) { } defer server.Close() - wh, err := newImagePolicyWebhook(server.URL, clientCert, clientKey, caCert, 0, true) + wh, err := newImagePolicyWebhook(t, server.URL, clientCert, clientKey, caCert, 0, true) if err != nil { t.Errorf("%s: failed to create client: %v", tt.test, err) return