[WIP] ipaserver: Add support for key_type_size

[rjeffman]

Since FreeIPA version 4.13.0, the CLI installation supports parameter --key-size-type which allows the selection of the key signing type and length. This will be used to support post-quantum cryptography, as can be seen in FreeIPA PR #8160 1.

This patch adds the argument 'ipaserver_key_type_size' to ipaserver deployment role, as is must be used on the deployment of the first server. The values for the parameter are defined as ':' and defaults to 'rsa:2048', as does FreeIPA in its current release.

After deployment, the change can be verified using the ipa command 'config-show', and looking at value for 'IPA Service key type:size'.

Summary by Sourcery

Add support for configuring the key type and size used by IPA server certificates during initial server deployment.

New Features:

• Introduce an ipaserver role parameter to specify key type and size for HTTP, LDAP, PKINIT and RA certificates, defaulting to rsa:2048.

Documentation:

• Document the new ipaserver_key_type_size variable and its default in the ipaserver role README.

[sourcery-ai]

Hey - I've found 5 issues, and left some high level feedback:

• There is an inconsistency between key_type_size and key_size_type (module docs use key_type_size, argument spec uses key_size_type); align the parameter name everywhere (docs, arg spec, options, and playbook variable) to avoid confusing users and breaking variable passing.
• Consider adding basic validation for the key_size_type format (e.g., <type>:<bitlength>) in the module to fail fast on invalid values rather than passing malformed strings through to the underlying FreeIPA CLI.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- There is an inconsistency between `key_type_size` and `key_size_type` (module docs use `key_type_size`, argument spec uses `key_size_type`); align the parameter name everywhere (docs, arg spec, options, and playbook variable) to avoid confusing users and breaking variable passing.
- Consider adding basic validation for the `key_size_type` format (e.g., `<type>:<bitlength>`) in the module to fail fast on invalid values rather than passing malformed strings through to the underlying FreeIPA CLI.

## Individual Comments

### Comment 1
<location> `roles/ipaserver/library/ipaserver_prepare.py:241-240` </location>
<code_context>
     type: bool
     default: no
     required: no
+  key_type_size:
+    description:
+        The key type and size for HTTP, LDAP, PKINIT and RA (if CA
+        configured) certificates (Requires IPA 4.13+)
+    type: str
+    default: "rsa:2048"
+    required: no
 author:
     - Thomas Woerner (@t-woerner)
</code_context>

<issue_to_address>
**issue (bug_risk):** Option name here (`key_type_size`) is inconsistent with the parameter name used later (`key_size_type`).

DOCUMENTATION defines this as `key_type_size`, but the argument spec uses `key_size_type`, so the user value will be ignored. Please choose one canonical name (ideally `key_type_size` to match `options.key_type_size`) and use it consistently in DOCUMENTATION, the argument spec, option assignment, and tasks.
</issue_to_address>

### Comment 2
<location> `roles/ipaserver/library/ipaserver_prepare.py:340` </location>
<code_context>
                                        default=False),
             _hostname_overridden=dict(required=False, type='bool',
                                       default=False),
+            key_size_type=dict(required=False, type="str", default="rsa:2048"),
         ),
         supports_check_mode=False,
</code_context>

<issue_to_address>
**issue (bug_risk):** Argument spec uses `key_size_type` which doesn’t match the documented option or the `options` attribute name.

This arg is declared as `key_size_type`, but the code reads `key_type_size` into `options.key_type_size`. Because of this mismatch, the value from Ansible is never used. Please rename the argument to `key_type_size` to match the DOCUMENTATION and the `options` attribute and prevent silent misconfiguration.
</issue_to_address>

### Comment 3
<location> `roles/ipaserver/library/ipaserver_prepare.py:423` </location>
<code_context>
         '_hostname_overridden')
     sid_generation_always = ansible_module.params.get('sid_generation_always')
     options.kasp_db_file = None
+    options.key_type_size = ansible_module.params.get('key_type_size')

     # init ##################################################################
</code_context>

<issue_to_address>
**issue (bug_risk):** `ansible_module.params.get('key_type_size')` won’t be populated due to the mismatched argument name.

Because the arg spec defines `key_size_type`, this `params.get('key_type_size')` call will always be `None`. After you pick a single canonical name, update this lookup to use it so the value actually reaches `options.key_type_size`.
</issue_to_address>

### Comment 4
<location> `roles/ipaserver/tasks/install.yml:226` </location>
<code_context>
       sid_generation_always: "{{ result_ipaserver_test.sid_generation_always }}"
       random_serial_numbers: "{{ result_ipaserver_test.random_serial_numbers }}"
       _hostname_overridden: "{{ result_ipaserver_test._hostname_overridden }}"
+      key_size_type: "{{ ipaserver_key_size_type | default(omit) }}"
     register: result_ipaserver_prepare

</code_context>

<issue_to_address>
**issue (bug_risk):** Task variable name `key_size_type` is inconsistent with the module option name.

This task passes `key_size_type` to the module, but the module’s option is named `key_type_size`. Once you choose the final option name in the module, update this task to use that exact key so the user’s value is passed through correctly.
</issue_to_address>

### Comment 5
<location> `roles/ipaserver/README.md:330` </location>
<code_context>
 `ipaserver_subject_base` | The certificate subject base (default O=<realm-name>). RDNs are in LDAP order (most specific RDN first). (string) | no
 `ipaserver_ca_subject` | The CA certificate subject DN (default CN=Certificate Authority,O=<realm-name>). RDNs are in LDAP order (most specific RDN first). (string) | no
 `ipaserver_ca_signing_algorithm` | Signing algorithm of the IPA CA certificate. (choice: SHA1withRSA, SHA256withRSA, SHA512withRSA) | no
+`ipaserver_key_size_type` | The key type and size for HTTP, LDAP, PKINIT and RA (if CA configured) certificates. Requires FreeIPA version 4.13 or later. (default:"rsa:2048") | no

 DNS Variables
</code_context>

<issue_to_address>
**suggestion (typo):** Consider fixing the grammar in the parenthetical phrase "if CA configured".

This reads as if a word is missing; consider rephrasing to "if the CA is configured" or at least "if CA is configured" for clearer grammar.

```suggestion
`ipaserver_key_size_type` | The key type and size for HTTP, LDAP, PKINIT and RA (if the CA is configured) certificates. Requires FreeIPA version 4.13 or later. (default:"rsa:2048") | no
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}