Added 'ignore_timestamps' parameter

[JochenKorge]

SUMMARY

added 'ignore_timestamps' parameter to openssh_cert so it can be used semi-idempotent with relative timestamps in valid_to/valid_from.

Defaults to false (to be consistent with older versions)

Fixes #379

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

openssh_cert

ADDITIONAL INFORMATION

It´s my (second try on the) first pull-request and I´m new to python, so please be gentle ;)

[Ajpantuso]

Thank you for your contribution! Please add a changelog fragment and review the initial comments. Also, I would suggest at least two test cases added here. The first will test that with ignore_timestamps: true, equivalent relative timestamps for valid_from/valid_to, and a valid_at value within an existing certificates valid time range a subsequent task does not generate a new certificate. The second test is the same, but with ignore_timestamps omitted which should generate a new certificate.

[JochenKorge]

Did the proposed changes, added third test for valid_at > valid_to (has to change the cert)

[JochenKorge]

Out of curiosity, is there a way to run those tests local? I feel bad for spamming your repo.

[felixfontein]

You can run them locally as follows: ansible-test sanity --docker -v to run the sanity tests, ansible-test integration --docker fedora35 -v openssh_cert for the integration tests. You need to have checked out the repo in the correct path structure, though.

[JochenKorge]

Is there any reason to separate valid_at and ignore_timestamps other than it changes behavior?

Even the Documentation states:

Check if the certificate is valid at a certain point in time. If it is not the certificate will be regenerated. Time will always be interpreted as UTC. Mainly to be used with relative timespec for valid_from and / or valid_to. Note that if using relative time this module is NOT idempotent.

So I propose to merge ignore_timestamps as a crutch until the next big release and then change default behavior to the less surprising "if valid_at is set, ignore relative time"

Please correct me if I miss some use case where you need valid_at and ignore_timestamp false

[felixfontein]

I don't see why this is a crutch. From my POV this is the correct change, and somehow coupling valid_at and ignore_timestamps would be simply wrong. There are use-cases where ignore_timestamp makes sense without (relative) valid_at. Also absolute timestamps do not mean that they are constants in the playbook, they could have been templated (the module doesn't know how they looked in the playbook).

[felixfontein]

Besides this, looks good!

[Ajpantuso]

LGTM, thanks for implementing this!

[felixfontein]

@JochenKorge thanks for implementing this!
@Ajpantuso thanks for reviewing!