Feat: Add support for arbitrary custom values in regions.json #286
Conversation
Custom values can be added to regions.json via the field "customValues" and will then be accessible in the helm chart
johnksv
force-pushed
the
feat-support-arbitrary-custom-values
branch
from
77601a8 to
5ef2dec
Compare
| @@ -240,6 +240,7 @@ private Collection<Object> publishApps( | |||
| User user = userProvider.getUser(region); | |||
| Map<String, Object> fusion = new HashMap<>(); | |||
| fusion.putAll((Map<String, Object>) requestDTO.getOptions()); | |||
| fusion.putAll(region.getServices().getCustomValues()); | |||
There was a problem hiding this comment.
I am not sur of this one.
I don't know why we should put that custom values here
@olevitt any thought ?
There was a problem hiding this comment.
Hi !
Adding the custom values here does not work as you expect.
This fusion map is supposed to have keys corresponding to the helm package values keys. There is almost no chance that the customValues keys exactly correspond to the helm package values keys.
The mapping between configuration keys and helm package specific keys is done in web app based on x-onyxia specified in values.schema.json file.
So I think this line should be removed
There was a problem hiding this comment.
Thanks for feedback!
The mapping between configuration keys and helm package specific keys is done in web app based on x-onyxia specified in values.schema.json file.
Do I understand you correctly if you think that the values from customValues should be sent to web such that they can be edited there, @olevitt ?
I would say one approach to providing custom values in regions.json is to do the mapping in API, and not let the user be able to override these custom values.
| @@ -240,6 +240,7 @@ private Collection<Object> publishApps( | |||
| User user = userProvider.getUser(region); | |||
| Map<String, Object> fusion = new HashMap<>(); | |||
| fusion.putAll((Map<String, Object>) requestDTO.getOptions()); | |||
| fusion.putAll(region.getServices().getCustomValues()); | |||
There was a problem hiding this comment.
Hi !
Adding the custom values here does not work as you expect.
This fusion map is supposed to have keys corresponding to the helm package values keys. There is almost no chance that the customValues keys exactly correspond to the helm package values keys.
The mapping between configuration keys and helm package specific keys is done in web app based on x-onyxia specified in values.schema.json file.
So I think this line should be removed
| @@ -65,6 +65,7 @@ Users can work on Onyxia as a User or as a Group to which they belong. Each user | |||
| | `quotas` | | Properties setting quotas on how many resources a user can get on the services provider. | See [Quotas properties](#quotas-properties) | | |||
| | `defaultConfiguration` | | Default configuration on services that a user can override. For client purposes only. | See [Default Configuration](#default-configuration-properties) | | |||
| | `customInitScript` | | This can be used to customize user environments using a regional script executed by some users' pods. | See [CustomInitScript properties](#custom-init-script-properties) | | |||
| | `customValues` | | This can be used to specify custom values thath should be avaiable in the underlaying helm chart when a service is launched. Nested values are supported. | ` "customValues": {"myCustomKey": "myValue", "myNestedCustomKey": {"nestedKey": "nestedValue"} }` | | |||
There was a problem hiding this comment.
typos in thath and underlaying.
I suggest we rephrase with something like :
This can be used to specify custom values that will be available for chart values injection (see catalog documentation). Nested values are supported.
|
Sorry for not updating this PR for the last couple of weeks. Other things have been taking my time.. A small update: For us this feature is a must to support multiple environments, so I hope someone on my team, or myself, can put in necessary time to implement this. |
Sure. I will take it from here. After that, next step will be on web app |
|
Superseeded by #298 |
Background: As we are developing our services helm charts we need to inject some values per region (values that are not a part of region.json today). We don’t want to make constant changes (and releases) to the onyxia-api (several reasons for that), and a suggestion would therefore be to have a field in regions.json where one can add arbitrary values to be injected/made available into the charts.
Solution proposed in this PR:
Custom values can be added to regions.json via the field "customValues" and will then be accessible in the helm chart.
E.g. in
regions.json:would result in the properties
myCustomKeyandmyNestedCustomKey.nestedKeybeing available in the helm chart:Should the field be named
customValuesor something else?Does the frontend need any changes to accommodate for this? I guess there might be use cases where these custom values would be configurable?