-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conflicting vendored version of mergo breaks k8s config merging #1670
Comments
I cannot reproduce this problem. Using the following kubeconfig file both
I have also tried putting the certificate in a separate file (
|
It turns out that the problem happens when using This works on kubectl 1.13.0 but fails on fluxctl 1.10.1 My guess is that this is coming from a bug in a Go library whose dependency we haven't bumped yet. |
This is still a mystery. It works with kubectl, back to version |
I think I found the culprit at https://github.com/kubernetes/client-go/blob/v8.0.0/tools/clientcmd/loader.go#L227-L229 config := clientcmdapi.NewConfig()
mergo.Merge(config, mapConfig)
mergo.Merge(config, nonMapConfig) The code above results in the certificate being duplicated if it doesn't have an EOL at the end :S |
Actually, it always concats the certificate with itself but, if there is an EOL, then it complies with the pem format. |
TLDR: this is caused by a vendoring mess, and will be fixed after bumping the kubernetes go client to version 10.0.0
The problem is that we are vendoring a version of https://github.com/imdario/mergo with different operation semantics than the one expected by the version of https://github.com/kubernetes/client-go we vendor. Specifically, the semantics of the This explains why this problem didn't surface with We vendored version 0.3.2 of mergo at 6635484#diff-bd247e83efc3c45ae9e8c47233249f18R176 but I still can't understand why that version was chosen instead of respecting the requirement from https://github.com/kubernetes/client-go. Good news is that https://github.com/kubernetes/client-go already upgraded to mergo 0.3.5 (downstreamed from kubernetes/kubernetes#65779 and included in the 10.0.0 release), adapting the code to the new Mergo semantics. This means we can fix the problem by bumping our vendored https://github.com/kubernetes/client-go to version 10.0.0 As a final pun to myself ... this is the second time I run into this problem, but I didn't remember darccio/mergo#33 ... sigh |
Marking as blocked until we try to bump the kubernetes client library. I may give it a go soon. |
Reported by user Andrew Lewis on Slack:
The text was updated successfully, but these errors were encountered: