Hi @steffengy. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: costinm

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

/test all [submit-queue is verifying that this PR is safe to merge]

Automatic merge from submit-queue.

the changes from this pr btw are:
istio-releases/vendor@fb63920

curious which line in that diff is the bug we are fixing ?

@ldemailly
darccio/mergo@0b8c27e
breaks this.

In hindsight that's quite logical that it's that commit, but bisecting works too:

package main

import (
	"bytes"
	"fmt"
	"github.com/imdario/mergo"
	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
)

func main() {
	mapConfig := clientcmdapi.NewConfig()
	cluster := clientcmdapi.NewCluster()
	cluster.CertificateAuthorityData = []byte("testdata")
	mapConfig.Clusters["test"] = cluster

	nonMapConfig := clientcmdapi.NewConfig()
	cluster = clientcmdapi.NewCluster()
	cluster.CertificateAuthorityData = []byte("testdata")
	nonMapConfig.Clusters["test"] = cluster

	config := clientcmdapi.NewConfig()
	mergo.Merge(config, mapConfig)
	mergo.Merge(config, nonMapConfig)

	fmt.Println(string(config.Clusters["test"].CertificateAuthorityData))
	authority := config.Clusters["test"].CertificateAuthorityData
	if bytes.Compare(authority, []byte("testdata")) != 0 {
		panic("CertificateAuthorityData does not match")
	}
}

ic, thanks a lot, I would report the bug in client go, as mergo quite clearly document their intent?

@ldemailly
The people on client-go are quite likely aware of this, since there seems to be quite a bit of behavior that changed in mergo.
https://github.com/kubernetes/client-go/blob/7cd1d3291b7d9b1e2d54d4b69eb65995eaf8888e/tools/clientcmd/client_config.go#L160-L162
is a good indicator for that, where they note that they explicitly for that reason use an old version
of mergo (tagging against it in godeps).

A comment at https://github.com/kubernetes/client-go/blob/b6a34c5a002893138005771f76480e7166da9310/tools/clientcmd/loader.go#L225-L227
to indicate this breaks with darccio/mergo@0b8c27e might make sense though.
Maybe even in the readme, indicating that one should make sure to respect the
dependency versions client-go requires (since the godep tagging will be ignored when using dep).

mergo should probably have bumped major or at least minor on that change
instead of a sha can we put an = constraint?

not sure what you mean by ignore by go dep btw

@ldemailly
I'd advise against that since mergo can cause issues at places where it obviously won't immediately be noticable.
The reason for tagging against that sha-hash is that it's surely known to work and consistent with what client-go does/expects: https://github.com/kubernetes/client-go/blob/35d357565b16ae43c8366248258d910df7c70841/Godeps/Godeps.json#L179.
With ignore godep I mean, that dep in istio doesn't pickup this tagging specified in the file linked above.