Templates in new-app allow not "closed" parameters

[soltysh]

Currently our templates allow both types of parameters ${foo and ${foo}. As long as the latter is perfectly fine, the former is wrong and should error out

[bparees]

Is this different from the new-app issue you found?

[soltysh]

That's the one, I moved it here so it doesn't get forgotten.

[bparees]

ok so it's not a template issue. it's a new-app issue.

specifically IsParameterizableValue() in app.go isn't being as strict in its checking for "things that look like parameters" as it should be because it doesn't require a closing "}" in the value.

@smarterclayton never weighed back in as to whether that was intentional but i assume it was just a lazy check as originally written and we should be fine to add the stricter checking.

[smarterclayton]

It was intended to be lazy since a precise check would potentially be out of sync with parameter parsing on the master.

[soltysh]

Hmmm... can't we do strict checking on the master as well?

[bparees]

No. That would mean ${ was invalid as a raw value and it isn't. We don't know if "${foo" is a bad parameter reference, or a valid intended value for the field. Ben Parees | OpenShift On May 3, 2017 09:11, "Maciej Szulik" <[email protected]> wrote: Hmmm... can't we do strict checking on the master as well? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#13994 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEvl3hQcMFCJxIE_XJ848D2DqkXQkly9ks5r2H0OgaJpZM4NOH5B> .

[bparees]

It was intended to be lazy since a precise check would potentially be out of sync with parameter parsing on the master.

i don't see us ever not requiring a closing } when parsing parameters. What has to go in between the {}s might be more questionable, but at a minimum ${.*} seems like an acceptable regex to validate against. For bonus points, perhaps ${[^\s].*} (any non-whitespace character) would be ok.

[smarterclayton]

What about escaped segments? On May 3, 2017, at 2:20 PM, Ben Parees <[email protected]> wrote: It was intended to be lazy since a precise check would potentially be out of sync with parameter parsing on the master. i don't see us ever not requiring a closing } when parsing parameters. What has to go in between the {}s might be more questionable, but at a minimum ${.*} seems like an acceptable regex to validate against. For bonus points, perhaps ${[^\s].*} (any non-whitespace character) would be ok. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13994 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p1xoktnZAvSJsUpzmnu5Srll36Qyks5r2MV0gaJpZM4NOH5B> .