Dispose certificates in Kubernetes.Dispose() (#1191)

shenglol · web-flow · commit e1508370f596 · 2023-02-01T15:47:29.000-08:00
* Dispose certs created by Kuberentes

* Update tests
diff --git a/src/KubernetesClient/Kubernetes.ConfigInit.cs b/src/KubernetesClient/Kubernetes.ConfigInit.cs
@@ -90,27 +90,27 @@ private void InitializeFromConfig(KubernetesClientConfiguration config)
             // set credentails for the kubernetes client
             SetCredentials(config);
 
-            var clientCert = CertUtils.GetClientCert(config);
-            if (clientCert != null)
+            ClientCert = CertUtils.GetClientCert(config);
+            if (ClientCert != null)
             {
 #if NET5_0_OR_GREATER
-                HttpClientHandler.SslOptions.ClientCertificates.Add(clientCert);
+                HttpClientHandler.SslOptions.ClientCertificates.Add(ClientCert);
 
                 // TODO this is workaround for net7.0, remove it when the issue is fixed
                 // seems the client certificate is cached and cannot be updated
                 HttpClientHandler.SslOptions.LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, acceptableIssuers) =>
                 {
-                    return clientCert;
+                    return ClientCert;
                 };
 #else
-                HttpClientHandler.ClientCertificates.Add(clientCert);
+                HttpClientHandler.ClientCertificates.Add(ClientCert);
 #endif
             }
         }
 
         private X509Certificate2Collection CaCerts { get; }
 
-        private X509Certificate2 ClientCert { get; }
+        private X509Certificate2 ClientCert { get; set; }
 
         private bool SkipTlsVerify { get; }
 
diff --git a/src/KubernetesClient/Kubernetes.WebSocket.cs b/src/KubernetesClient/Kubernetes.WebSocket.cs
@@ -239,11 +239,6 @@ protected async Task<WebSocket> StreamConnectAsync(Uri uri, string webSocketSubP
             }
 
             // Set Credentials
