https://projects.duckcorp.org/https://projects.duckcorp.org/favicon.ico?16699090422017-05-16T14:31:39ZDuckCorp ProjectsDuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=11582017-05-16T14:31:39ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul><li><strong>Copied from</strong> <i><a class="issue tracker-8 status-3 priority-4 priority-default closed" href="/issues/543">Review #543</a>: Install apt-transport-https before updating cache</i> added</li></ul> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=11592017-05-16T14:31:58ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul><li><strong>Branch</strong> set to <i>ca-certificates</i></li></ul> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=11602017-05-16T14:49:11ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul><li><strong>Copied to</strong> <i><a class="issue tracker-8 status-3 priority-4 priority-default closed" href="/issues/545">Review #545</a>: s/ansible_hostname/inventory_hostname/</i> added</li></ul> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=11662017-05-18T17:27:57ZMarc Dequènesduck@duckcorp.org
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul><pre>
+ debconf: "name=ca-certificates setting={{ 'ca-certificates/' + item.0 }} vtype={{ item.1 }} value='{{ item.2 }}'"
</pre><br />Use YAML structures please, my eyes are bleeding.
<p>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.<br />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?</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=11712017-05-19T15:50:54ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul></ul><p>Marc Dequènes wrote:</p>
<blockquote>
<p>[...]<br />Use YAML structures please, my eyes are bleeding.</p>
</blockquote>
<p>Done (branch updated)</p>
<blockquote>
<p>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.<br />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?</p>
</blockquote>
<p>Updated: I added a <code>trust_distribution_crts</code>: when <code>trust_distribution_crts</code> is set to <code>no</code>, <code>enable_crts</code> is set to an empty string (like <code>new_crts</code>).</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=11722017-05-19T16:10:38ZMarc Dequènesduck@duckcorp.org
<ul></ul><p>Pierre-Louis Bonicoli wrote:</p>
<blockquote><blockquote>
<p>Use YAML structures please, my eyes are bleeding.</p>
</blockquote>
<p>Done (branch updated)</p>
</blockquote>
<p>Sorry, I can't see any difference. Now there are two <em>debconf</em> actions with a string.</p>
<blockquote>
<p>Updated: I added a <code>trust_distribution_crts</code>: when <code>trust_distribution_crts</code> is set to <code>no</code>, <code>enable_crts</code> is set to an empty string (like <code>new_crts</code>).</p>
</blockquote>
<p>This is a very good idea.<br />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 <em>"true"</em>, <em>"yes"</em>, <em>True</em>, <em>1</em>, <em>"1"</em>… and provide a coherent API across all roles.</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=12112017-06-06T16:30:19ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul></ul><p>Marc Dequènes wrote:</p>
<blockquote>
<p>Pierre-Louis Bonicoli wrote:</p>
<blockquote><blockquote>
<p>Use YAML structures please, my eyes are bleeding.</p>
</blockquote>
<p>Done (branch updated)</p>
</blockquote>
<p>Sorry, I can't see any difference. Now there are two <em>debconf</em> actions with a string.</p>
</blockquote>
<p>Oups, updated again.</p>
<blockquote><blockquote>
<p>Updated: I added a <code>trust_distribution_crts</code>: when <code>trust_distribution_crts</code> is set to <code>no</code>, <code>enable_crts</code> is set to an empty string (like <code>new_crts</code>).</p>
</blockquote>
<p>This is a very good idea.<br />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 <em>"true"</em>, <em>"yes"</em>, <em>True</em>, <em>1</em>, <em>"1"</em>… and provide a coherent API across all roles.</p>
</blockquote>
<p>What is a <code>true boolean</code> ? Is a YAML boolean a real one ?</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=12152017-06-10T06:06:29ZMarc Dequènesduck@duckcorp.org
<ul></ul><p>Pierre-Louis Bonicoli wrote:</p>
<blockquote>
<p>Oups, updated again.</p>
</blockquote>
<p>Thanks.</p>
<blockquote>
<p>What is a <code>true boolean</code> ? Is a YAML boolean a real one ?</p>
</blockquote>
<p>YAML is typed so there are booleans, not just empty-for-false or interpreted strings. So currently you are using <em>yes</em> but it is quoted, so it is a string. Unquote it and it becomes a boolean value. Please see <a class="external" href="http://yaml.org/refcard.html">http://yaml.org/refcard.html</a></p>
<p>So, you can then use <em>when: trust_distribution_crts</em>, which is simpler, does not need any filter (you already used <em>defaults/main.yml</em> 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 <b>MUST</b> provide a YAML boolean or face an unwanted behavior.</p>
<p>Another comment: I think <em>Update ca-certificates configuration</em> should be a handler; this way we can reuse it if needed.</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=12192017-06-16T21:29:27ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul></ul><p>The variable could be defined in a <em>INI-like</em> inventory (or on the command line using <code>-e</code>).<br />Upstream <a href="https://github.com/ansible/ansible/pull/25798" class="external">strongly recommends</a> to use <code>bool</code> filter.</p>
I have updated the code (e31ccf820ec759847b58b3a21e648a6f707d301c):
<ul>
<li>YAML boolean are used but I have kept the <code>bool</code> filter</li>
<li><code>Update ca-certificates configuration</code> is now an handler</li>
</ul>
<p>Once reviewed and before merging I will squash this commit.</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=12232017-06-17T03:39:12ZMarc Dequènesduck@duckcorp.org
<ul></ul><p>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.</p>
<p>Also you're right we should fortify our role APIs.</p>
<p>The changes are fine and you can merge.</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=12242017-06-18T21:20:34ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul><p>Merged, thanks for the reviews.</p> DuckCorp Infrastructure - Review #544: Install and configure ca-certificateshttps://projects.duckcorp.org/issues/544?journal_id=12442017-06-19T14:19:31ZPierre-Louis Bonicolipierre-louis.bonicoli@ir5.eu
<ul><li><strong>Related to</strong> <i><a class="issue tracker-2 status-3 priority-4 priority-default closed child" href="/issues/533">Enhancement #533</a>: Install a Jessie LXC container with systemd enabled in order to test/validate Burp setup</i> added</li></ul>