Fix/17/lifecycle rules #19

kopachevsky · 2019-11-04T16:09:53Z

Added lifecycle_rules variable and corresponding dynamic block.

Fixes #17

examples/simple_example/variables.tf

main.tf

test/integration/simple_example/controls/gsutil.rb

morgante · 2019-11-20T10:24:36Z

README.md

@@ -1,4 +1,4 @@
-# Terraform Google Cloud Storage Module
+    # Terraform Google Cloud Storage Module


Looks like a misformatting? Shouldn't have a tab here.

morgante · 2019-11-20T10:25:18Z

examples/simple_example/main.tf

@@ -24,5 +24,6 @@ module "cloud_storage" {
  prefix             = var.prefix
  names              = var.names
  bucket_policy_only = var.bucket_policy_only
+  lifecycle_rules    = var.lifecycle_rules


Please hard-code an example directly instead of making this a variable.

morgante · 2019-11-20T10:31:18Z

variables.tf

+    condition = map(string)
+  }))
+  default     = []
+  description = "The bucket's Lifecycle Rules configuration. Multiple blocks of this type are permitted. Format is the same as described in provider documentation https://www.terraform.io/docs/providers/google/r/storage_bucket.html#lifecycle_rule except condition.matches_storage_class - it should be comma delimited string."


Referring to "blocks" is confusing in this context.

Instead we should say something like:

Suggested change

description = "The bucket's Lifecycle Rules configuration. Multiple blocks of this type are permitted. Format is the same as described in provider documentation https://www.terraform.io/docs/providers/google/r/storage_bucket.html#lifecycle_rule except condition.matches_storage_class - it should be comma delimited string."

description = "List of lifecycle rules to configure. Format is the same as described in provider documentation https://www.terraform.io/docs/providers/google/r/storage_bucket.html#lifecycle_rule except condition.matches_storage_class should be a comma delimited string."

kopachevsky · 2019-11-20T12:18:04Z

@morgante please re-review

morgante · 2019-11-20T12:25:32Z

examples/simple_example/variables.tf

@@ -34,3 +34,14 @@ variable "bucket_policy_only" {
  type        = map(string)
 }

