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

feat!: Add matches_prefix and matches_suffix conditions #202

Conversation

zmaupin
Copy link
Contributor

@zmaupin zmaupin commented Nov 30, 2022

No description provided.

@zmaupin zmaupin requested a review from a team as a code owner November 30, 2022 17:02
@zmaupin zmaupin force-pushed the zmaupin/add-support-for-more-conditions branch 5 times, most recently from 57fe5c4 to dc0f1ab Compare November 30, 2022 18:01
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @zmaupin! A few asks:

version = ">= 3.53, < 5.0"

main.tf Outdated Show resolved Hide resolved
@bharathkkb bharathkkb changed the title Add matches_prefix and matches_suffix conditions feat!: Add matches_prefix and matches_suffix conditions Dec 6, 2022
@zmaupin zmaupin force-pushed the zmaupin/add-support-for-more-conditions branch 2 times, most recently from 6e0a2f1 to c6cd629 Compare December 8, 2022 15:39
@zmaupin
Copy link
Contributor Author

zmaupin commented Dec 9, 2022

@bharathkkb Hello! Is there anything I can do to help move this forward?

Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks @zmaupin
Just waiting on #205 to run the tests

modules/simple_bucket/main.tf Outdated Show resolved Hide resolved
@zmaupin zmaupin force-pushed the zmaupin/add-support-for-more-conditions branch from c6cd629 to 6dc8e98 Compare December 12, 2022 16:29
with_state = "ANY"
age = 365
with_state = "ANY"
matches_prefix = [var.project_id]
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should now be just a string? Tests are failing with

TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185: Error: Invalid function argument
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185: 
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185:   on ../../modules/simple_bucket/main.tf line 75, in resource "google_storage_bucket" "bucket":
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185:   75:         matches_prefix             = contains(keys(lifecycle_rule.value.condition), "matches_prefix") ? split(",", lifecycle_rule.value.condition["matches_prefix"]) : null
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185:     ├────────────────
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185:     │ lifecycle_rule.value.condition["matches_prefix"] is tuple with 1 element
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185: 
TestAll/examples/simple_bucket 2022-12-12T17:57:50Z command.go:185: Invalid value for "str" parameter: string required.

@zmaupin zmaupin force-pushed the zmaupin/add-support-for-more-conditions branch from 73af305 to de6c9db Compare January 3, 2023 15:30
Update versions.tf to use minimum provider version `4.31`
Update simple_bucket module to include matches_prefix and matches_suffix conditions.
@zmaupin zmaupin force-pushed the zmaupin/add-support-for-more-conditions branch from de6c9db to ddec583 Compare January 9, 2023 15:28
@comment-bot-dev
Copy link

@zmaupin
Thanks for the PR! 🚀
✅ Lint checks have passed.

@zmaupin
Copy link
Contributor Author

zmaupin commented Jan 9, 2023

@bharathkkb I figured out the problem. Looks like things are all passing now!

@bharathkkb bharathkkb merged commit 8db2eb3 into terraform-google-modules:master Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants