Confusing 'conditional expression without else clause' error with a yaml error before the if statement

[irbekrm]

What steps did you take:

I attempted to template a yaml file with some invalid yttt syntax:

...
  - #@ "--webhook-port={}".format(webhook_port <-  MISSING PARENTHESIS
#@ if image_pull_secrets_enabled:
imagePullSecrets:
- name: #@ image_pull_secrets_name
#@ end

What happened:
An error that suggested that my if block was missing an else clause was thrown:

ytt: Error: 
- conditional expression without else clause
    deployment.yaml:48 |       #@ if image_pull_secrets_enabled:

What did you expect:
Ideally, an error that would point out the line that has the error.

Failing that, an error that would suggest that the issue might be before the if block as the current error seems to strongly suggest that the issue is with the if block.

Environment:

• ytt version: v0.36.0

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[pivotaljohn]

Thanks for the simple example (reproduced in Playground).

Wow, this is an unfortunate error message.

TL;DR — at this point changing this behavior may be very hard.

So, under the covers, ytt uses a slightly modified Starlark. That runtime is what's compiling and executing this code.
One of the modifications is the inclusion of the end keyword and therefore whitespace/indent-insensitive parsing.

In Starlark, the if keyword can either be a statement or a conditional expression.

Some inside baseball: ytt compiles all YAML templates into Starlark programs that describe how to build the resulting YAML.

For example, this template gets compiled to:

...
__ytt_tpl2_start_node(6)
__ytt_tpl2_set_node(6, ("--webhook-port={}".format(webhook_port))
if image_pull_secrets_enabled:
__ytt_tpl2_start_node(7)
__ytt_tpl2_start_node(8)
__ytt_tpl2_start_node(9)
__ytt_tpl2_start_node(10)
__ytt_tpl2_start_node(11)
__ytt_tpl2_set_node(11, (image_pull_secrets_name))
end
__ytt_tpl2_start_node(12)
...

(compiled template listings are included in the --debug output)

It's subtle, but given that there's one fewer parens on that second line of the listing and that Starlark in ytt is whitespace/indentation-insensitive (meaning that the next line beginning with if ... appears to the parser to be a continuation of the previous line), that subsequent if keyword appears to be modifying the second parameter of the __ytt_tpl2_set_node() call...

i.e. the if is marking a conditional expression ... and thus "technically" the error message is correct.

I absolutely get that this is not how it reads to the eye within the template. So, I want to chew on this for a bit. As it stands, tho, I'm currently not bullish it's feasible to fix this error message. 😬

[irbekrm]

Thanks for the detailed reply 👍🏼

[cppforlife]

@pivotaljohn hmm, i wonder if we can run set node lines through its own ast parse... should that always succeed, i think so... in theory it should be self-contained (though symbols wont be resolved).

[pivotaljohn]

@pivotaljohn hmm, i wonder if we can run set node lines through its own ast parse... should that always succeed, i think so... in theory it should be self-contained (though symbols wont be resolved).

🤔 nice. Does this hold for when we're replacing too? I think so... hmmmmm.

[cppforlife]

no idea...

[pivotaljohn]

No question this would be great to be able to provide a better experience, here. Next step is to explore either the option @cppforlife noted, above, and/or other ways to catch this situation so we can then provide a better message to the user.

[pivotaljohn]

This is related to other less-than-great UX in templates:

Is it possible to pass the params of a function on several lines to improve the readability. Something like

 --- #@ generate_certificate(harbor_core_internal_tls_cert,
                             harbor_namespace,
                             harbor_cert_duration,
                             harbor_cert_renew_before,
                             harbor_core_internal_tls_secret,
                             [harbor_organization],
                             harbor_core_common_name,
                             generate_dns_names(harbor_core_common_name, harbor_namespace),
                             harbor_ca_issuer)

src: https://kubernetes.slack.com/archives/CH8KCCKA5/p1649264496571349