New resource azurerm_resource_provider #7951

Humoiz · 2020-07-29T17:15:21Z

Implementing feature mentioned in the feature request: #1832

This PR adds following new resource, tests and its documentation:
azurerm_resource_provider_registration

It allows to register specified resource providers.

(fixes #1832)

ArcturusZhang

Hi @Humoiz thanks for this PR! This new resource is very useful.

But I suppose the resource name azurerm_resource_provider is not quite clear enough since this resource does not really manage all of the things from a resource provider (it cannot manage features for example) but only the registration, how about rename this to azurerm_resource_provider_registration which should be more specific?

Also I think maybe this resource could just go to the resources category, what do you think?

website/docs/r/resource_provider.html.markdown

ArcturusZhang

LGTM 👍

WodansSon

@Humoiz, thanks for the PR, this LGTM! 🚀

katbyte

Thank you for this pr @Humoiz! overall looks good but i have a few UX concerns that i've commented on inline

azurerm/internal/services/resource/resource_arm_provider_registration.go

website/docs/r/resource_provider_registration.html.markdown

tombuildsstuff

hey @Humoiz

Thanks for pushing those changes - I've taken a look through and left some comments inline but if we can fix those up (and the tests pass) then this otherwise looks good to me 👍

Thanks!

azurerm/internal/services/resource/registration.go

azurerm/internal/services/resource/resource_arm_provider_registration.go

azurerm/internal/services/resource/tests/resource_arm_provider_registration_test.go

dismissing since changes have been pushed

Humoiz · 2020-08-20T18:55:08Z

@tombuildsstuff Thanks for the feedback. I`ll review it and address it soon.

Humoiz · 2020-08-23T07:50:03Z

hey @Humoiz

Thanks for pushing those changes - I've taken a look through and left some comments inline but if we can fix those up (and the tests pass) then this otherwise looks good to me 👍

Thanks!

@tombuildsstuff - Thank you for your feedback. I have addressed all the code comments. Tests are passing after making the changes.

$ TF_ACC=1 go test -v ./azurerm/internal/services/resource/tests/ -run='(TestAccAzureRMResourceProviderRegistration)' === RUN TestAccAzureRMResourceProviderRegistration_basic === PAUSE TestAccAzureRMResourceProviderRegistration_basic === RUN TestAccAzureRMResourceProviderRegistration_requiresImport === PAUSE TestAccAzureRMResourceProviderRegistration_requiresImport === CONT TestAccAzureRMResourceProviderRegistration_basic === CONT TestAccAzureRMResourceProviderRegistration_requiresImport --- PASS: TestAccAzureRMResourceProviderRegistration_requiresImport (78.37s) --- PASS: TestAccAzureRMResourceProviderRegistration_basic (98.71s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/resource/tests 106.920s

tombuildsstuff · 2020-08-24T16:45:07Z

hey @Humoiz

Apologies for the delayed response here - thought I'd replied to this - but thanks for pushing those changes.

Chatting with @grayzu last week, we think there's 4 scenarios we need to cover:

if auto-registration is enabled, and a user tries to enable an RP (which we would auto-register) - error
if auto-registration is enabled, and a user tries to register an RP we're not auto-registering (that's not already registered) - allow/register
if auto-registration is disabled, and a user tries to register an RP that's already registered - raise a "requires import" error
if auto-registration is disabled, and a user tries to register an RP that's not already registered - allow/register

Does that make sense?

Humoiz · 2020-08-24T18:09:50Z

hey @Humoiz

Apologies for the delayed response here - thought I'd replied to this - but thanks for pushing those changes.

Chatting with @grayzu last week, we think there's 4 scenarios we need to cover:

if auto-registration is enabled, and a user tries to enable an RP (which we would auto-register) - error

if auto-registration is enabled, and a user tries to register an RP we're not auto-registering (that's not already registered) - allow/register

if auto-registration is disabled, and a user tries to register an RP that's already registered - raise a "requires import" error

if auto-registration is disabled, and a user tries to register an RP that's not already registered - allow/register

Does that make sense?

@tombuildsstuff - For the scenario, where auto-registration is enabled, and a user tries to register an RP. How do I determine the ones which terraform is registering or not registering?

tombuildsstuff · 2020-08-31T17:06:34Z

@Humoiz at the moment that's a little tricky since you'll have a circular reference - I'll refactor this tomorrow so that's not the case but in theory you'd get this list and string compare them (but as mentioned before, that's not possible right now due to a circular reference).

I'll post an update in the morning once that's been moved which'll make this possible - sorry for the delay.

This is required to unblock #7951, which has a circular reference otherwise

Co-authored-by: Hussain <[email protected]>

…ster RP's Unfortunately the Azure API's error messages when an RP is unregistered are unhelpful as such we recommend folks unfamiliar with the specifics of Resource Provider Registration don't use this resource by default.

dismissing since changes have been pushed

jackofallops

LGTM 👍

ghost · 2020-12-10T20:26:50Z

This has been released in version 2.40.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.40.0"
}
# ... other configuration ...

ghost · 2021-01-06T14:50:12Z

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!

ghost added size/XL dependencies documentation size/XXL and removed size/XL labels Jul 29, 2020

ArcturusZhang reviewed Aug 5, 2020

View reviewed changes

website/docs/r/resource_provider.html.markdown Outdated Show resolved Hide resolved

website/docs/r/resource_provider.html.markdown Outdated Show resolved Hide resolved

ArcturusZhang added new-resource waiting-response labels Aug 5, 2020

ghost added size/XL and removed size/XXL labels Aug 12, 2020

Humoiz requested a review from ArcturusZhang August 12, 2020 18:27

ArcturusZhang approved these changes Aug 15, 2020

View reviewed changes

WodansSon approved these changes Aug 15, 2020

View reviewed changes

katbyte previously requested changes Aug 15, 2020

View reviewed changes

tombuildsstuff self-assigned this Aug 18, 2020

tombuildsstuff previously requested changes Aug 18, 2020

View reviewed changes

tombuildsstuff removed their assignment Aug 18, 2020

tombuildsstuff added the service/resources label Aug 18, 2020

ghost removed the waiting-response label Aug 20, 2020

Humoiz requested a review from tombuildsstuff August 23, 2020 07:47

tombuildsstuff added the waiting-response label Aug 24, 2020

ghost removed the waiting-response label Aug 24, 2020

tombuildsstuff added a commit that referenced this pull request Sep 1, 2020

refactor: splitting Resource Providers out to their own package

b475637

This is required to unblock #7951, which has a circular reference otherwise

tombuildsstuff mentioned this pull request Sep 1, 2020

refactor: splitting Resource Providers out to their own package #8309

Merged

Hussain and others added 16 commits December 4, 2020 11:32

New Resource: azurerm_resource_provider_registration

127c97c

Co-authored-by: Hussain <[email protected]>

resource: adding a Resource ID Parser for Resource Providers

a5be8f3

resourceproviders: support for enhanced validation

7c1e7c8

client/builder: enhanced validation for Resource Providers

58eae31

resource: noting the RP ID Parser isn't generated for now

faf7a43

r/provider_registration: refactoring and support for enhanced validation

429c3db

r/provider_registration: updating the tests

fc4a759

r/provider_registration: updating the tests

0411377

r/resource_provider_registration: updating the docs

80e506d

resource: hooking up the Typed Service Registration

2ee06e9

sdk: support for custom importers

472ef39

r/provider_registration: switching to be a typed resource

43a76e0

r/provider_registration: fixing the import error

900259d

r/provider_registration: renaming to match the convention

449c919

gofmt

45f5384

tombuildsstuff assigned mbfrahry and jackofallops and unassigned tombuildsstuff Dec 4, 2020

tombuildsstuff requested a review from mbfrahry December 4, 2020 10:44

jackofallops approved these changes Dec 7, 2020

View reviewed changes

tombuildsstuff merged commit 360d204 into hashicorp:master Dec 7, 2020

tombuildsstuff added a commit that referenced this pull request Dec 7, 2020

updating to include #7951

e1fc698

Humoiz deleted the hussain/register_providers branch December 8, 2020 17:51

sharebear mentioned this pull request Dec 10, 2020

Add property to document required providers when using azurerm_resource_group_template_deployment #9714

Closed

ghost locked as resolved and limited conversation to collaborators Jan 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New resource azurerm_resource_provider #7951

New resource azurerm_resource_provider #7951

Humoiz commented Jul 29, 2020 •

edited by WodansSon

Loading

ArcturusZhang left a comment

ArcturusZhang left a comment

WodansSon left a comment

katbyte left a comment

tombuildsstuff left a comment

Humoiz commented Aug 20, 2020

Humoiz commented Aug 23, 2020

tombuildsstuff commented Aug 24, 2020

Humoiz commented Aug 24, 2020 •

edited

Loading

tombuildsstuff commented Aug 31, 2020

jackofallops left a comment

ghost commented Dec 10, 2020

ghost commented Jan 6, 2021

New resource azurerm_resource_provider #7951

New resource azurerm_resource_provider #7951

Conversation

Humoiz commented Jul 29, 2020 • edited by WodansSon Loading

ArcturusZhang left a comment

Choose a reason for hiding this comment

ArcturusZhang left a comment

Choose a reason for hiding this comment

WodansSon left a comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

Humoiz commented Aug 20, 2020

Humoiz commented Aug 23, 2020

tombuildsstuff commented Aug 24, 2020

Humoiz commented Aug 24, 2020 • edited Loading

tombuildsstuff commented Aug 31, 2020

jackofallops left a comment

Choose a reason for hiding this comment

ghost commented Dec 10, 2020

ghost commented Jan 6, 2021

Humoiz commented Jul 29, 2020 •

edited by WodansSon

Loading

Humoiz commented Aug 24, 2020 •

edited

Loading