New resources: azurerm_lighthouse_definition and 'azurerm_lighthouse_assignment'

[Humoiz]

Implementing features mentioned in the feature request: #3941

This PR adds following new resources and its documentation:
azurerm_lighthouse_definition
azurerm_lighthouse_assignment

Data source:
azurerm_lighthouse_definition

The resources in this PR needs multiple tenant to run the scenario. Tenant ID and Object ID of the user, usergroup or service principal is needed from the second tenant. I ran the tests after setting the environment variable ARM_TENANT_ID_ALT & ARM_PRINCIPAL_ID_ALT_TENANT locally.

Here is the azure docs which explains how to setup these resources:
https://docs.microsoft.com/en-us/azure/lighthouse/how-to/onboard-customer#gather-tenant-and-subscription-details

All the tests are passing:

$  TF_ACC=1 go test -v ./azurerm/internal/services/lighthouse/tests/ -run='(TestAccDataSourceAzureRMLighthouseDefinition)'
=== RUN   TestAccDataSourceAzureRMLighthouseDefinition_basic
=== PAUSE TestAccDataSourceAzureRMLighthouseDefinition_basic
=== CONT  TestAccDataSourceAzureRMLighthouseDefinition_basic
--- PASS: TestAccDataSourceAzureRMLighthouseDefinition_basic (39.53s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/lighthouse/tests    41.870s

$  TF_ACC=1 go test -v ./azurerm/internal/services/lighthouse/tests/ -run='(TestAccAzureRMLighthouseAssignment)'
=== RUN   TestAccAzureRMLighthouseAssignment_basic
=== PAUSE TestAccAzureRMLighthouseAssignment_basic
=== RUN   TestAccAzureRMLighthouseAssignment_requiresImport
=== PAUSE TestAccAzureRMLighthouseAssignment_requiresImport
=== RUN   TestAccAzureRMLighthouseAssignment_emptyID
=== PAUSE TestAccAzureRMLighthouseAssignment_emptyID
=== CONT  TestAccAzureRMLighthouseAssignment_basic
=== CONT  TestAccAzureRMLighthouseAssignment_emptyID
=== CONT  TestAccAzureRMLighthouseAssignment_requiresImport
--- PASS: TestAccAzureRMLighthouseAssignment_emptyID (57.84s)
--- PASS: TestAccAzureRMLighthouseAssignment_basic (58.12s)
--- PASS: TestAccAzureRMLighthouseAssignment_requiresImport (70.02s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/lighthouse/tests    72.475s

$  TF_ACC=1 go test -v ./azurerm/internal/services/lighthouse/tests/ -run='(TestAccAzureRMLighthouseDefinition)'
=== RUN   TestAccAzureRMLighthouseDefinition_basic
=== PAUSE TestAccAzureRMLighthouseDefinition_basic
=== RUN   TestAccAzureRMLighthouseDefinition_requiresImport
=== PAUSE TestAccAzureRMLighthouseDefinition_requiresImport
=== RUN   TestAccAzureRMLighthouseDefinition_complete
=== PAUSE TestAccAzureRMLighthouseDefinition_complete
=== RUN   TestAccAzureRMLighthouseDefinition_update
=== PAUSE TestAccAzureRMLighthouseDefinition_update
=== RUN   TestAccAzureRMLighthouseDefinition_emptyID
=== PAUSE TestAccAzureRMLighthouseDefinition_emptyID
=== CONT  TestAccAzureRMLighthouseDefinition_basic
=== CONT  TestAccAzureRMLighthouseDefinition_update
=== CONT  TestAccAzureRMLighthouseDefinition_requiresImport
=== CONT  TestAccAzureRMLighthouseDefinition_complete
=== CONT  TestAccAzureRMLighthouseDefinition_emptyID
--- PASS: TestAccAzureRMLighthouseDefinition_emptyID (35.96s)
--- PASS: TestAccAzureRMLighthouseDefinition_complete (37.20s)
--- PASS: TestAccAzureRMLighthouseDefinition_update (59.27s)
--- PASS: TestAccAzureRMLighthouseDefinition_basic (68.36s)
--- PASS: TestAccAzureRMLighthouseDefinition_requiresImport (87.92s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/lighthouse/tests    90.403s

[katbyte]

@Humoiz,

It's not entire clear what a registration definition applies to, i think as the ticket suggested azurerm_lighthous_* would be better. WDYT?

[Humoiz]

@katbyte

It's not entire clear what a registration definition applies to, i think as the ticket suggested azurerm_lighthous_* would be better. WDYT?

I have used the naming conventions used in the SDK and Azure REST docs. However, I`m open to changing it to azurerm_lighthouse_* if that is recommended. Let me know.
https://docs.microsoft.com/en-us/rest/api/managedservices/

[Humoiz]

@katbyte - This PR is ready for your review. Please provide your feedback when time permits.

[Humoiz]

@katbyte - Just checking if I can answer any questions you may have.

[katbyte]

@Humoiz - the SDK & API often leave much to be desired in their clarity of naming. Could we update these to all use lighthouse including the service package name? thanks!

[Humoiz]

@Humoiz - the SDK & API often leave much to be desired in their clarity of naming. Could we update these to all use lighthouse including the service package name? thanks!

@katbyte - I have renamed from registration_* to lighthouse_*, also updated the function names, etc. Let me know how does it looks to you now.

[Humoiz]

@katbyte -I have addressed your initial comments. I need to use these new resources in about two weeks. Can you please provide your feedback?

[Humoiz]

@katbyte -The build is failing and I am not sure how to fix it. I tried 'go mod vendor' command and it deletes several folders, not sure if I am missing anything. Any pointers?

[katbyte]

@Humoiz - i merged master and fixed up the vendor folder 🙂

[Humoiz]

@Humoiz - i merged master and fixed up the vendor folder 🙂

Thank you!! :)

[katbyte]

Thanks @Humoiz, i;ve left some comments mostly around naming - otherwise this is looking good and should get into the release next week!

[jackofallops]

Hi @Humoiz
I hope you don't mind, but I've resolved a merge conflict and rebased to bring the branch up to date to help us assess if this can go in manually tested ahead of the changes we're working on for the binary testing capability that's imminently due. I'll be focused on this work today and I'm hoping we'll be able to get it in for the next release.

[Humoiz]

Hi @Humoiz
I hope you don't mind, but I've resolved a merge conflict and rebased to bring the branch up to date to help us assess if this can go in manually tested ahead of the changes we're working on for the binary testing capability that's imminently due. I'll be focused on this work today and I'm hoping we'll be able to get it in for the next release.

Thank you. I`m glad that this work is getting the traction again.

[jackofallops]

Thanks @Humoiz - I think I'm done with my changes, and I've tested all the scenarios I can think of manually, and we'll turn that into acceptance tests when we complete the binary testing work. I'm going to get someone else from the team to double check my work, and if that gets the all clear we'll get this merged asap.

[tombuildsstuff]

Some minor comments whilst passing through but this otherwise LGTM 👍🏻

[mbfrahry]

LGTM!

Outdated

[ghost]

This has been released in version 2.28.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.28.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!