Rework api feature - #465
Conversation
dgoetz
left a comment
dgoetz
left a comment
There was a problem hiding this comment.
Not tested yet, just commented what I did see while carefully reading it.
| You can use the `netways.icinga.icinga2_ticket` filter to create a valid ticket if you know the secret `TicketSalt`. | ||
| Example: `ticket: "{{ <common_name> | netways.icinga.icinga2_ticket(ticketsalt='<secret_ticket_salt>') }}"` | ||
|
|
||
| **Auto-signing without ticket:** |
There was a problem hiding this comment.
Not sure if the headline is misleading as a ticket is still required here, you just don't provide it explicitly. Would "Auto-signing without providing a ticket" be a better headline? If changed, would the headline before also need to be changed to "Providing a ticket for auto-signing"?
| **Auto-signing with reverse connection:** | ||
| Used in environments where the agent cannot connect to its parent but the parent can connect to the agent. | ||
| Here delegation to `parent_host` (or `icinga2_delegate_host`) is used to retrieve the CA certificate and generate a ticket. | ||
| This is used if `delegate_pki: true`. |
There was a problem hiding this comment.
I would change the wording and order here to more explicitly to tell the user it is required to set delegate_pki to true which changes now what parent_host does.
|
|
||
| By default the FQDN is used as certificate common name, to put a name | ||
| yourself: | ||
| By default the FQDN is used as certificate common name, to put a name yourself: |
There was a problem hiding this comment.
Should we add some disclaimer here that this is typically not best practice and can require additional work in further steps like the director setup not being able to utilize the agent setting or at least give the one good example for this feature which would be lower-casing the hostname?
| ``` | ||
|
|
||
| To increase your security set `ca_fingerprint` to validate the certificate of the `ca_host`: | ||
| To increase your security set `ca_fingerprint` to validate the CA certificate: |
There was a problem hiding this comment.
Should we add how to get the fingerprint for using it here?
| * `ca_host: string` | ||
| * Use to decide where to gather the certificates. When set to **None**, Ansible will create a local Certificate Authority on the Host. Use **hostname** or **ipaddress** as value. | ||
| * `parent_host: string` | ||
| * Use to decide where to gather the certificates. When set to **none**, Ansible will create a local Certificate Authority on the Host. Use **hostname** or **ipaddress** as value. |
There was a problem hiding this comment.
Is hostname or ipaddress here really a good choice as the certificate will contain an FQDN?
| # Potential answers | ||
| # information/cli: Writing CA certificate to file '/var/lib/icinga2/certs/ca.crt'. | ||
| # information/cli: Writing signed certificate to file '/var/lib/icinga2/certs/<node_name>.crt'. | ||
| # → RC X |
| # → First connection after certificate is signed | ||
| # | ||
| # Could not fetch valid response. Please check the master log. | ||
| # → RC X |
| ) | ||
| ret['changed'] = True | ||
|
|
||
| # WIP: Trusted parent cert could be wrong at this point because parent might have forcefully created a new cert |
There was a problem hiding this comment.
A WIP that perhaps should be addressed
| if not cn: | ||
| cn = socket.getfqdn() | ||
|
|
||
| if mode == 'agent': |
There was a problem hiding this comment.
Should be the same for satellite or did I just miss where satellite is changed to agent?
| mode_ret = dict() | ||
| config_ret = dict() | ||
|
|
||
| if mode == 'agent': |
There was a problem hiding this comment.
Should be the same for satellite or did I just miss where satellite is changed to agent?
|
Also tested by/with @Saeid-Abadi. We should add a |
Add module `netways.icinga.icinga2_api` to handle setting up the node as a master / agent. Rework api feature tasks to make use of the new module instead of having all logic in tasks.
|
Just as a note for myself for further reviews: Reviewed all the changes till now which address my comments, no additional comments, but not everything is solved yet. |
dgoetz
left a comment
dgoetz
left a comment
There was a problem hiding this comment.
Thanks for addressing all my nitpicking! ;-)
|
Thank you for nitpicking! :D |
2 participants
Add module
netways.icinga.icinga2_apito handle setting up the node as a master / agent.Rework api feature tasks to make use of the new module instead of having all logic in tasks.