Check enablement from values part as well #727
Conversation
istio-testing
added
do-not-merge/work-in-progress
googlebot
added
the
cla: yes
richardwxn
changed the title
istio-testing
removed
the
do-not-merge/work-in-progress
| @@ -103,6 +103,22 @@ var ( | |||
| KialiComponentName: ThirdPartyFeatureName, | |||
| TracingComponentName: ThirdPartyFeatureName, | |||
| } | |||
| // ComponentNameToHelmComponentPath defines mapping from component name to helm component root path. | |||
There was a problem hiding this comment.
should be fairly easy to create this programmatically, maybe when you create a translator/reverse translator?
There was a problem hiding this comment.
The problem is that the checkenablement part does not depend on translator so I don’t want to get this mapping from the translator, not sure whether there is better way possibly?
There was a problem hiding this comment.
It probably should depend on it, or actually should go into Translator. Name is a very low level pkg and shouldn't know anything about things like helm so anyway this function probably doesn't belong there now.
There was a problem hiding this comment.
@ostromart translation is the process to convert between operator spec and helm values spec, however to get enablement values or some other paths from one of them does not reply on the other, right? I think it is ok to put them in names now as it does not need a process of translation. I add a TODO to refactor this part later but hope to get this merged soon to fix related bugs first.
|
Let's hold off merging this till #713 goes in - I'm concerned I may break something in this change if I merge into that PR. |
richardwxn
force-pushed
the
check-enablement-values
branch
from
4d0fd12
to
22db95f
Compare
richardwxn
added
the
do-not-merge/work-in-progress
|
@richardwxn please move this over to istio/istio since istio/operator has now been merged there. |
should help resolve istio/istio#19799