Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Would it make sense to document the precise format of not_valid_after in hcloud_certificate_info? #91

Closed
te-online opened this issue Jun 15, 2021 · 3 comments · Fixed by #453
Labels
bug Something isn't working pinned
Milestone

Comments

@te-online
Copy link

SUMMARY

The docs state that not_valid_after is ISO-8601 formatted, which I think is true.

Point in time when the Certificate stops being valid (in ISO-8601 format)

It seems to be %Y-%m-%d %H:%M:%S%z (example: 2021-06-15 13:37:08+00:00)?

However, I ran into issues, because I was comparing to ansible_date_time.iso8601, which is something like %Y-%m-%dT%H:%M:%SZ (example: 2021-06-15T13:37:08Z).

Note the T is a space in Hetzner Cloud's API response and the timezone notation is different.

Would it make sense to document an example of the precise date format? Or is that just common knowledge that ISO-8601 can have slightly different representations?

ISSUE TYPE
  • Documentation Report
COMPONENT NAME

hcloud_certificate_info

ANSIBLE VERSION
ansible 2.10.9
  config file = None
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.10 (default, May  4 2021, 18:59:47) [GCC 8.3.0]
@github-actions
Copy link

github-actions bot commented Oct 9, 2023

This issue has been marked as stale because it has not had recent activity. The bot will close the issue if no further action occurs.

@github-actions github-actions bot added the stale label Oct 9, 2023
@jooola jooola added pinned and removed stale labels Oct 9, 2023
@jooola jooola added this to the v3.0.0 milestone Nov 24, 2023
@jooola
Copy link
Collaborator

jooola commented Dec 21, 2023

@te-online Sorry for not getting back to you earlier.

We use the format defined in RFC 3339, Section 5.6 ('Internet Date/Time Format', date-time in the ABNF) for producing timestamps returned by the public API with the following restrictions:

  • We use Z as time offset (time-offset in the ABNF)
  • The T and Z characters are always uppercase.
  • We don't return fractional seconds (time-secfrac in the ABNF is always empty).

That said, there might be disparities in the API which should be considered as a bug.

On the ansible collection side, I believe that we also have some inconsistency: we use either the string representation of the datetime.datetime.__str__ or the output of datetime.datetime.isoformat function.

Python 3.11.6 (main, Oct  5 2023, 17:25:11) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from datetime import datetime
>>> d = datetime.now()
>>> str(d)
'2023-12-21 18:54:18.762601'
>>> str(d.isoformat())
'2023-12-21T18:54:18.762601'

I added this ticket to the v3 milestone, because I am not sure if I want to risk breaking user code by changing all the format to isoformat. I might reconsider this in the future.

@jooola jooola added the bug Something isn't working label Dec 21, 2023
@te-online
Copy link
Author

te-online commented Dec 21, 2023

No worries 😊 If you've used ansible's str() output format, you're at least following some standard.

My whole point was to have it documented, because it's a bit annoying I had to debug log the value to find what the issue with comparing the dates was. I personally think, hinting at the default format of the date in the docs would go a long way 🤓

I solved it like this, which is acceptable in my opinion (and can probably be improved anyways):

- name: Try to get current certificate expiry date
  ansible.builtin.set_fact:
    certificate_expiry_date: "{{ certinfo.hcloud_certificate_info[0].not_valid_after }}"
  when: existing_loadbalancer and (certinfo is defined)
  ignore_errors: yes

- name: Calculate certificate expiry days
  ansible.builtin.set_fact:
    certificate_expiry_days: "{{ ((certificate_expiry_date | to_datetime('%Y-%m-%d %H:%M:%S%z')) - (ansible_date_time.iso8601 | to_datetime('%Y-%m-%dT%H:%M:%S%z'))).days }}"
  when: certificate_expiry_date is defined

jooola added a commit that referenced this issue Feb 5, 2024
##### SUMMARY

Fixes #91

Always return datetime as iso-8601 formatted strings.


##### ISSUE TYPE

- Bugfix Pull Request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pinned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants