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

New data source: azurerm_consumption_budget_subscription #12540

Merged

Conversation

gro1m
Copy link
Contributor

@gro1m gro1m commented Jul 9, 2021

Based off of PR #12150.

@gro1m gro1m changed the title D/azurerm consumption budget subscription D/azurerm_consumption_budget_subscription Jul 10, 2021
@manicminer manicminer changed the title D/azurerm_consumption_budget_subscription New data source: azurerm_consumption_budget_subscription Jul 14, 2021
@gro1m
Copy link
Contributor Author

gro1m commented Jul 14, 2021

Hey @marc-sensenich, it would be great if you find some time to review my current changes, especially because I unfortunately will not be able to run any tests myself (due to new employer restrictions regarding deployments to Azure) for some time now.

I think for me it actually mainly boils down to two questions:

  1. Can I really reuse the common Resource schema in schema.go or do I need to make a modification for data sources?
  2. Does https://github.com/terraform-providers/terraform-provider-azurerm/pull/12540/files#diff-353f0b349789e1658632f79136ef0a279c4f022d5c0bf2395bdd63cadaa2a83dR24 really make all the necessary work to set the values needed for the data source to operate correctly?

(I assume the logic will then also apply to the azurerm_consumption_budget_resource_group and would carry them over to there)

Copy link
Contributor

@marc-sensenich marc-sensenich left a comment

Choose a reason for hiding this comment

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

  1. Can I really reuse the common Resource schema in schema.go or do I need to make a modification for data sources?

You'll need to add a schema for a budget data source with the populated fields being computed = true

  1. Does https://github.com/terraform-providers/terraform-provider-azurerm/pull/12540/files#diff-353f0b349789e1658632f79136ef0a279c4f022d5c0bf2395bdd63cadaa2a83dR24 really make all the necessary work to set the values needed for the data source to operate correctly?

With the correct parameters passed it, it should.

I unfortunately will not be able to run any tests myself (due to new employer restrictions regarding deployments to Azure) for some time now.

I made changes up to the point of getting the tests running and seeing them fail due to the errors shown below. Based on that I made comments on your PR. I figured that you'd want to make the changes yourself as a learning experience and to be a contributor to the project. Depending on your employer's restrictions on Azure at this time, you can create a personal Azure account to run tests against and if you scope your tests correctly you'll just be testing against free resources with subscriptions, resource groups, and budgets. Otherwise, I am happy to help from a peer perspective and assist you with questions and/or testing.

❯ make testacc TEST='./azurerm/internal/services/consumption/' TESTARGS='-run=TestAccBudgetSubscriptionDataSource_current'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test ./azurerm/internal/services/consumption/ -v -run=TestAccBudgetSubscriptionDataSource_current -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
2021/08/03 20:45:20 [DEBUG] not using binary driver name, it's no longer needed
2021/08/03 20:45:21 [DEBUG] not using binary driver name, it's no longer needed
=== RUN   TestAccBudgetSubscriptionDataSource_current
=== PAUSE TestAccBudgetSubscriptionDataSource_current
=== CONT  TestAccBudgetSubscriptionDataSource_current
    testing.go:620: Step 2/2 error: Error running pre-apply refresh: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 8, in data "azurerm_consumption_budget_subscription" "current":
           8: data "azurerm_consumption_budget_subscription" "current" {
        
        The argument "amount" is required, but no definition was found.
        
    testing_new.go:21: WARNING: destroy failed, so remote objects may still exist and be subject to billing
    testing_new.go:21: failed to destroy: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 8, in data "azurerm_consumption_budget_subscription" "current":
           8: data "azurerm_consumption_budget_subscription" "current" {
        
        The argument "amount" is required, but no definition was found.
        
        
--- FAIL: TestAccBudgetSubscriptionDataSource_current (64.52s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/consumption 66.158s
FAIL
make: *** [testacc] Error 1

@marc-sensenich
Copy link
Contributor

@gro1m I've opened a PR on your fork at gro1m#1 to address the larger issue of the schema, feel free to review and merge

…t_subscription

Add schema for consumption budget subscription
@github-actions github-actions bot removed the size/L label Aug 25, 2021
@gro1m gro1m requested a review from jackofallops September 21, 2021 15:21
@gro1m
Copy link
Contributor Author

gro1m commented Oct 9, 2021

Hi @stephybun
Could you review this as well ?
I've incorporated your changes from #12538 analogously here as well.

Copy link
Member

@stephybun stephybun 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 porting over the changes from #12538 to here @gro1m. A few things in the schema and docs need to be fixed up as well as the test since it's failing:

------- Stdout: -------
=== RUN   TestAccBudgetSubscriptionDataSource_basic
=== PAUSE TestAccBudgetSubscriptionDataSource_basic
=== CONT  TestAccBudgetSubscriptionDataSource_basic
testcase.go:88: Step 1/1 error: Error running pre-apply refresh: exit status 1
Error: Reference to undeclared resource
on terraform_plugin_test.tf line 48, in data "azurerm_consumption_budget_subscription" "test":
48:   name            = azurerm_subscription.test.name
A managed resource "azurerm_subscription" "test" has not been declared in the
root module.
Error: Reference to undeclared resource
on terraform_plugin_test.tf line 49, in data "azurerm_consumption_budget_subscription" "test":
49:   subscription_id = azurerm_subscription.test.subscription_id
A managed resource "azurerm_subscription" "test" has not been declared in the
root module.
--- FAIL: TestAccBudgetSubscriptionDataSource_basic (1.96s)
FAIL

@gro1m gro1m requested a review from stephybun October 13, 2021 13:11
Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Thanks @gro1m and @stephybun - This LGTM 👍

@jackofallops jackofallops added this to the v2.81.0 milestone Oct 14, 2021
@stephybun stephybun merged commit 99e17f6 into hashicorp:main Oct 14, 2021
stephybun added a commit that referenced this pull request Oct 14, 2021
@github-actions
Copy link

This functionality has been released in v2.81.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants