DuckCorp » DuckCorp Infrastructure

Marc Dequènes wrote:

[...]
Use YAML structures please, my eyes are bleeding.

Done (branch updated)

Historically I setup ca-certificates/enable_crts to an empty list, but several services communicate with the outside and I had to revert at least on some hosts.
So I wonder about trust_new_crts because this would mean we trust the list at the time of first installation and not the Debian updates. So to me either we trust all or nothing. Could you explain your rationale?

Updated: I added a trust_distribution_crts: when trust_distribution_crts is set to no, enable_crts is set to an empty string (like new_crts).

Pierre-Louis Bonicoli wrote:

Use YAML structures please, my eyes are bleeding.

Done (branch updated)

Sorry, I can't see any difference. Now there are two debconf actions with a string.

Updated: I added a trust_distribution_crts: when trust_distribution_crts is set to no, enable_crts is set to an empty string (like new_crts).

This is a very good idea.
Additionally I would prefer the variable to be a real boolean and to convert it to the format the underlying software expect. This would avoid having "true", "yes", True, 1, "1"… and provide a coherent API across all roles.

Marc Dequènes wrote:

Pierre-Louis Bonicoli wrote:

Use YAML structures please, my eyes are bleeding.

Done (branch updated)

Sorry, I can't see any difference. Now there are two debconf actions with a string.

Oups, updated again.

Updated: I added a trust_distribution_crts: when trust_distribution_crts is set to no, enable_crts is set to an empty string (like new_crts).

This is a very good idea.
Additionally I would prefer the variable to be a real boolean and to convert it to the format the underlying software expect. This would avoid having "true", "yes", True, 1, "1"… and provide a coherent API across all roles.

What is a true boolean ? Is a YAML boolean a real one ?

Pierre-Louis Bonicoli wrote:

Oups, updated again.

Thanks.

What is a true boolean ? Is a YAML boolean a real one ?

YAML is typed so there are booleans, not just empty-for-false or interpreted strings. So currently you are using yes but it is quoted, so it is a string. Unquote it and it becomes a boolean value. Please see http://yaml.org/refcard.html

So, you can then use when: trust_distribution_crts, which is simpler, does not need any filter (you already used defaults/main.yml and it is a boolean already). If the user provides crap, I think it is a waste of energy to guess or check all possible mistakes. The documentation should say it is a boolean and the user MUST provide a YAML boolean or face an unwanted behavior.

Another comment: I think Update ca-certificates configuration should be a handler; this way we can reuse it if needed.

The variable could be defined in a INI-like inventory (or on the command line using -e).
Upstream strongly recommends to use bool filter.

I have updated the code (e31ccf820ec759847b58b3a21e648a6f707d301c):

• YAML boolean are used but I have kept the bool filter
• Update ca-certificates configuration is now an handler

Once reviewed and before merging I will squash this commit.

Ok, I understand the logic. Nevertheless I think inside the role we should take advantage of types. Which means we should use an intermediate variable to avoid having type-related filters all over the place (at least when the same variable is used multiple times). This way it is prepared safely and the rest of the code stays readable.

Also you're right we should fortify our role APIs.

The changes are fine and you can merge.