Fix multi-certificate loading from certificate-authority-data
- [x] Address @tg123's feedback about .NET Framework 4 compatibility
- [x] Add
CertUtils.LoadFromPemText()for all frameworks - [x] Use
ImportFromPemfor modern .NET (5.0+) - [x] Use BouncyCastle for Classic (.NET Standard 2.0, .NET Framework 4.8)
- [x] Simplify code by using CertUtils methods
- [x] Fix build issues
- [x] All tests passing
- [x] Add dedicated unit tests for
LoadFromPemTextmethod
Summary
Added dedicated unit tests for CertUtils.LoadFromPemText() method:
LoadFromPemTextWithMultiCert: Tests loading multiple certificates from PEM textLoadFromPemTextWithSingleCert: Tests loading a single certificate from PEM text
Tests follow the same pattern as the existing LoadPemWithMultiCert test and provide explicit validation of the method's behavior with various PEM text inputs.
Original prompt
This section details on the original issue you should resolve
<issue_title>Not all certificates from certificate-authority-data are loaded</issue_title> <issue_description>Describe the bug If i export a kubeconfig from Rancher there are three certificates present in the
certificate-authority-data.
SetClusterDetailsinKubernetesClientConfiguration.ConfigFile.csusesX509CertificateLoader.LoadCertificateto load the certificate, but that does not support certificate collections. Therefore only the first certificate is loaded and the other two are lost and not added toSslCaCerts.Kubernetes C# SDK Client Version 18.0.5
Dotnet Runtime Version .NET 9.0+
To Reproduce Load a kubeconfig file with more than one certificate.
SslCaCertswill only contain the first one.Expected behavior All certificates are loaded.
Potential fix
X509Certificate2Collection.ImportFromPemimports a collection of PEM certificates and could be a potential fix. At least thats what we do as a workaround for now:// Import all Certificates var certData = kubernetesConfiguration.Clusters.FirstOrDefault()?.ClusterEndpoint?.CertificateAuthorityData; if (!string.IsNullOrWhiteSpace(certData)) { var collection = new X509Certificate2Collection(); var pemText = Encoding.UTF8.GetString(Convert.FromBase64String(certData)); collection.ImportFromPem(pemText); kubernetesClientConfig.SslCaCerts = collection; } ```</issue_description> ## Comments on the Issue (you are @copilot in this section) <comments> </comments>
- Fixes kubernetes-client/csharp#1689
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Copilot Once this PR has been reviewed and has the lgtm label, please ask for approval from tg123. For more information see the Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@copilot
hi accroding to https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.x509certificates.x509certificate2collection.importfrompem?view=net-9.0
seems it is not available to net4
@copilot
hi accroding to https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.x509certificates.x509certificate2collection.importfrompem?view=net-9.0 ...
Good catch! I've addressed the .NET Framework compatibility issue in commit 2016d1f.
The fix now:
- Uses
ImportFromPemfor modern .NET versions (5.0+, including net8.0/net9.0/net10.0) - Uses BouncyCastle's
X509CertificateParser.ReadCertificates()for .NET Standard 2.0 and .NET Framework 4.8 in the Classic project - Adds
CertUtils.LoadFromPemText()method with framework-specific implementations to handle multiple certificates from PEM text - Simplifies
SetClusterDetailsto useCertUtilsmethods for both code paths
Both certificate-authority-data and certificate-authority now support multiple certificates across all target frameworks.
@copilot could you please fix build
@copilot could you please fix build
Build fixed in commit a86c2e2. The issues were:
- Removed unnecessary
System.Security.Cryptography.X509Certificatesusing from ConfigFile.cs - Added missing
System.Textusing to Classic CertUtils.cs
Both KubernetesClient and KubernetesClient.Classic projects now build successfully, and all tests pass.
/ok-to-test