Adding --set-string flag to force string values. #3599

arturo-c · 2018-03-02T23:16:55Z

This addresses the problem of not being able to pass a long int value via the --set flag, and helm converting it to float64 scientific notation, one can then manipulate the string as needed inside the helm templates:

Example usage:
helm install --set long_string_value=1234567890 results in value once in helm template of: 1.23456789e+09

with new flag:
helm install --set-string long_string_value=1234567890 -> value = "1234567890"

Addresses these github issues:
#1707
#1694
#3155
#3001
#3246

This is my first dive at go, so please be gentle if I'm doing something completely wrong :)

k8s-ci-robot · 2018-03-02T23:16:57Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

thomastaylor312 · 2018-03-10T00:24:56Z

@arturo-c Thanks for the contribution! I will take a look at this on Monday. If for some reason I don't get around to it, feel free to ping me. Sorry for the delay on this

thomastaylor312

One change to make sure we don't break publicly exposed APIs, otherwise this looks pretty straightforward and you added unit tests 🎉

thomastaylor312 · 2018-03-13T03:14:57Z

pkg/strvals/parser.go

@@ -54,9 +54,9 @@ func Parse(s string) (map[string]interface{}, error) {
 //
 // If the strval string has a key that exists in dest, it overwrites the
 // dest version.
-func ParseInto(s string, dest map[string]interface{}) error {
+func ParseInto(s string, dest map[string]interface{}, stringBoolean bool) error {


I think it may be clearer and won't break the current API if you add a new method called ParseIntoString. The other private methods can still use a bool, but for someone consuming this package, they won't need to change how they are calling the methods. There might even be a better idea from one of the other maintainers as well. ParseIntoString seems pretty simple though

arturo-c · 2018-03-13T15:03:03Z

@thomastaylor312 thanks for the input, I had to add two public methods Parse and ParseIntoString, definitely will keep in mind not changing publicly exposed methods next time 👍

thomastaylor312

Sorry, one more thing I missed in initial review that I caught during some testing. After that, this should be good to go with one more review

thomastaylor312 · 2018-03-13T23:57:07Z

cmd/helm/template.go

@@ -96,6 +97,7 @@ func newTemplateCmd(out io.Writer) *cobra.Command {
 	f.VarP(&t.valueFiles, "values", "f", "specify values in a YAML file (can specify multiple)")
 	f.StringVar(&t.namespace, "namespace", "", "namespace to install the release into")
 	f.StringArrayVar(&t.values, "set", []string{}, "set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)")
+	f.StringArrayVar(&t.values, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)")


Looks like a copy/paste error. This should be t.stringValues.

Nice catch, updated 👍

thomastaylor312 · 2018-03-14T21:36:04Z

@arturo-c It looks like this should be rebased against master because it is overwriting changes on helm/master

arturo-c · 2018-03-14T22:33:40Z

@thomastaylor312 cool, not sure what happened there, but fixed the commit.

thomastaylor312 · 2018-03-15T00:27:17Z

@arturo-c So because this is labeled as a large, it will need 2 reviews before merge. We have the dev call tomorrow, so I will make sure someone reviews it. Great work on this!

bacongobbler

this looks like a good approach. Thanks!

babatundebusari · 2018-04-20T17:55:36Z

@arturo-c
here is what i have when i have --set-string for my helm install command

Error: unknown flag: --set-string

not yet released i guess?

bacongobbler · 2018-04-20T17:58:22Z

you can test it with either the canary releases or v2.9.0-rc4 :)

OndraZizka · 2018-05-21T15:07:16Z

While I consider this a workaround for the bug #1707 , it will be useful until the fix for it is released.

However. Will this be also applied to helm lint? See #4098

OndraZizka · 2018-05-21T15:08:50Z

Btw when is this supposed to be released? I checked with 2.9.1 and gett this:

+ helm install --replace --name h2dc6 --wait --timeout 600 ./... --set-string image.tag=2dc667a5
Error: unknown flag: --set-string

ashahba · 2018-05-24T20:47:40Z

@babatundebusari and @OndraZizka What is your: helm version output.
If you don't get:

$ helm version
Client: &version.Version{SemVer:"v2.9.0", GitCommit:"f6025bb9ee7daf9fee0026541c90a6f557a3e0bc", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.9.0", GitCommit:"f6025bb9ee7daf9fee0026541c90a6f557a3e0bc", GitTreeState:"clean"}

Then I recommend first updating your client version to v2.9.0 and then run helm init --upgrade so both client and server run matching versions.
Finally you can use --set-string option.

OndraZizka · 2018-08-14T00:30:41Z

I think both server and client were 2.9.1 in my case.

OndraZizka · 2018-08-14T00:32:31Z

Anyway - does the introduction of --set-string mean that --set sets other type? What type? Is it a generic number? Or can we expect more surprises, like, "516F" will be converted to Celsius? Wouldn't it be wiser to just have --set set a string?

This resolves an issue where numeric values intended to be used as strings could be interpreted as float64 values, causing varius things to fail in unexpected ways. This fix mirrors the changes in the following PR to Helm itself: helm/helm#3599 Closes databus23#83

… string. See: helm/helm#1707 helm/helm#3599

Adding --set-string flag to force string values.

arturo-c mentioned this pull request Mar 7, 2018

impossible to set string values on the command line that look like floats #1558

Closed

arturo-c force-pushed the master branch 2 times, most recently from d6165af to 6bd587c Compare March 9, 2018 15:04

thomastaylor312 added the feature label Mar 10, 2018

thomastaylor312 self-requested a review March 10, 2018 00:24

thomastaylor312 added the awaiting review label Mar 10, 2018

thomastaylor312 suggested changes Mar 13, 2018

View reviewed changes

thomastaylor312 added in progress and removed awaiting review labels Mar 13, 2018

arturo-c force-pushed the master branch 2 times, most recently from cbaad12 to 5300a29 Compare March 13, 2018 15:00

thomastaylor312 suggested changes Mar 13, 2018

View reviewed changes

arturo-c force-pushed the master branch from 5300a29 to 6e89f27 Compare March 14, 2018 12:38

thomastaylor312 approved these changes Mar 14, 2018

View reviewed changes

thomastaylor312 added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2018

arturo-c force-pushed the master branch 2 times, most recently from 769d05e to 18e27d2 Compare March 14, 2018 22:33

thomastaylor312 removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2018

gianrubio mentioned this pull request Mar 20, 2018

grafana extravars not quoting strings that needs to be quoted. prometheus-operator/prometheus-operator#1106

Closed

Adding --set-string flag to force string values.

a615f80

arturo-c force-pushed the master branch from 18e27d2 to a615f80 Compare March 20, 2018 19:36

arturo-c mentioned this pull request Mar 20, 2018

--set parses number only values as float64 #1707

Closed

bacongobbler approved these changes Mar 20, 2018

View reviewed changes

bacongobbler merged commit 48e7039 into helm:master Mar 20, 2018

arturo-c mentioned this pull request Mar 21, 2018

Force string values with --set #3155

Closed

bacongobbler mentioned this pull request Mar 27, 2018

There's no way to set a value to literal string "true", "false" or number #2848

Closed

bacongobbler mentioned this pull request Apr 11, 2018

extend value range for versions #3776

Closed

osterman mentioned this pull request Apr 26, 2018

Feature Request: Add support for helm --set-string roboll/helmfile#119

Closed

bacongobbler mentioned this pull request May 3, 2018

--set-string doesn't seem to handle booleans properly #4006

Closed

ashahba mentioned this pull request May 18, 2018

Ensure yaml is handling large Integers comming from git commit IDs co… intel/vck#42

Closed

kobel169 mentioned this pull request Aug 6, 2018

Please add --set-string option to dapp kube deploy command for force string values in helm. werf/werf#843

Closed

kujenga mentioned this pull request Aug 21, 2018

--set flag parses numbers as float64 databus23/helm-diff#83

Closed

kujenga mentioned this pull request Aug 21, 2018

add --set-string flag to ensure proper parsing of strings databus23/helm-diff#84

Merged

mgar added a commit to mgar/drone-helm that referenced this pull request Sep 3, 2018

Add --set-string flag to pass string values introduced in helm/helm#3599

caedb41

rdgoite pushed a commit to HumanCellAtlas/ingest-kube-deployment that referenced this pull request Oct 17, 2018

Updated make deployment script to force namespace and image values to…

994f91a

… string. See: helm/helm#1707 helm/helm#3599

splisson pushed a commit to splisson/helm that referenced this pull request Dec 6, 2018

Merge pull request helm#3599 from arturo-c/master

4dc07a5

Adding --set-string flag to force string values.

kolesoffac mentioned this pull request Dec 12, 2018

[stable/prometheus-postgres-exporter] Wrong when set port helm/charts#9896

Closed

Evesy mentioned this pull request Mar 14, 2019

Integer environment variable gets converted to scientific notation even when quoted roboll/helmfile#476

Open

lukasmrtvy mentioned this pull request Jul 7, 2021

Bug: integer values are rendered as scientific notations jfrog/charts#1466

Closed

micgruber mentioned this pull request Feb 23, 2023

Upgrade deployment with reuse-values #11844

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding --set-string flag to force string values. #3599

Adding --set-string flag to force string values. #3599

arturo-c commented Mar 2, 2018 •

edited

k8s-ci-robot commented Mar 2, 2018

thomastaylor312 commented Mar 10, 2018

thomastaylor312 left a comment

thomastaylor312 Mar 13, 2018

arturo-c commented Mar 13, 2018

thomastaylor312 left a comment

thomastaylor312 Mar 13, 2018

arturo-c Mar 14, 2018

thomastaylor312 commented Mar 14, 2018

arturo-c commented Mar 14, 2018

thomastaylor312 commented Mar 15, 2018

bacongobbler left a comment

babatundebusari commented Apr 20, 2018 •

edited

bacongobbler commented Apr 20, 2018

OndraZizka commented May 21, 2018

OndraZizka commented May 21, 2018 •

edited

ashahba commented May 24, 2018

OndraZizka commented Aug 14, 2018

OndraZizka commented Aug 14, 2018 •

edited

Adding --set-string flag to force string values. #3599

Adding --set-string flag to force string values. #3599

Conversation

arturo-c commented Mar 2, 2018 • edited

k8s-ci-robot commented Mar 2, 2018

thomastaylor312 commented Mar 10, 2018

thomastaylor312 left a comment

Choose a reason for hiding this comment

thomastaylor312 Mar 13, 2018

Choose a reason for hiding this comment

arturo-c commented Mar 13, 2018

thomastaylor312 left a comment

Choose a reason for hiding this comment

thomastaylor312 Mar 13, 2018

Choose a reason for hiding this comment

arturo-c Mar 14, 2018

Choose a reason for hiding this comment

thomastaylor312 commented Mar 14, 2018

arturo-c commented Mar 14, 2018

thomastaylor312 commented Mar 15, 2018

bacongobbler left a comment

Choose a reason for hiding this comment

babatundebusari commented Apr 20, 2018 • edited

bacongobbler commented Apr 20, 2018

OndraZizka commented May 21, 2018

OndraZizka commented May 21, 2018 • edited

ashahba commented May 24, 2018

OndraZizka commented Aug 14, 2018

OndraZizka commented Aug 14, 2018 • edited

arturo-c commented Mar 2, 2018 •

edited

babatundebusari commented Apr 20, 2018 •

edited

OndraZizka commented May 21, 2018 •

edited

OndraZizka commented Aug 14, 2018 •

edited