-
Notifications
You must be signed in to change notification settings - Fork 307
Fix multi-certificate loading from certificate-authority-data #1690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
|
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 |
- Add CertUtils.LoadFromPemText() method for both modern .NET and Classic - Use ImportFromPem for .NET 5.0+ (net8.0, net9.0, net10.0) - Use BouncyCastle for .NET Standard 2.0 and .NET Framework 4.8 - Simplify SetClusterDetails to use CertUtils methods for both certificate-authority-data and certificate-authority Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
Good catch! I've addressed the .NET Framework compatibility issue in commit 2016d1f. The fix now:
Both certificate-authority-data and certificate-authority now support multiple certificates across all target frameworks. |
|
@copilot could you please fix build |
- Remove unused System.Security.Cryptography.X509Certificates using from ConfigFile.cs - Add System.Text using to Classic CertUtils.cs for Encoding class Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
Build fixed in commit a86c2e2. The issues were:
Both KubernetesClient and KubernetesClient.Classic projects now build successfully, and all tests pass. |
CertUtils.LoadFromPemText()for all frameworksImportFromPemfor modern .NET (5.0+)Summary
Successfully fixed the build by:
System.Security.Cryptography.X509Certificatesusing fromKubernetesClientConfiguration.ConfigFile.cs(no longer needed since we use CertUtils methods)System.Textusing toKubernetesClient.Classic/CertUtils.csfor theEncodingclassBoth the main KubernetesClient and Classic projects now build successfully, and all multi-certificate tests pass.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.