Improve error messaging with OpenSSL #17
Conversation
src/Kubernetes.Auth.cs
Outdated
| else if (!string.IsNullOrWhiteSpace(config.ClientCertificateData) && !string.IsNullOrWhiteSpace(config.ClientCertificateKey)) | ||
| { | ||
| var pfxFilePath = await Utils.GeneratePfxAsync(config).ConfigureAwait(false); | ||
| if (string.IsNullOrWhiteSpace(pfxFilePath)) |
There was a problem hiding this comment.
FYI. Updated GeneratePfxAsync to always return a path which makes this check moot.
|
LGTM. Thank you @jpoon! |
src/KubernetesClientConfiguration.cs
Outdated
| else | ||
|
|
||
| if (!cluster.ClusterEndpoint.SkipTlsVerify && | ||
| string.IsNullOrWhiteSpace(cluster.ClusterEndpoint.CertificateAuthorityData)) |
There was a problem hiding this comment.
Certificate Authority is not actually required (imagine for example that a user uses a certificate signed by a generally accepted authority [e.g. let's encrypt]) it will all just work because of the default roots.
There was a problem hiding this comment.
It seems you extended the null check to add certificate-data in your last PR. Should we leave it as is or considering all these fields are optional remove this check entirely?
|
@jpoon LGTM, one small comment about certificate authority not being required. Thanks! |
|
(oh and it needs a rebase now that I merged my PR. Sorry!) |
`KubernetesClientConfiguration`
|
Thanks @brendanburns. Rebased, squashed commits, and replied to your comment. |
|
LGTM, thanks. |
KubernetesClientConfigurationgetting rid of some duplicate logicOpenSSL