Skip to content

feat(aws): parameterize time_sleep duration with input wait_time#62

Merged
afiune merged 3 commits intolacework:masterfrom
dh11640:parameterize-time-sleep
Aug 25, 2020
Merged

feat(aws): parameterize time_sleep duration with input wait_time#62
afiune merged 3 commits intolacework:masterfrom
dh11640:parameterize-time-sleep

Conversation

@dh11640
Copy link
Contributor

@dh11640 dh11640 commented Aug 25, 2020

Parameterize the create_duration parameter for time_sleep resources.

Closes #61

@afiune afiune self-requested a review August 25, 2020 21:36
Copy link
Contributor

@afiune afiune left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks great, I have just two quick comments:

  1. Set the default time to 10s
  2. Add a quick documentation about this new input in the Inputs sectio of the README.md


variable "wait_time" {
type = string
default = "5s"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change the default to be 10s? I think we want to make increase the default so that others won't have to customize it that often.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, missed the readme bit. Just pushed that as well

@afiune afiune self-assigned this Aug 25, 2020
@afiune afiune added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 25, 2020
@afiune afiune changed the title Parameterize time_sleep duration feat(aws): parameterize time_sleep duration with input wait_time Aug 25, 2020
Copy link
Contributor

@afiune afiune left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YESS!! Thank You!

tenor-150496881

@afiune afiune merged commit e7892c0 into lacework:master Aug 25, 2020
@afiune
Copy link
Contributor

afiune commented Aug 25, 2020

Just upgraded our demo environment and ran a terraform apply and we see a few resources that will be destroyed since this change is replacing the time_sleep resources, users will see plan output similar to:

  # module.tech_ally_aws_config.module.lacework_cfg_iam_role.time_sleep.wait_5_seconds will be destroyed
  - resource "time_sleep" "wait_5_seconds" {
      - create_duration = "5s" -> null
      - id              = "2020-07-01T19:54:19Z" -> null
    }
  # module.tech_ally_aws_config.module.lacework_cfg_iam_role.time_sleep.wait_time will be created
  + resource "time_sleep" "wait_time" {
      + create_duration = "10s"
      + id              = (known after apply)
    }

Plan: 4 to add, 0 to change, 4 to destroy.

After applied the time resources were destroyed and the new ones took their place:

Apply complete! Resources: 4 added, 0 changed, 4 destroyed.

@afiune afiune mentioned this pull request Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make the delay time in the iam_role module tunable

2 participants