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

[DRAFT] Rewrite Rotation Logic in time_rotating #180

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SBGoods
Copy link
Contributor

@SBGoods SBGoods commented Mar 13, 2023

The time_rotating resource currently triggers a "rotation" by comparing the current time to rotation_rfc3339 and if the current time is after rotation_rfc3339 it will remove the resource in the read method.

The rotation_rfc3339 is currently set in three different ways:

  1. Manually configured via the rotation_rfc3339 attribute
  2. Computed using the configured rfc3339 and at least one configured rotation_ attribute to be added to the base rfc3339
  3. Computed using at least one configured rotation_ attribute to be added to the rfc3339 which is computed to the current time

The current implementation has several issues:

  1. Removing the resource in the read method and creating it again in create is a different behavior than triggering a recreate by setting requiresReplace. Use of the depends_on meta argument on the time_rotating resource or it's attributes does not properly trigger a replacement of another resource or data source
  2. if the rfc3339 or rotation_rfc3339 is set in the configuration and the time has expired and the resource is removed in read and created again. Every subsequent plan will prompt a resource creation but the state does not change.
  3. if the rfc3339 or rotation_rfc3339 is set, then the "rotation" will only happen once. Once the time has expired for the first "rotation", the rotation_rfc3339 will not change unless there is a configuration change.

As a rough implementation to solve these issues, this draft PR:

  • Moved the time expiration logic outside of the read method and into the planModifier method.
  • Add logic to automatically recalculate the rotation values after the time has expired when rfc3339 is set in the configuration. The recalculated rotation value will be set to the current time plus any additional rotation_ values in the configuration.
  • Use the id to store any new rotation value and change the time expiration logic to compare against the id attribute instead of rotation_rfc3339, to prevent errors when recalculating the rotation since rotation_rfc3339 is a Computed + Optional attribute.

This is a very rough draft of an implementation as there are many edge case and further design decisions to be made, but this solution does solve the problems outlined above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant