Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing json objects/arrays as values through --set #10428

Closed
lucadirocco opened this issue Dec 3, 2021 · 11 comments · Fixed by #10693
Closed

Passing json objects/arrays as values through --set #10428

lucadirocco opened this issue Dec 3, 2021 · 11 comments · Fixed by #10693
Labels
Milestone

Comments

@lucadirocco
Copy link
Contributor

lucadirocco commented Dec 3, 2021

I would like to set json objects/arrays as values through --set.
According to https://helm.sh/docs/intro/using_helm/#the-format-and-limitations-of---set
currently that is not possible (with the exception of arrays of scalar types)

As example, let's take the following excerpt from my values.yaml file:

spec:
  ....
  sNssaiUpfInfoList:
  - sNssai:
      sd: 0002f0
      sst: 1
    dnnUpfInfoList:
    - dnn: intranet
      ipv4AddressRanges:
      - end: 9.9.9.94
        start: 9.9.9.65
  ....

In the template there is all the "range" based logic for slices and maps, to correctly render sNssaiUpfInfoList from .Values.spec.sNssaiUpfInfoList, so further elements could be added to each of the above arrays and they would appear in the final manifest generated by helm.

In order to set sNssaiUpfInfoList above, currently I have to set the individual leaf attributes of the data structure, by providing the following with --set on command line:

'spec.sNssaiUpfInfoList[0].sNssai.sd=0002f0,spec.sNssaiUpfInfoList[0].sNssai.sst=1,spec.sNssaiUpfInfoList[0].dnnUpfInfoList[0].dnn=intranet, spec.sNssaiUpfInfoList[0].dnnUpfInfoList[0].ipv4AddressRanges[0].end=9.9.9.94, spec.sNssaiUpfInfoList[0].dnnUpfInfoList[0].ipv4AddressRanges[0].start=9.9.9.65'

This is not very flexible because extra logic is required to translate the data structure into individual key=val for each of its leaf attributes. Moreover, the arrays are dynamic, and quite a long list of key=val could be generated.

I think it would be much simpler if we had an option to set a complete json array/object on the command line.
For example, to set sNssaiUpfInfoList above, we could use something like:

--set-json 'spec.sNssaiUpfInfoList=[{"sNssai":{"sd":"0002f0","sst":1},"dnnUpfInfoList":[{"dnn":"intranet","ipv4AddressRanges":[{"end":"9.9.9.94","start":"9.9.9.65"}]}]}]'

Is it possible to add this option to helm?

@lucadirocco lucadirocco changed the title Passing json objects/arrays as value through --set Passing json objects/arrays as values through --set Dec 3, 2021
@mattfarina
Copy link
Collaborator

I think this would be possible.

@lucadirocco
Copy link
Contributor Author

Thank you for your reply.
Do you think it can be an interesting feature and get implemented?
In case, what the next steps would be?

@mattfarina
Copy link
Collaborator

@lucadirocco I think it's an interesting feature. The next step would be for someone who is interested in the feature to create a pull request and try to implement it. Anyone is welcome to do this.

@lucadirocco
Copy link
Contributor Author

lucadirocco commented Feb 16, 2022

Hello @mattfarina , I have implemented this feature already last month, but I haven't issued a pull request due to lack of time and info about possible conditions/processes to be fulfilled/followed before issuing a pull request.
Can you please share any links/docs that can help me with the info above?
I'm going to have some spare time in the coming weeks and I would like to complete the work and issue a pull request.
Thanks in advance.

