support ${{KEY}} substitution syntax for non-string fields #11068
Conversation
bparees
force-pushed
the
int_parameter_support
branch
from
961491c to
2a62d7a
Compare
bparees
changed the title
|
@csrwng @smarterclayton ptal. i'm hoping you don't completely hate this json/regex approach. |
bparees
force-pushed
the
int_parameter_support
branch
from
2a62d7a to
0b7b013
Compare
bparees
changed the title
bparees
force-pushed
the
int_parameter_support
branch
2 times, most recently
from
139349f to
1493973
Compare
|
[test] |
|
[testextended][extended:core(images)] |
|
[testextended][extended:core(builds)] |
|
flake #11016 |
| return templateErrors, errors.New("Could not load json serializer") | ||
| } | ||
|
|
||
| templateBytes, err := runtime.Encode(serializer, template) |
There was a problem hiding this comment.
We can't use regex over json unfortunately, there are to many ways it can fail. Going to have to transform in place over the unstructured scheme.
There was a problem hiding this comment.
For instance you can't transform keys correctly (you need to error).
There was a problem hiding this comment.
what do you mean i can't transform keys correctly? my logic in this PR was "as a template author you now have the power to substitute anywhere in the json/yaml that you want(including keys), but with that power comes the responsibility that you ensure the result is sane" and if the result isn't sane, we'll get an error transforming it back and report that error to you so you can fix it.
| @@ -47,10 +47,15 @@ func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (runtime.Object, err | |||
| "expression": generator.NewExpressionValueGenerator(rand.New(rand.NewSource(time.Now().UnixNano()))), | |||
| } | |||
| processor := template.NewProcessor(generators) | |||
| if errs := processor.Process(tpl); len(errs) > 0 { | |||
| errs, err := processor.Process(tpl) | |||
There was a problem hiding this comment.
maybe use different var names to distinguish the types of error you're expecting returned? (templateErrors, err)
| } | ||
| templateString := string(templateBytes) | ||
|
|
||
| // consider we start with a field like "${PARAM1}${{PARAM2}" |
bparees
force-pushed
the
int_parameter_support
branch
from
1493973 to
d84fa21
Compare
|
Evaluated for origin test up to d84fa21 |
|
Evaluated for origin testextended up to d84fa21 |
|
@smarterclayton to do this with unstructured json object seems ugly at best. On deserialization we're going to end up with a collection of map[string]interface{} but in reality the type of every value is going to be a string. Then we do the substitution, but the value will still be a string and when it gets serialized back out, the quotes will be put back in place. The only way to successfully strip the quotes if is we replace the value object in the struct with a non-string object, but that requires us understanding what type the new value is after substitution, when it could be an int, a string, a float, an unsigned float/int, a boolean, etc. Furthermore i don't see the downside to supporting parameterization of keys. It's a generic solution for parameterizing template json/yaml. what is the downside other than the argument that it more closely approximates a generic templating language (except that it's still much less powerful)? |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/9791/) |
|
continuous-integration/openshift-jenkins/testextended FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/562/) (Extended Tests: core(builds), core(images)) |
fixes #3946