Origin: Cannot specify integer/bool/base64 types in templates

[nstrug]

It is not currently possible to specify integer types in template parameters.

For example, we want to be able to specify a parameter:

- name: N_MIAMI
  description: Number of Miami pods
  value: 1

where N_MIAMI will actually be fed to a replication controller to specify the number of replicas.
As written, this fails during oc process with:
error: unable to load "agree21-template.yaml": json: cannot unmarshal number into Go value of type string

We can fix this with:

- name: N_MIAMI
  description: Number of Miami pods
  value: "1"

which now successfully processes. However, the output now contains:

"replicas": "1"
which fails on creation with:
json: cannot unmarshall string into Go value of type int

A possible fix would be to be able to specify the expected parameter type (string, int or float should cover it) in the parameter definition.

[liggitt]

Even if you find a way to make the parameter typed, doing the actual replacement is more complicated. The template with the replaceable token has to be valid JSON, so you couldn't do this:

      "spec": {
        "replicas": ${MY_REPLICA_COUNT},

We'd have to find a way to:

1. Make a template with a replacement token which was valid JSON (which implies a string)
2. Indicate when the token is replaced, it should replace as a number (e.g. remove the wrapping quotes).

I can't think of a clean way to do that off the top of my head

[bparees]

@liggitt and I discussed this and he had a proposal of a format like:

"replicas": ${MY_REPLICA_COUNT}{INTEGER}

(ok we didn't discus the exact formatting so he may disagree there).

then we'd do normal parameter generation but then cast/convert the generated/overridden value into the appropriate type and specify it in the json in the correct format (eg stripping quotes for non-stirngs).

this would also enable parameterizing secrets with a format like:

"mySecret": ${SOME_PARAM}{BASE64}

though it would still not make it possible to supply a binary value or disk file as a secret parameter since you still have to first provide it in the Parameter definition and that only takes strings right now.

[liggitt]

the whole syntax would have to be in a string to be valid JSON and parameterize the value. also, my preference would be to include it in the parameter syntax, e.g.

"replicas":"${MY_REPLICA_COUNT | int}"

the pipe syntax would only be allowed if the parameter was the entire value (you couldn't do "${FOO | int}${BAR | int}"

Possible transformations:

• int, for replicas, etc. converted using http://golang.org/pkg/strconv/#ParseInt (with base 10 and bitsize 53?)

"${FOO | int}" -> 1

• float, for fractional quota limits. converted using http://golang.org/pkg/strconv/#ParseFloat (with bitsize 64?)

"${FOO | float}" -> 1.5

• bool, useful for pod options like hostNetwork, etc. converted using http://golang.org/pkg/strconv/#ParseBool

"${FOO | bool}" -> true

• base64, for any binary-encoded data, which is base64 encoded in the API:

"${FOO | base64}" -> "SGVsbG8gd29ybGQ="

For the conversion cases, we'd have to define what happens in error cases, either the template processing fails, or a default value (probably empty, so 0, 0.0, false, respectively) is used. I think my preference would be to fail processing in conversion error cases (with the possible exception of empty strings, which I would be ok converting to 0,0.0, and false)

[jorgemoralespou]

Hey @bparees @liggitt ,
What would be the problem extending the https://docs.openshift.org/latest/rest_api/openshift_v1.html#v1-parameter with a "type", like it has been done with the required?

Like it says on the 1.0.5 release notes:

Added required attribute to template parameters. Templates now cannot be instantiated without supplying a value for all required parameters.

[liggitt]

validating the input and describing the format of the output are different. What if you wanted to validate a numeric input (like "1"), but you wanted to set it in an envvar definition (which expects a string, so "1").

[jorgemoralespou]

And is there an issue to validate the input? This should be also needed, as this could allow to fail soon before doing the template processing.
We are seeing many templates process and resources created and then falling for to some invalid value.

[mfojtik]

@liggitt @bparees alternative proposal (just brainstorming)...
Let allow to specify the 'JSONPath' as part of the parameter definition:

    {
      "name": "INITIAL_REPLICAS_COUNT",
      "description": "The number of initial replicas",
      "path": ".deploymentConfig[name=foo]....",
      "value": 2,
    },

This will allow us to substitute basically any type (bool, array, etc..).

[bparees]

That feels harder to me as a user and more work to maintain if I rename
something or otherwise restructure my template. (and more error prone if I
mess up a path)

I take it your point is we could just do direct assignment then? "value"
would be an untyped interface?

Ben Parees | OpenShift
On Aug 28, 2015 04:52, "Michal Fojtik" [email protected] wrote:

@liggitt https://github.com/liggitt @bparees
https://github.com/bparees alternative proposal (just brainstorming)...
Let allow to specify the 'JSONPath' as part of the parameter definition:

{
  "name": "INITIAL_REPLICAS_COUNT",
  "description": "The number of initial replicas",
  "path": ".deploymentConfig[name=foo]....",
  "value": 2,
},

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/issues/3946#issuecomment-135688036.

[bparees]

Is failing at processing so bad? Processing is fast.

Whether we should do more validation as part of process is a valid
question/separate issue though.

Ben Parees | OpenShift
On Aug 28, 2015 02:51, "Jorge Morales Pou" [email protected] wrote:

And is there an issue to validate the input? This should be also needed,
as this could allow to fail soon before doing the template processing.
We are seeing many templates process and resources created and then
falling for to some invalid value.

—
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/issues/3946#issuecomment-135655996.

[mfojtik]

@bparees yes, the value is interface{}

Making the integer and boolean accepting string values with parameters seems to be much harder to implement (the JSON parser will have to be more relaxed on it).

[liggitt]

Validation is orthogonal. I might want to validate something as an integer, even if it is going to be plugged in as a string.

the JSON parser will have to be more relaxed on it

Which parser, parsing when we process the template?

[mfojtik]

@liggitt yes, like if you parse replicas: "${NUM | integer}". When reading the template, we will have to treat all fields as strings, do substitution and then change them to integers/bools?

[liggitt]

any field that contains a parameter substitution already has to be a string, in order to contain the param name syntax... I think we have to let that be transformed from a string to a non-string

[bparees]

It is a problem since the template today consists of (among other things) a list of API Structs, so the assumption is that the json in the template can be directly deserialized into an api object. (which is what we do, and then we iterate the objects/fields and do the substitution).

So @mfojtik makes a valid point that we can't just put String values into non-String fields in the template json, because it won't deserialize anymore (without some help)

[liggitt]

I thought this meant the objects could be arbitrary JSON:

    Objects []runtime.RawExtension `json:"objects" description:"list of objects to include in the template"`

I thought the template process visited all the structs using reflection, and could swap a string value out with an int value, then reserialize. I didn't think templates items got converted to actual API structs during processing.

[bparees]

could be. i'm guilty of assuming/speculating w/o checking the code.

[sosiouxme]

If we're adding types, how about a file type?

"${FOO | file}"
"${BAR | base64file}"
$ oc process -v FOO=./foo -v BAR=./bar 
  -> FOO replaced with contents of ./foo, BAR replaced with base64-encoded contents of ./bar

Ref. https://trello.com/c/70O1sCBv

[liggitt]

this is about how parameters are rendered into the template, not input... you can already do

oc process -v "FOO=`cat foo`"

[sosiouxme]

You can, at least under bash and if your file has no commas, but is that what we want to tell users? Anyway, point taken, this might be too much overloading of the "types" concept.

[donovanmuller]

Any update on this? I just gained a few new grey hairs because of this 😀

As a temporary hack I'm doing the following:
Given that the containerPort/port is parameterised with ${SERVER_PORT} and that the value is say 8080:

oc process a-template \
  -v SERVER_PORT="8080" | sed 's/"8080"/8080/g' \
  | oc create -f -

Nasty but works

[bparees]

At this point we're looking to address this as part of moving templates upstream to kubernetes, you can see the proposal here: https://github.com/kubernetes/kubernetes/blob/master/docs/proposals/templates.md

it includes support for handling this sort of scenario.

[alwinmark]

As referenced here: https://github.com/openshift/origin/pull/11068

There is a workaround using ${{ VARIABLE }} instead of ${ VARIABLE}, but I missed that. Hopefully this comment helps somebody in the same situation

[030]

In my case I had to remove the spaces as well: ${{VARIABLE}}. @alwinmark thanks for pointing me in the right direction.