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(azure): add azurerm_active_directory_domain_service #907

Merged

Conversation

kevinkengne
Copy link
Contributor

Objective:

Add support for azurerm_active_directory_domain_service.

Status:

  • Added to resource_registry.go
  • Added resource file
  • Added integration tests
  • Added unit tests if applicable - Not applicable

Issues:

Did not add support for azurerm_active_directory_domain_service_replica_set as I'd like to get some feedback before proceeding with it since this PR is the first I'm raising for infracost.

Useful links:

Pricing: https://azure.microsoft.com/en-us/pricing/details/active-directory-ds/
Terraform: https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/active_directory_domain_service

@CLAassistant
Copy link

CLAassistant commented Aug 14, 2021

CLA assistant check
All committers have signed the CLA.

@tim775
Copy link
Member

tim775 commented Aug 16, 2021

Looks great so far!

Just to be sure you're aware, there are two checks we do after adding a resource that we don't have a great process for (yet!).

First, run the test with warning log level so you can see if there are missing prices or if the price filters are not selective enough: https://github.com/infracost/infracost/blob/master/CONTRIBUTING.md#integration-tests

Second, we compare the golden file output with the cloud pricing calculator (https://azure.microsoft.com/en-us/pricing/calculator/) to make sure the numbers match/make sense.

@kevinkengne
Copy link
Contributor Author

Thanks! Yes, I've done both checks and the numbers match what I've been getting with the cloud pricing calculator.

@alikhajeh1 alikhajeh1 requested a review from tim775 August 17, 2021 10:58
Copy link
Member

@tim775 tim775 left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks again!

@tim775 tim775 merged commit 730c68e into infracost:master Aug 19, 2021
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