-
Notifications
You must be signed in to change notification settings - Fork 156
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
Issue #391: fix idempotency issues with ttls in approle_role #459
base: main
Are you sure you want to change the base?
Conversation
Take all TTL arguments to hashivault_approle_role in seconds as this is the unit that Vault/HVAC will return.
As discussed in #391 this change forces all TTLs to be specified in seconds as this is the value that the Vault API and HVAC return regardless of how the value was originally specified. Any playbooks using "friendly" values will need to be converted to seconds. |
token_max_ttl: | ||
description: | ||
- Duration after which the issued token can no longer be renewed. | ||
- Duration after which the issued token can no longer be renewed, in seconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was some code I thought to convert times to an idempotent value. I'm not sure if that would help here. This is a change to the API which would trigger a new major release since it is breaking.
I will take a look and see if I can find something.
…On Tue, Jan 23, 2024, 9:01 AM Terry Howe ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In ansible/modules/hashivault/hashivault_approle_role.py
<#459 (comment)>
:
> token_max_ttl:
description:
- - Duration after which the issued token can no longer be renewed.
+ - Duration after which the issued token can no longer be renewed, in seconds.
There was some code I thought to convert times to an idempotent value. I'm
not sure if that would help here. This is a change to the API which would
trigger a new major release since it is breaking.
—
Reply to this email directly, view it on GitHub
<#459 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AWETUDKWOKQVZ5CO7QXO4XTYP7GDJAVCNFSM6AAAAABCGO4FC6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMZZGAYDQNZQGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***
com>
|
This is a breaking change since current support allows different time formats. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pr https://github.com/TerryHowe/ansible-modules-hashivault/pull/484/files move the parse method to a common area which is better than changing the interface
Take all TTL arguments to hashivault_approle_role in seconds as this is the unit that Vault/HVAC will return.