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(billingbudget): support project level recicipients on budgets #10926

Conversation

renescheepers
Copy link
Contributor

@renescheepers renescheepers commented Jun 10, 2024

Support for setting the project level recipients: https://cloud.google.com/billing/docs/reference/budget/rest/v1beta1/billingAccounts.budgets#allupdatesrule.

FIXES hashicorp/terraform-provider-google#18182

billingbudget: added `enable_project_level_recipients` field to `google_billing_budget` resource

@github-actions github-actions bot requested a review from c2thorn June 10, 2024 12:27
Copy link

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@c2thorn, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@modular-magician modular-magician added the awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests label Jun 10, 2024
@@ -440,6 +440,7 @@ properties:
- 'notificationsRule.schemaVersion'
- 'notificationsRule.monitoringNotificationChannels'
- 'notificationsRule.disableDefaultIamRecipients'
- 'notificationsRule.enableProjectLevelRecipients'
Copy link
Member

Choose a reason for hiding this comment

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

@modular-magician modular-magician added service/billingbudgets and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Jun 11, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 36 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 2 files changed, 36 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_billing_budget (16 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_billing_budget" "primary" {
  all_updates_rule {
    enable_project_level_recipients = # value needed
  }
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 15
Passed tests: 13
Skipped tests: 1
Affected tests: 1

Click here to see the affected service packages
  • billing

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccBillingBudget_billingBudgetUpdate

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBillingBudget_billingBudgetUpdate[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

@github-actions github-actions bot requested a review from c2thorn June 12, 2024 12:05
@modular-magician modular-magician added awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Jun 12, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 124 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 124 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 16
Passed tests: 14
Skipped tests: 1
Affected tests: 1

Click here to see the affected service packages
  • billing

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccBillingBudget_billingBudgetNotifyProjectRecipientExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccBillingBudget_billingBudgetNotifyProjectRecipientExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$
View the build log or the debug log for each test

Copy link
Member

@c2thorn c2thorn left a comment

Choose a reason for hiding this comment

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

    vcr_utils.go:152: Step 1/2 error: Error running pre-apply refresh: exit status 1
        
        Error: Missing required argument
        
          with google_billing_budget.budget,
          on terraform_plugin_test.tf line 24, in resource "google_billing_budget" "budget":
          24:   all_updates_rule {
        
        "all_updates_rule.0.monitoring_notification_channels": one of
        `all_updates_rule.0.monitoring_notification_channels,all_updates_rule.0.pubsub_topic`
        must be specified
        
        Error: Missing required argument
        
          with google_billing_budget.budget,
          on terraform_plugin_test.tf line 24, in resource "google_billing_budget" "budget":
          24:   all_updates_rule {
        
        "all_updates_rule.0.pubsub_topic": one of
        `all_updates_rule.0.monitoring_notification_channels,all_updates_rule.0.pubsub_topic`
        must be specified

@renescheepers
Copy link
Contributor Author

renescheepers commented Jun 13, 2024

I can't properly test this on my personal account and don't really want to run these tests against a real billing account. The test probably requires an organisation account.

image

When running a test I haven't touched:
image

I've instead just tried applying this Terraform, which should be the same as in the acceptance test:

resource "google_billing_budget" "budget" {
  count = local.billing_account != null ? 1 : 0 # Only create budget if billing is enabled.

  billing_account = local.billing_account
  display_name    = format("Starter budget for %s", google_project.current.project_id)
  budget_filter {
    credit_types_treatment = "EXCLUDE_ALL_CREDITS"
    projects               = ["projects/${google_project.current.number}"]
  }
  amount {
    specified_amount {
      currency_code = "USD"
      units         = tostring(var.billing_budget)
    }
  }
  threshold_rules {
    threshold_percent = 1.0
  }

  all_updates_rule {

    monitoring_notification_channels = []
    enable_project_level_recipients = false
  }
}

@github-actions github-actions bot requested a review from c2thorn June 13, 2024 14:29
@modular-magician modular-magician added awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Jun 13, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 3 files changed, 126 insertions(+), 1 deletion(-))
google-beta provider: Diff ( 3 files changed, 126 insertions(+), 1 deletion(-))
terraform-google-conversion: Diff ( 1 file changed, 11 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 16
Passed tests: 14
Skipped tests: 1
Affected tests: 1

Click here to see the affected service packages
  • billing

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccBillingBudget_billingBudgetNotifyProjectRecipientExample

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccBillingBudget_billingBudgetNotifyProjectRecipientExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

Copy link
Member

@c2thorn c2thorn left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the contribution

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

Successfully merging this pull request may close these issues.

google_billing_budget support for notificationsRule
3 participants