Use the system certificate store if no certificates are specified.

[marcusbooyah]

This is a fix for #621.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: marcusbooyah / name: Marcus Bowyer (f2be6f6)

[tg123]

i would say this is not included in kubectl

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (master@2af57ca). Click here to learn what that means.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff            @@
##             master    #1261   +/-   ##
=========================================
  Coverage          ?   69.96%           
=========================================
  Files             ?       95           
  Lines             ?     2690           
  Branches          ?        0           
=========================================
  Hits              ?     1882           
  Misses            ?      808           
  Partials          ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[marcusbooyah]

@tg123 an alternative would be to not set the ServerCertificateCustomValidationCallback at all.

kubectl doesn't require a CA to be set when TLS verification is disabled. It would make sense for this client to be the same. The current behavior is causing issues for us because we're using Let's Encrypt certificates for our API Server.

[tg123]

do you mean SkipTlsVerify=false and let sdk use cert store if cacert == null

[marcusbooyah]

do you mean SkipTlsVerify=false and let sdk use cert store if cacert == null

That's what this PR does

[tg123]

do you mean SkipTlsVerify=false and let sdk use cert store if cacert == null

That's what this PR does

why not put your letsencrypt ca into kubeconfig

[marcusbooyah]

I can do that. But since kubectl works without it I'm a bit confused as to why this client requires it. I think it would be better to have this client match kubectl behavior.

[jefflill]

This PR seems like a bit of a no-brainer. Having KubernetesClient be consistent with kubectl is a good idea and it would be nice if future developers didn't need to debug and then workaround things like this.

[tg123]

totally agree the sdk should have same behavior as kubectl,
let me double check what is the default behavior if no ca is set in kubectl

and btw I believe

 to not set the ServerCertificateCustomValidationCallback at all.

is better

[marcusbooyah]

@tg123 same, I updated the PR.

[tg123]

Thanks, I double confirmed you are right, should set callback to null in order to use system default.

here is how kubectlRset rootCA
https://github.com/kubernetes/client-go/blob/704cac462081473b521f15547302f001f78ef2e4/transport/transport.go#L98

// RootCAs defines the set of root certificate authorities
	// that clients use when verifying server certificates.
	// If RootCAs is nil, TLS uses the host's root CA set.
	RootCAs *[x509](https://pkg.go.dev/crypto/x509).[CertPool](https://pkg.go.dev/crypto/x509#CertPool)

[tg123]

/LGTM

cc @brendanburns
do you think we need to upgrade the ver for this?

[brendandburns]

/lgtm
/approve

No, I don't think this is a breaking change, I think it is more of a bug-fix. (honestly I doubt anyone will notice as few people use well-known CAs for the kube apiservers)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, marcusbooyah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [brendandburns]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment