Skip to content

Commit

Permalink
Fix kubernetes-client#1446 without introducing IDisposable on `Kube…
Browse files Browse the repository at this point in the history
…rnetesClientConfiguration`

Breaking Changes:
* Using `KubernetesClientConfiguration.SslCaCerts` or `KubernetesClientConfiguration.LoadSslCaCerts()` requires the user to dispose the returned `X509Certificate2Collection` after usage.
* Setter of `KubernetesClientConfiguration.SslCaCerts` was removed.
  • Loading branch information
PSanetra committed Nov 10, 2023
1 parent 15e7fda commit 62477b3
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 11 deletions.
11 changes: 2 additions & 9 deletions src/KubernetesClient/KubernetesClientConfiguration.ConfigFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Diagnostics;
using System.Net;
using System.Runtime.InteropServices;
using System.Security.Cryptography.X509Certificates;

namespace k8s
{
Expand Down Expand Up @@ -306,17 +305,11 @@ private void SetClusterDetails(K8SConfiguration k8SConfig, Context activeContext
{
if (!string.IsNullOrEmpty(clusterDetails.ClusterEndpoint.CertificateAuthorityData))
{
// This null password is to change the constructor to fix this KB:
// https://support.microsoft.com/en-us/topic/kb5025823-change-in-how-net-applications-import-x-509-certificates-bf81c936-af2b-446e-9f7a-016f4713b46b
string nullPassword = null;
var data = clusterDetails.ClusterEndpoint.CertificateAuthorityData;
SslCaCerts = new X509Certificate2Collection(new X509Certificate2(Convert.FromBase64String(data), nullPassword));
CaCertificateData = Convert.FromBase64String(clusterDetails.ClusterEndpoint.CertificateAuthorityData);
}
else if (!string.IsNullOrEmpty(clusterDetails.ClusterEndpoint.CertificateAuthority))
{
SslCaCerts = new X509Certificate2Collection(new X509Certificate2(GetFullPath(
k8SConfig,
clusterDetails.ClusterEndpoint.CertificateAuthority)));
CaCertificateFullFilePath = GetFullPath(k8SConfig, clusterDetails.ClusterEndpoint.CertificateAuthority);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static KubernetesClientConfiguration InClusterConfig()
{
Host = new UriBuilder("https", host, Convert.ToInt32(port)).ToString(),
TokenProvider = new TokenFileAuth(Path.Combine(ServiceAccountPath, ServiceAccountTokenKeyFileName)),
SslCaCerts = CertUtils.LoadPemFileCert(rootCAFile),
CaCertificateFullFilePath = rootCAFile,
};

var namespaceFile = Path.Combine(ServiceAccountPath, ServiceAccountNamespaceFileName);
Expand Down
83 changes: 82 additions & 1 deletion src/KubernetesClient/KubernetesClientConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace k8s
/// </summary>
public partial class KubernetesClientConfiguration
{
private string caCertificateFullFilePath;
private byte[] caCertificateData;
private JsonSerializerOptions jsonSerializerOptions;

/// <summary>
Expand All @@ -24,7 +26,60 @@ public partial class KubernetesClientConfiguration
/// <summary>
/// Gets SslCaCerts
/// </summary>
public X509Certificate2Collection SslCaCerts { get; set; }
/// <remarks>The user of this property is responsible to dispose all elements in the returned <see cref="X509Certificate2Collection"/></remarks>
/// <exception cref="CryptographicException">An error with the certificate occurs. For example: - The certificate file does not exist. - The certificate is invalid. - The certificate's password is incorrect</exception>
[Obsolete("Use LoadSslCaCerts() instead.")]
public X509Certificate2Collection SslCaCerts => LoadSslCaCerts();

/// <summary>
/// Gets or sets absolute to to a cluster ca certificate
/// </summary>
/// <exception cref="ArgumentException">Thrown if assigned path is not rooted.</exception>
public string CaCertificateFullFilePath
{
get
{
return caCertificateFullFilePath;
}

set
{
if (value != null && !Path.IsPathRooted(value))
{
throw new ArgumentException("Path is not rooted!", nameof(value));
}

caCertificateFullFilePath = value;
}
}

/// <summary>
/// Gets or sets ClientCertificateData
/// </summary>
/// <exception cref="CryptographicException">An error with the certificate occurs. For example: - The certificate file does not exist. - The certificate is invalid. - The certificate's password is incorrect</exception>
public ReadOnlySpan<byte> CaCertificateData
{
get
{
return caCertificateData;
}

set
{
if (value == null)
{
caCertificateData = null;
return;
}

var data = value.ToArray();

// Load once to ensure validity assigned data
using var certificate = new X509Certificate2(data);

caCertificateData = value.ToArray();
}
}

/// <summary>
/// Gets ClientCertificateData
Expand Down Expand Up @@ -148,5 +203,31 @@ public void AddJsonOptions(Action<JsonSerializerOptions> configure)

configure(JsonSerializerOptions);
}

/// <summary>
/// Returns a <see cref="X509Certificate2Collection"/> of <see cref="X509Certificate2"/> instances.
/// </summary>
/// <remarks>
/// The caller is responsible to dispose all elements in the returned <see cref="X509Certificate2Collection"/>.
/// </remarks>
/// <exception cref="CryptographicException">An error with the certificate occurs. For example: - The certificate file does not exist. - The certificate is invalid. - The certificate's password is incorrect</exception>
/// <returns>A <see cref="X509Certificate2Collection"/> of <see cref="X509Certificate2"/> instances.</returns>
public X509Certificate2Collection LoadSslCaCerts()
{
if (CaCertificateData != null)
{
// This null password is to change the constructor to fix this KB:
// https://support.microsoft.com/en-us/topic/kb5025823-change-in-how-net-applications-import-x-509-certificates-bf81c936-af2b-446e-9f7a-016f4713b46b
string nullPassword = null;
return new X509Certificate2Collection(new X509Certificate2(CaCertificateData, nullPassword));
}

if (!string.IsNullOrEmpty(CaCertificateFullFilePath))
{
return new X509Certificate2Collection(new X509Certificate2(caCertificateFullFilePath));
}

return new X509Certificate2Collection();
}
}
}

0 comments on commit 62477b3

Please sign in to comment.