Skip to content

Commit

Permalink
Fix double revision in istio configmap name
Browse files Browse the repository at this point in the history
  • Loading branch information
nrfox committed Oct 5, 2023
1 parent 40e8179 commit 32e776a
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 5 deletions.
25 changes: 21 additions & 4 deletions business/mesh.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,7 @@ func (in *MeshService) GetMesh(ctx context.Context) (*Mesh, error) {
Revision: istiod.Labels[IstioRevisionLabel],
}

configMapName := in.conf.ExternalServices.Istio.ConfigMapName
if revLabel := istiod.Labels[IstioRevisionLabel]; revLabel != "default" && revLabel != "" {
configMapName = configMapName + "-" + revLabel
}
configMapName := IstioConfigMapName(in.conf, controlPlane.Revision)

controlPlaneConfig, err := getControlPlaneConfiguration(kubeCache, istiod.Namespace, configMapName)
if err != nil {
Expand Down Expand Up @@ -226,6 +223,26 @@ func (in *MeshService) GetMesh(ctx context.Context) (*Mesh, error) {
return mesh, nil
}

// IstioConfigMapName guesses the istio configmap name.
func IstioConfigMapName(conf config.Config, revision string) string {
// If the config map name is explicitly set and it's not the default value, we should always use that.
// Note that this means that the revision is ignored and every controlplane
// will use this configmap regardless of which configmap actually corresponds
// to the revision.
if conf.ExternalServices.Istio.ConfigMapName != "" && conf.ExternalServices.Istio.ConfigMapName != "istio" {
return conf.ExternalServices.Istio.ConfigMapName
}

// If the revision is set, we should use the revisioned configmap name
// otherwise the hardcoded 'istio' value is used.
configMapName := "istio" // As of 1.19 this is hardcoded in the helm charts.
if revision != "default" && revision != "" {
configMapName = configMapName + "-" + revision
}

return configMapName
}

func envVarIsSet(key string, env core_v1.EnvVar) bool {
return env.Name == key && env.Value == "true"
}
Expand Down
86 changes: 85 additions & 1 deletion business/mesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ trustDomain: cluster.local
}
require := require.New(t)
conf := config.NewConfig()
config.Set(conf)
k8s := kubetest.NewFakeK8sClient(
&core_v1.Namespace{ObjectMeta: v1.ObjectMeta{Name: "istio-system"}},
istiod_1_18_Deployment,
Expand All @@ -668,7 +669,7 @@ trustDomain: cluster.local
istio_1_19_ConfigMap,
)

business.SetupBusinessLayer(t, k8s, *config.NewConfig())
business.SetupBusinessLayer(t, k8s, *conf)

clients := map[string]kubernetes.ClientInterface{conf.KubernetesConfig.ClusterName: k8s}
svc := business.NewWithBackends(clients, clients, nil, nil).Mesh
Expand All @@ -687,6 +688,18 @@ trustDomain: cluster.local
})
require.True(*controlPlane_1_19.Config.EnableAutoMtls)
require.Len(controlPlane_1_19.ManagedClusters, 1)

// Test for setting the configmap name explicitly due to regression: https://github.com/kiali/kiali/issues/6669
conf.ExternalServices.Istio.ConfigMapName = istio_1_19_ConfigMap.Name
config.Set(conf)
svc = business.NewWithBackends(clients, clients, nil, nil).Mesh
mesh, err = svc.GetMesh(context.TODO())
require.NoError(err)

require.Len(mesh.ControlPlanes, 2)
// Both controlplanes should set this to true since both will use the 1.19 configmap.
require.True(*mesh.ControlPlanes[0].Config.EnableAutoMtls)
require.True(*mesh.ControlPlanes[1].Config.EnableAutoMtls)
}

func TestGetMeshRemoteClusters(t *testing.T) {
Expand Down Expand Up @@ -1016,3 +1029,74 @@ func fakeIstiodDeployment(cluster string, manageExternal bool) *apps_v1.Deployme
}
return deployment
}

func TestIstioConfigMapName(t *testing.T) {
testCases := map[string]struct {
conf config.Config
revision string
expected string
}{
"ConfigMapName is empty and revision is default": {
conf: config.Config{
ExternalServices: config.ExternalServices{
Istio: config.IstioConfig{
ConfigMapName: "",
},
},
},
revision: "default",
expected: "istio",
},
"ConfigMapName is empty and revision is v1": {
conf: config.Config{
ExternalServices: config.ExternalServices{
Istio: config.IstioConfig{
ConfigMapName: "",
},
},
},
revision: "v1",
expected: "istio-v1",
},
"ConfigMapName is set and revision is default": {
conf: config.Config{
ExternalServices: config.ExternalServices{
Istio: config.IstioConfig{
ConfigMapName: "my-istio-config",
},
},
},
revision: "default",
expected: "my-istio-config",
},
"ConfigMapName is set and revision is v2": {
conf: config.Config{
ExternalServices: config.ExternalServices{
Istio: config.IstioConfig{
ConfigMapName: "my-istio-config",
},
},
},
revision: "v2",
expected: "my-istio-config",
},
"ConfigMapName is set and revision is empty": {
conf: config.Config{
ExternalServices: config.ExternalServices{
Istio: config.IstioConfig{
ConfigMapName: "my-istio-config",
},
},
},
revision: "",
expected: "my-istio-config",
},
}

for desc, tc := range testCases {
t.Run(desc, func(t *testing.T) {
result := business.IstioConfigMapName(tc.conf, tc.revision)
assert.Equal(t, tc.expected, result)
})
}
}

0 comments on commit 32e776a

Please sign in to comment.