Actually I was able to find the Contributing Guidelines: https://github.com/helm/helm/blob/main/CONTRIBUTING.md
(Besides the well known Developer Guide: https://helm.sh/docs/community/developers/)

@anurnomeru
Copy link

anurnomeru commented Feb 16, 2022

I am writing this pr, which can implement the json replacement for a key mentioned in the issues, but there are several problems here, and K8s has encountered similar problems.

And according to the existing helm design, the actual effect is not to replace the value of entire key, but to cover the specific value. In addition, ambiguity arises when dealing with arrays:

That is to say, the original values.yaml is as follows:

spec:
  ....
  sNssaiUpfInfoList:
  - sNssai:
      sd: 0002f0
      sst: 1
    dnnUpfInfoList:
    - dnn: intranet
      ipv4AddressRanges:
      - end: 9.9.9.94
        start: 9.9.9.65
  ....

After "--set-json 'spec.sNssaiUpfInfoList[0]=[{"dnnUpfInfoList":[{"dnn":"intranet","ipv4AddressRanges":[{"end":"9.9.9.100"}]}]}]'"

The result obtained is actually:

(append to the array dnnUpfInfoList)

spec:
  ....
  sNssaiUpfInfoList:
  - sNssai:
      sd: 0002f0
      sst: 1
    dnnUpfInfoList:

    - dnn: intranet
      ipv4AddressRanges:
      - end: 9.9.9.100

    - dnn: intranet
      ipv4AddressRanges:
      - end: 9.9.9.94
        start: 9.9.9.65
  ....

or (replace the original array dnnUpfInfoList):

spec:
  ....
  sNssaiUpfInfoList:
  - sNssai:
      sd: 0002f0
      sst: 1
    dnnUpfInfoList:

    - dnn: intranet
      ipv4AddressRanges:
      - end: 9.9.9.100

  ....

but not (merge by dnn: intranet):

spec:
  ....
  sNssaiUpfInfoList:
  - sNssai:
      sd: 0002f0
      sst: 1
    dnnUpfInfoList:

    - dnn: intranet
      ipv4AddressRanges:
      - end: 9.9.9.100
        start: 9.9.9.65
  ....

Because in fact, the values ​​of helm do not have a design similar to patchStrategy, it is impossible to know whether the user wants to merge according to some item's trait in the array, or append the item ​​into the array, or even replace all the elements of the array.

@anurnomeru
Copy link

@lucadirocco How did you solve the problem I mentioned above? thx

@lucadirocco
Copy link
Contributor Author

lucadirocco commented Feb 16, 2022

@anurnomeru
I didn't implement any patch strategy option.
At the end, after considering several options and also introducing new operators (e.g. =* for merge, =+ to add, =- to remove), I decided to make the --set-json logic consistent with the logic used for the the other --set commands, which means values are always overridden.
This allows to introduce in a quite straightforward way the basic feature of setting json values from command line.
And this is what I was mainly needing for the scenario where helm charts are managed through a resource orchestrator, who decides the actual configuration to be applied/modified.

Later, we can see if it make sense to introduce more advanced mechanism to update the values through json on command line.

lucadirocco added a commit to lucadirocco/helm that referenced this issue Feb 21, 2022
When used with helm install, helm template, helm upgrade, it enables
to set json values (scalars/objects/arrays) from the command line.

Closess helm#10428
lucadirocco added a commit to lucadirocco/helm that referenced this issue Feb 21, 2022
When used with helm install, helm template, helm upgrade, it enables
to set json values (scalars/objects/arrays) from the command line.

Closes helm#10428
lucadirocco added a commit to lucadirocco/helm that referenced this issue Feb 21, 2022
When used with helm install, helm template, helm upgrade, it enables
to set json values (scalars/objects/arrays) from the command line.

Closes helm#10428

Signed-off-by: Luca Di Rocco <lucadirocco@gmail.com>
lucadirocco added a commit to lucadirocco/helm that referenced this issue Feb 22, 2022
When used with helm install, helm template, helm upgrade, it enables
to set json values (scalars/objects/arrays) from the command line.

Closes helm#10428

Signed-off-by: Luca Di Rocco <lucadirocco@gmail.com>
lucadirocco added a commit to lucadirocco/helm that referenced this issue Mar 7, 2022
When used with helm install, helm template, helm upgrade, it enables
to set json values (scalars/objects/arrays) from the command line.

Closes helm#10428

Signed-off-by: Luca Di Rocco <lucadirocco@gmail.com>
@lucadirocco
Copy link
Contributor Author

Hello @mattfarina, I have issued a pull request #10693 for this issue, which has also got a review (LGTM) by joejulian, who however said he is not an approver.
Am I required to do anything for the pull request to get approved/merged?

@hickeyma hickeyma added this to the 3.9.0 milestone Mar 29, 2022
@mattfarina mattfarina modified the milestones: 3.9.0, 3.10.0 May 18, 2022
@romanogit
Copy link

romanogit commented Jul 14, 2022

Would it also be nice if --set-json could receive a file, like --set-json config=@/tmp/file.json

@hari4147
Copy link

feat: add --set-json flag to set json values This feature available in which release?.

image

@hickeyma
Copy link
Contributor

@hari4147 #10693 is waiting on a another maintainer approval to merge.

zak905 pushed a commit to zak905/helm that referenced this issue Jan 19, 2023
When used with helm install, helm template, helm upgrade, it enables
to set json values (scalars/objects/arrays) from the command line.

Closes helm#10428

Signed-off-by: Luca Di Rocco <lucadirocco@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants