Is openssl_csr_pipe check mode working as intended?

[yann-soubeyrand]

Hi,

When referencing an openssl_csr_pipe in an openssl_tls_certificate (see below), the check mode fails because the csr_content is empty. Looking at the code, shouldn’t this check be removed in order to always generate the CSR (since it’s a pipe resource)?

- name: "Create TLS certificate signing request"
  community.crypto.openssl_csr_pipe:
    common_name: "{{ ansible_fqdn }}"
    privatekey_path: "{{ postgresql_tls_key.filename }}"
    use_common_name_for_san: true
    subject_alt_name_critical: true
    basic_constraints:
      - "CA:FALSE"
    basic_constraints_critical: true
    key_usage:
      - "digitalSignature"
      - "keyEncipherment"
    key_usage_critical: true
  register: "postgresql_tls_csr"
  changed_when: "postgresql_tls_key is changed"

- name: "Create TLS certificate"
  community.crypto.x509_certificate:
    path: "{{ postgresql_tls_cert_path }}"
    state: "present"
    provider: "selfsigned"
    csr_content: "{{ postgresql_tls_csr.csr }}"
    privatekey_path: "{{ postgresql_tls_key.filename }}"
    owner: "root"
    group: "root"
    mode: "u=rw,g=r,o=r"
    return_content: true
  register: "postgresql_tls_cert"
  notify: "postgresql_service"

[yann-soubeyrand]

I confirm that removing this check makes check mode work.

[felixfontein]

Hmm, this is a really good question. I'm currently tending to agree with you, that the module should simply ignore check mode. (As a workaround, you can add check_mode: false to the task BTW, then check mode is always disabled for that task.) I'm trying to come up with scenarios where not ignoring check mode is a bad idea, but so far I found none. The module is similar to a Jinja2 filter, which also doesn't care about check mode, since it doesn't modify state actively - the user has to do something with the return value, and only that can actually modify something.

(Sorry for the late reply, I wanted to comment earlier but then I got side-tracked, and then I suddenly saw it's four days later and I still haven't replied :) )

[felixfontein]

I've been thinking about this some more. I think we have three possibilities:

1. Keep the module as-is.
2. Make the behavior configurable (in another way than specifying check_mode: false for the task), and eventually change the default.
3. Change the behavior.

Basically both changing the default in 2, and doing 3, is a breaking change, since the current behavior has been there for a long time. This could potentially break users. (Which obviously is true for any change.) So IMO we should not do that without a new major release. We currently don't have concrete plans for a 3.0.0 release (#559), but I wouldn't mind doing that not too far in the future.

Not doing the change now is fine I think, since it is easy to work around this by adding check_mode: false to the task. So we shouldn't rush anything.

I'm currently trying to figure out whether making the behavior configurable is worth the trouble. I guess the main question is whether the old behavior is something that folks need. Or whether they need something else than the current and the potentially new behavior - like generate the object in question in case it doesn't exist, but don't re-generate it if it should be re-generated, but only in check mode. I don't think anything more complex than "current behavior or check_mode: false behavior" is really necessary.

@MarkusTeufelberger @Ajpantuso what do you think?

[Ajpantuso]

@felixfontein I also don't see real benefit from making this configurable. I think option 3 in the 3.0.0 release sounds right given there is a workaround and changing the behavior will result in the "least surprise".

[felixfontein]

@Ajpantuso :+1: I think it makes sense to add a change now that emits a deprecation warning in cases where the behavior will change, so that users know that something will happen.

[felixfontein]

I created a PR for the deprecation: #714.

[yann-soubeyrand]

Thanks @felixfontein!

[felixfontein]

I'm going to merge #714 this weekend (and do a new release) if nobody objects.