feat: skip provider registration

[stef-p]

This PR enables the skip_provider_registration field in azurerm provider as documented in:

https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/resource_provider_registration

The default behaviour will set the skip_provider_registration = false

[Carus11]

Simple enough, preserves existing functionality. lgtm. The commit is missing the DCO signoff which is required as part of the contribution guidelines.
Just curious however, in generic terms what was the situation which drove this enhancement? Is it something that should be considered broadly?

[Carus11]

Does this address the issue in #397 ?

[iadomi]

@stef-p Thank you for your contribution and we are currently looking over your PR. But going back to @Carus11's question, what was the situation which drove this enhancement and is it something that should be considered broadly considering in the doc you referenced it states that "if in doubt we strongly recommend letting Terraform register these for you" in regards to Resource Providers

[iadomi]

@stef-p After reviewing your PR a little more and attempting to test your changes locally, we noticed that the skip_provider_registration property has since been deprecated and will be removed in v5.0.

Warning: Argument is deprecated
│
│ with provider["registry.terraform.io/hashicorp/azurerm"],
│ on main.tf line 17, in provider "azurerm":
│ 17: skip_provider_registration = var.skip_provider_registration
│
│ This property is deprecated and will be removed in v5.0 of the AzureRM
│ provider. Please use the resource_provider_registrations property
│ instead.
│
│ (and one more similar warning elsewhere)

Digging at bit deeper it seems that this deprecation was implemented in v4.0.0 (https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/4.0-upgrade-guide#improved-resource-provider-registration) which took place around August 22, 2024 which is after your PR was created.

If you are still interested in this enhancement, we will gladly work with on that but please consider modifying your changes to match the current implementation (https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/guides/4.0-upgrade-guide#improved-resource-provider-registration). Otherwise, if we don't get a response back from you within the next 30 days we will mark this PR stale and will be closed

[stef-p]

@stef-p Thank you for your contribution and we are currently looking over your PR. But going back to @Carus11's question, what was the situation which drove this enhancement and is it something that should be considered broadly considering in the doc you referenced it states that "if in doubt we strongly recommend letting Terraform register these for you" in regards to Resource Providers

I think skip registration was required because the customer has BYO defined for many resources. They have defined aks_uai_name and the managed identity is created out of band from terraform. Terraform was failing because it was expecting to create the managed identity during its run.

[stef-p]

@stef-p Thank you for your contribution and we are currently looking over your PR. But going back to @Carus11's question, what was the situation which drove this enhancement and is it something that should be considered broadly considering in the doc you referenced it states that "if in doubt we strongly recommend letting Terraform register these for you" in regards to Resource Providers

I think skip registration was required because the customer had several BYO resources. They have defined aks_uai_name and the managed identity is created out of band from terraform. Terraform was failing because it was expecting to create the managed identity during its run.

[saschjmil]

Hey @stef-p, the issues you are describing sound similar to #450, which is being fixed in PR #477. Do you think that #477 would solve your issue?

[stef-p]

Hey @stef-p, the issues you are describing sound similar to #450, which is being fixed in PR #477. Do you think that #477 would solve your issue?

I don't think so, in our scenario we are using an existing identity.

[saschjmil]

Hey @stef-p, sorry for closing and re-opening your PR. I deleted the staging branch, which closed all PR's targeting it. I've updated your PR to point to the main branch.

[iadomi]

@stef-p
Before we can move forward with you PR, it needs to meet our Pull Request Requirements. The notable thing there is that we require unit and/or integration test. You can refer to our Testing Philosophy for guidance but we would more be more than happy to help and advise you on that. You can also refer to another PR that you suggested in #389 and #469 for a unit test example as reference for all the changes we would need for this PR. Note the updates to the /docs/CONFIG-VARS.md
and variables.tf and main.tf under /modules/azure_aks

[github-actions]

This PR is stale because it has been open 30 days with no activity.

[abhikumar2204]

Hi @stef-p @Carus11

Could you please check the failure for PR Conventional Commit Validation / validate-pr-title (pull_request), post this we will be able to merge this.

Also let us know if this still needed to be a part of this Repo, or we could stale this and Close without merging.

Thanks!