Factories fail when empty YaML files are passed

[juliocc]

Factories fail when empty YaML files are passed.
Some factories mitigate the problem by using try(yamldecode()), which however masks potential parse errors.

original message below:

getting the following error when applying 02-networking stage with empty YAML files for firewall rules
example files:
2-networking-b-vpn/data/hierarchical-ingress-rules.yaml
2-networking-b-vpn/data/firewall-rules/dev/rules.yaml

cc @aaarel

[sruffilli]

WIP tests log:

Works if empty
2-networking-b-vpn/data/hierarchical-ingress-rules.yaml

Fails if empty
2-networking-b-vpn/data/firewall-rules/dev/rules.yaml

[ludoo]

Yep, problem is yamldecode not liking empty/comment only YAML files. We had try around the yamldecode in some places and not others, then Terraform was supposedly fixed and we removed it, and then it was added back in some places.

Discussed it the other day with Julio, and as long as we're consistent and users know what to expect, either solution works each with its own drawbacks

• try makes comment-only files work but hides errors
• no try exposes errors but then it should be a surprise that a comment-only file breaks

[sruffilli]

Came here with the same comments.

I'd be tempted to remove try everywhere, at which point users have two options:

• Create a valid YaML empty map {} (which can be a comment documenting the behavior at the top of each sample file)
• Remove the factory configuration altogether.

Explicit is better than implicit

Wdyt?

[ludoo]

Agree, I would also drop a comment in our example files, stating that the file should either be valid or be removed altogether. So users know what to expect.

[juliocc]

After a couple of tests, a empty file can be "fixed" with ---

> yamldecode("")
╷
│ Error: Error in function call
│
│   on <console-input> line 1:
│   (source code not available)
│
│ Call to function "yamldecode" failed: on line 1, column 1: missing start of document.
╵


> yamldecode("#")
╷
│ Error: Error in function call
│
│   on <console-input> line 1:
│   (source code not available)
│
│ Call to function "yamldecode" failed: on line 1, column 1: missing start of document.
╵


> yamldecode("---")
null
> yamldecode("#\n---")
null

[ludoo]

That's genius. So we just append \n--- to any file before passing to yamldecode?

[ludoo]

nope, does not work

> yamldecode("foo: {}\n---")
╷
│ Error: Error in function call
│ 
│   on <console-input> line 1:
│   (source code not available)
│ 
│ Call to function "yamldecode" failed: on line 1, column 1: unexpected extra content after value.
╵


> yamldecode("foo: {}\n")
{
  "foo" = {}
}

[sruffilli]

you got it reversed - should be

> yamldecode("---\n{}")
{}

[ludoo]

> yamldecode("--\nfoo: {}\n")
╷
│ Error: Error in function call
│ 
│   on <console-input> line 1:
│   (source code not available)
│ 
│ Call to function "yamldecode" failed: on line 2, column 1: mapping values are not allowed in this context.

[juliocc]

That's genius. So we just append \n--- to any file before passing to yamldecode?

I was thinking we can add that to any empty file

# boilerplate
---

[ludoo]

I would go with removing try and adding comments to warn users. The --- trick seems fragile, and more magical/implicit than having a try IMHO.

[ludoo]

That's genius. So we just append \n--- to any file before passing to yamldecode?

I was thinking we can add that to any empty file

# boilerplate
---

harder to understand than an explicit comment saying something like

# Terraform will be unable to decode this file if it does not contain valid YAML (empty, or comments only)

[wiktorn]

> yamldecode("---\nfoo: {}")
{
  "foo" = {}
}

You missed on - @ludoo

+1 from me to:

I'd be tempted to remove try everywhere, at which point users have two options:

• Create a valid YaML empty map {} (which can be a comment documenting the behavior at the top of each sample file)
• Remove the factory configuration altogether.

Explicit is better than implicit

Wdyt?

[juliocc]

It's part of the yaml spec, so not really a trick.

But agreed, it's more obscure than just a comment

[ludoo]

It's part of the yaml spec, so not really a trick.

But agreed, it's more obscure than just a comment

Absolutely, the whole issue is yamldecode barfing, which was supposed to be fixed as you noted the other day.

[ludoo]

You missed on - @ludoo

🤦

[sruffilli]

That's genius. So we just append \n--- to any file before passing to yamldecode?

I was thinking we can add that to any empty file

# boilerplate
---

harder to understand than an explicit comment saying something like

# Terraform will be unable to decode this file if it does not contain valid YAML (empty, or comments only)

IMHO we should do both.
--- signals the beginning of a YaML document, so it's fair game to bake it into our files - and we could add the comment @ludoo wrote following/preceding it.

[ludoo]

+1 Simo

[ludoo]

And we do this for all modules exposing a factory, and add the comment to YAMLs in the doc examples too, right? Otherwise we'll have this same discussion all over again in a few months...

[sruffilli]

SG. Editing the original post to extend scope, and will TAL later today.

[sruffilli]

Fixed by #1812