Revert common cfgmap func and protect against failure #6791
Conversation
This reverts the callers of IstioConfigMapName back to using `cfg.external_services.istio.config_map_name` directly. It now logs failures in GetMesh to auto-detect the configmap instead of returning an error. Nothing is currently using that configuration so an error shouldn't be returned there. Related to: kiali#6669.
|
cc @yohanb if you want to take a look. Basically this change does 2 things:
Another option is to just rip out fetching the guessed configmap entirely. I kept it since it will be used for in progress future work for #6035 but really it doesn't need to be there until it is being used directly. |
| @@ -103,7 +103,7 @@ func (ics *IstioCertsService) getCertificateFromSecret(secretName, certName stri | |||
| func (ics *IstioCertsService) getCertsConfigFromIstioConfigMap() ([]certConfig, error) { | |||
| cfg := config.Get() | |||
|
|
|||
| istioConfigMap, err := ics.k8s.GetConfigMap(cfg.IstioNamespace, IstioConfigMapName(*cfg, "")) | |||
| istioConfigMap, err := ics.k8s.GetConfigMap(cfg.IstioNamespace, cfg.ExternalServices.Istio.ConfigMapName) | |||
There was a problem hiding this comment.
Why wouldn't we want to keep this common function here and in the common function just return cfg.ExternalServices.Istio.ConfigMapName... in the future, if anything needs to change (like trying to get the name automagically) you only have to change this function and everywhere the name needs to be obtained will just work.
With this revert, if we ever have to change this, we need to go through all these places again and change it back to calling a common function.
I would leave the common function here - and for now just have it simply return config.Get().ExternalServices.Istio.ConfigMapName
There was a problem hiding this comment.
Why wouldn't we want to keep this common function here and in the common function just return cfg.ExternalServices.Istio.ConfigMapName... in the future, if anything needs to change (like trying to get the name automagically) you only have to change this function and everywhere the name needs to be obtained will just work.
In the future, I would expect callers to call GetMesh to get the config info from there instead of fetching and unmarshaling the configmap every the config is needed.
jmazzitelli
dismissed
their stale review
removing the common function - just rely on the config name and hope we don't have to refactor this again :)
This reverts the callers of
IstioConfigMapNameback to usingcfg.external_services.istio.config_map_namedirectly. It now logs failures inGetMeshto auto-detect the configmap instead of returning an error. Nothing is currently using that configuration so an error shouldn't be returned there. Related to: #6669.