feat!: Budget module should support filtering on labels #627
Conversation
|
@bharathkkb I'll give it a shot. Thanks for poking me here and on a few other related threads. |
|
@bharathkkb It appears to me that this was merged into the GA and beta providers simultaneously at versions 4.1.0. The CFT modules don't yet support 4.x. If I trace the PRs, the only thing I can find that is an example of what should work is a unit test that does: budget_filter {
projects = ["projects/${data.google_project.project.number}"]
labels = {
label = "bar"
}
}Is this the only supported format? A map from string to string with only 1 element? |
|
Note: GoogleCloudPlatform/magic-modules#5545 probably means provider v4.5 will support modifying budget filters with labels. Will test once available. |
|
@bharathkkb the remaining error appears to be addressed by: |
|
@bharathkkb I believe that I've addressed your concerns and that this is ready to go |
There was a problem hiding this comment.
Thanks for working on this! Lets also expose this via the main module maybe as budget_labels?
examples/budget_project/main.tf
Outdated
| @@ -48,4 +48,5 @@ module "additional_budget" { | |||
| services = var.budget_services | |||
| alert_spent_percents = var.budget_alert_spent_percents | |||
| alert_pubsub_topic = "projects/${module.budget_project.project_id}/topics/${google_pubsub_topic.budget.name}" | |||
| labels = var.budget_labels | |||
There was a problem hiding this comment.
Lets hardcode this in examples
| labels = var.budget_labels | |
| labels = { | |
| "cost-center" : "dept-x" | |
| } |
bharathkkb
changed the title
|
@bharathkkb I think this now merits a review |
| locals { | ||
| perimeter_name = "regular_service_perimeter_${var.random_string_for_testing}" | ||
| } | ||
|
|
||
| module "regular_service_perimeter_1" { | ||
| source = "terraform-google-modules/vpc-service-controls/google//modules/regular_service_perimeter" | ||
| source = "github.com/terraform-google-modules/terraform-google-vpc-service-controls//modules/regular_service_perimeter" |
There was a problem hiding this comment.
Can we open an issue to track revert of this in the future?
There was a problem hiding this comment.
You want me to do this before PR is merged?
There was a problem hiding this comment.
I think it makes the most sense to open this issue immediately after PR is closed to ensure I point to the right commit hash to revert. I've left this specific change as its own commit.
* Require provider 4.5 due to important fixes in 4.1 and 4.5
|
@bharathkkb I think the conversations are ready to be resolved. Can you do another review? I will open issue once you confirm that the PR is ready to go or if you merge it. |
* Update all examples and tests to allow version 4.x of Terraform provider * Bump required version of Google provider to ~> 4.5 for submodules, examples and tests that use budget submodule
|
@tpdownes |
|
Test failure looks unrelated |
This PR adds the ability to support creating budgets that track spending filtered by labels on GCP resources. There is a single CI error that traces to a separate repo. The error will be revised once this PR is accepted (and maybe tagged as well):
terraform-google-modules/terraform-google-vpc-service-controls#65