Double istio rev in configmap name

[laptubi]

Hello.

istio installed via helm

  values:
    revision: "1-18-0"

kiali

  values:
    allowAdHocKialiImage: true
    resources:
      limits:
        cpu: 1
    cr:
      create: true
      namespace: istio-system
      spec:
        istio_namespace: istio-system
        external_services:
          istio:
            config_map_name: "istio-1-18-0"
            istiod_deployment_name: "istiod-1-18-0"
            istio_sidecar_injector_config_map_name: "istio-sidecar-injector-1-18-0"

i receive this error from kiali log.

ERR Error getting mesh config: configmap "istio-1-18-0-1-18-0" not found

i do not understand why rev is doubled in configmap name?

[jmazzitelli]

What version of Kiali did you install?

What does your Kiali server pods logs say - any other errors or stack traces?

[laptubi]

1.73 version
no other errors in pod logs

[jmazzitelli]

I found the problem. For some reason, this code appends the version: https://github.com/kiali/kiali/blob/v1.74.1/business/mesh.go#L158-L161

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

But no where else in the code does it do this - anywhere else ExternalServices.Istio.ConfigMapName is used, it is used as-is and the istiod revision label is not appended to it.

We need to fix the Kiali code to be consistent. Right now, it is broken because it does things differently in different parts of the code.

[jmazzitelli]

@nrfox this was added in this PR from a couple months ago: https://github.com/kiali/kiali/pull/6473/files#diff-7d9539fc4723cc9b3288a3a7d8e40a1b674812037abef2b5a720ba87256cbf61R159

[nrfox]

Looks like this also goes against the documentation for how to handle revisions in Kiali so it should be fixed. Also need an e2e test added for this.

[jmazzitelli]

@leandroberetta knows about all this stuff - he did all that work on the canary stuff. Perhaps he can chime in with his thoughts.

[jmazzitelli]

BTW: I wrote a github issue a few months ago that is related - we need a common function to get the configmap name - precisely because of this canary/version information that might be in the name.

#6042

[aleksandermiszkiewicz]

@jmazzitelli @nrfox when the bug fix will be implemented ? Also would like to understand what issue it causes on the Kiali side (if any) ?

[jmazzitelli]

when the bug fix will be implemented ?

Follow this PR - #6675 and if it gets merged by end of week, it will be in this sprint's 1.76 release to be published Monday.

[nrfox]

Also would like to understand what issue it causes on the Kiali side (if any) ?

@aleksandermiszkiewicz from what I can tell, the only thing that was affected by this issue was validations for Gateways. If you were using PILOT_SCOPE_GATEWAY_TO_NAMESPACE = true, then you might have gotten a false negative (validation passed when it should've failed) if your Gateway selected a workload outside of the Gateway's namespace.

[aleksandermiszkiewicz]

@jmazzitelli @nrfox thank you guys for answers.

[jmazzitelli]

the fix needs to go into the 1.73 branch. backport PR: #6973

[jmazzitelli]

backport merged