+variable "lifecycle_rules" {


Since we're hardcoding this now, can we remove the variable?

@kopachevsky To be clear, I don't want any variables for lifecycle_rules in either examples or fixtures, it can just be hardcoded in the example.

did it in example

But you still have a variable in the example. Please remove the variables, as I've said a number of times.

sorry, my mistake

morgante · 2019-11-20T12:25:39Z

test/fixtures/simple_example/variables.tf

@@ -19,3 +19,14 @@ variable "project_id" {
  type        = string
 }

+variable "lifecycle_rules" {


Remove variable.

morgante · 2019-11-20T12:25:51Z

test/fixtures/simple_example/main.tf

@@ -34,5 +34,16 @@ module "example" {
    "one" = true
    "two" = false
  }
+
+  lifecycle_rules = [{


Remove, this is set in the example?

@morgante make sense to leave it in once place, you suggest to set it in example but not here, right?

morgante · 2019-11-20T15:11:21Z

examples/simple_example/README.md

@@ -8,6 +8,7 @@ This example illustrates how to use the `cloud-storage` module.
 | Name | Description | Type | Default | Required |
 |------|-------------|:----:|:-----:|:-----:|
 | bucket\_policy\_only | Disable ad-hoc ACLs on specified buckets. Defaults to true. Map of lowercase unprefixed name => boolean | map(string) | n/a | yes |
+| lifecycle\_rules | List of lifecycle rules to configure. Format is the same as described in provider documentation https://www.terraform.io/docs/providers/google/r/storage_bucket.html#lifecycle_rule except condition.matches_storage_class should be a comma delimited string. | object | `<list>` | no |


@kopachevsky Please regenerate docs.

Added support for lifecycle rules Updated tests and lints Updated variables and readme files

morgante

Looks like tests are failing, please fix.

kopachevsky · 2019-11-20T16:40:18Z

Looks like tests are failing, please fix.

fixed

Stale

aaron-lane assigned kopachevsky Nov 4, 2019

aaron-lane added the enhancement New feature or request label Nov 4, 2019

kopachevsky requested review from paulpalamarchuk and nick4fake November 5, 2019 08:56

nick4fake previously requested changes Nov 5, 2019

View reviewed changes

examples/simple_example/variables.tf Outdated Show resolved Hide resolved

main.tf Show resolved Hide resolved

test/integration/simple_example/controls/gsutil.rb Outdated Show resolved Hide resolved

paulpalamarchuk approved these changes Nov 15, 2019

View reviewed changes

kopachevsky marked this pull request as ready for review November 15, 2019 16:23

morgante suggested changes Nov 20, 2019

View reviewed changes

kopachevsky force-pushed the fix/17/lifecycle-rules branch from 0cf510b to 8ac280f Compare November 20, 2019 12:09

kopachevsky requested a review from morgante November 20, 2019 12:18

morgante suggested changes Nov 20, 2019

View reviewed changes

kopachevsky force-pushed the fix/17/lifecycle-rules branch 3 times, most recently from b58a0c1 to 7b7d9a6 Compare November 20, 2019 15:10

morgante reviewed Nov 20, 2019

View reviewed changes

* Fix terraform-google-modules#17

317c5a5

Added support for lifecycle rules Updated tests and lints Updated variables and readme files

kopachevsky force-pushed the fix/17/lifecycle-rules branch from 7b7d9a6 to 317c5a5 Compare November 20, 2019 15:13

morgante approved these changes Nov 20, 2019

View reviewed changes

Merge branch 'master' into fix/17/lifecycle-rules

ed63d28

morgante suggested changes Nov 20, 2019

View reviewed changes

kopachevsky force-pushed the fix/17/lifecycle-rules branch from 5cd02b7 to ed63d28 Compare November 20, 2019 16:04

Fixed provider for test setup

5b9f298

morgante approved these changes Nov 20, 2019

View reviewed changes

morgante merged commit 9514fd0 into terraform-google-modules:master Nov 20, 2019

terraform-google-modules deleted a comment from comment-bot-dev Jun 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/17/lifecycle rules #19

Fix/17/lifecycle rules #19

kopachevsky commented Nov 4, 2019 •

edited by aaron-lane

Loading

morgante Nov 20, 2019

morgante Nov 20, 2019

morgante Nov 20, 2019

kopachevsky commented Nov 20, 2019

morgante Nov 20, 2019

morgante Nov 20, 2019

kopachevsky Nov 20, 2019

morgante Nov 20, 2019 •

edited

Loading

kopachevsky Nov 20, 2019

morgante Nov 20, 2019

morgante Nov 20, 2019

kopachevsky Nov 20, 2019

morgante Nov 20, 2019

kopachevsky Nov 20, 2019

morgante left a comment

kopachevsky commented Nov 20, 2019

		@@ -1,4 +1,4 @@
		# Terraform Google Cloud Storage Module
		# Terraform Google Cloud Storage Module

	description = "The bucket's Lifecycle Rules configuration. Multiple blocks of this type are permitted. Format is the same as described in provider documentation https://www.terraform.io/docs/providers/google/r/storage_bucket.html#lifecycle_rule except condition.matches_storage_class - it should be comma delimited string."
	description = "List of lifecycle rules to configure. Format is the same as described in provider documentation https://www.terraform.io/docs/providers/google/r/storage_bucket.html#lifecycle_rule except condition.matches_storage_class should be a comma delimited string."

Fix/17/lifecycle rules #19

Fix/17/lifecycle rules #19

Conversation

kopachevsky commented Nov 4, 2019 • edited by aaron-lane Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kopachevsky commented Nov 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morgante Nov 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morgante left a comment

Choose a reason for hiding this comment

kopachevsky commented Nov 20, 2019

kopachevsky commented Nov 4, 2019 •

edited by aaron-lane

Loading

morgante Nov 20, 2019 •

edited

Loading