-            if (this.ClientCert != null)
-            {
-                webSocketBuilder.AddClientCertificate(this.ClientCert);
-            }
-
             if (this.HttpClientHandler != null)
             {
 #if NET5_0_OR_GREATER
diff --git a/src/KubernetesClient/Kubernetes.cs b/src/KubernetesClient/Kubernetes.cs
@@ -202,12 +202,27 @@ public void Dispose()
         /// <param name="disposing">True to release both managed and unmanaged resources; false to releases only unmanaged resources.</param>
         protected virtual void Dispose(bool disposing)
         {
-            if (!_disposed)
+            if (disposing && !_disposed)
             {
                 _disposed = true;
 
                 // Dispose the client
                 HttpClient?.Dispose();
+
+                // Dispose the certificates
+                if (CaCerts is not null)
+                {
+                    foreach (var caCert in CaCerts)
+                    {
+                        caCert.Dispose();
+                    }
+
+                    CaCerts.Clear();
+                }
+
+
+                ClientCert?.Dispose();
+
                 HttpClient = null;
                 FirstMessageHandler = null;
                 HttpClientHandler = null;
diff --git a/tests/E2E.Tests/MinikubeTests.cs b/tests/E2E.Tests/MinikubeTests.cs
@@ -28,7 +28,7 @@ public void SimpleTest()
             var namespaceParameter = "default";
             var podName = "k8scsharp-e2e-pod";
 
-            var client = CreateClient();
+            using var client = CreateClient();
 
             void Cleanup()
             {
@@ -79,7 +79,7 @@ public void PatchTest()
             var namespaceParameter = "default";
             var podName = "k8scsharp-e2e-patch-pod";
 
-            var client = CreateClient();
+            using var client = CreateClient();
 
             void Cleanup()
             {
@@ -183,7 +183,7 @@ void Cleanup()
         [MinikubeFact]
         public async Task WatcherIntegrationTest()
         {
-            var kubernetes = CreateClient();
+            using var kubernetes = CreateClient();
 
             var job = await kubernetes.BatchV1.CreateNamespacedJobAsync(
                 new V1Job()
@@ -251,7 +251,7 @@ public async Task WatcherIntegrationTest()
         [MinikubeFact]
         public void LeaderIntegrationTest()
         {
-            var client = CreateClient();
+            using var client = CreateClient();
             var namespaceParameter = "default";
 
             void Cleanup()
@@ -350,7 +350,7 @@ public async Task LogStreamTestAsync()
             var namespaceParameter = "default";
             var podName = "k8scsharp-e2e-logstream-pod";
 
-            var client = CreateClient();
+            using var client = CreateClient();
 
             void Cleanup()
             {
@@ -446,7 +446,7 @@ async Task<V1Pod> Pod()
         [MinikubeFact]
         public async Task DatetimeFieldTest()
         {
-            var kubernetes = CreateClient();
+            using var kubernetes = CreateClient();
 
             await kubernetes.CoreV1.CreateNamespacedEventAsync(
                 new Corev1Event(
@@ -478,7 +478,7 @@ public async void GenericTest()
             var namespaceParameter = "default";
             var podName = "k8scsharp-e2e-generic-pod";
 
-            var client = CreateClient();
+            using var client = CreateClient();
             var genericPods = new GenericClient(client, "", "v1", "pods");
 
             void Cleanup()
@@ -590,7 +590,7 @@ public async Task CopyToPodTestAsync()
             var namespaceParameter = "default";
             var podName = "k8scsharp-e2e-cp-pod";
 
-            var client = CreateClient();
+            using var client = CreateClient();
 
             async Task<int> CopyFileToPodAsync(string name, string @namespace, string container, Stream inputFileStream, string destinationFilePath, CancellationToken cancellationToken = default(CancellationToken))
             {
diff --git a/tests/Kubectl.Tests/KubectlTests.Version.cs b/tests/Kubectl.Tests/KubectlTests.Version.cs
@@ -1,4 +1,5 @@
 using k8s.E2E;
+using k8s.kubectl.beta;
 using System.Text.Json;
 using Xunit;
 
@@ -9,7 +10,8 @@ public partial class KubectlTests
     [MinikubeFact]
     public void Version()
     {
-        var client = CreateClient();
+        using var kubernetes = MinikubeTests.CreateClient();
+        var client = new Kubectl(kubernetes);
         var version = client.Version();
         var serverobj = version.ServerVersion;
 
diff --git a/tests/Kubectl.Tests/KubectlTests.cs b/tests/Kubectl.Tests/KubectlTests.cs
@@ -1,16 +1,9 @@
-using k8s.E2E;
-using k8s.kubectl.beta;
 using System.Diagnostics;
 
 namespace k8s.kubectl.Tests;
 
 public partial class KubectlTests
 {
-    private Kubectl CreateClient()
-    {
-        return new Kubectl(MinikubeTests.CreateClient());
-    }
-
     private string RunKubectl(string args)
     {
         var p = new Process
diff --git a/tests/KubernetesClient.Tests/CertUtilsTests.cs b/tests/KubernetesClient.Tests/CertUtilsTests.cs
@@ -29,7 +29,7 @@ public void LoadFromFiles()
                 useRelativePaths: false);
 
             // Just validate that this doesn't throw and private key is non-null
-            var cert = CertUtils.GeneratePfx(cfg);
+            using var cert = CertUtils.GeneratePfx(cfg);
             Assert.NotNull(cert.GetRSAPrivateKey());
         }
 
@@ -44,7 +44,7 @@ public void LoadFromFilesRelativePath()
                 "federal-context");
 
             // Just validate that this doesn't throw and private key is non-null
-            var cert = CertUtils.GeneratePfx(cfg);
+            using var cert = CertUtils.GeneratePfx(cfg);
             Assert.NotNull(cert.GetRSAPrivateKey());
         }
 
@@ -58,7 +58,7 @@ public void LoadFromInlineData()
                 useRelativePaths: false);
 
             // Just validate that this doesn't throw and private key is non-null
-            var cert = CertUtils.GeneratePfx(cfg);
+            using var cert = CertUtils.GeneratePfx(cfg);
             Assert.NotNull(cert.GetRSAPrivateKey());
         }
 
@@ -73,7 +73,7 @@ public void LoadFromInlineDataRelativePath()
                 "victorian-context");
 
             // Just validate that this doesn't throw and private key is non-null
-            var cert = CertUtils.GeneratePfx(cfg);
+            using var cert = CertUtils.GeneratePfx(cfg);
             Assert.NotNull(cert.GetRSAPrivateKey());
         }
 
@@ -85,8 +85,8 @@ public void LoadPemWithMultiCert()
         {
             var certCollection = CertUtils.LoadPemFileCert("assets/ca-bundle.crt");
 
-            var intermediateCert = new X509Certificate2("assets/ca-bundle-intermediate.crt");
-            var rootCert = new X509Certificate2("assets/ca-bundle-root.crt");
+            using var intermediateCert = new X509Certificate2("assets/ca-bundle-intermediate.crt");
+            using var rootCert = new X509Certificate2("assets/ca-bundle-root.crt");
 
             Assert.Equal(2, certCollection.Count);
 

Original file line number	Diff line number	Diff line change
`@@ -239,11 +239,6 @@ protected async Task<WebSocket> StreamConnectAsync(Uri uri, string webSocketSubP`
`239`	`239`	`}`
`240`	`240`
`241`	`241`	`// Set Credentials`
`242`		`- if (this.ClientCert != null)`
`243`		`- {`
`244`		`- webSocketBuilder.AddClientCertificate(this.ClientCert);`
`245`		`- }`
`246`		`-`
`247`	`242`	`if (this.HttpClientHandler != null)`
`248`	`243`	`{`
`249`	`244`	`#if NET5_0_OR_GREATER`