azurerm_data_factory_linked_service_data_lake_storage_gen2 - Supports managed identity auth through use_managed_identity
#8938
Conversation
…ate using managed identity Error if neither MI nor SPN are specified Remove auth ocnflict
There was a problem hiding this comment.
Hey @dnlbunting, thanks for opening this PR! Things look nearly there but some of the validation you wrote can be better surfaced at the plan level so we error before Terraform even applies. I've detailed some of what you need below and feel free to reach out if you have any questions around it.
| ValidateFunc: validation.IsUUID, | ||
| }, | ||
|
|
||
| "service_principal_key": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| Optional: true, |
There was a problem hiding this comment.
We have a nifty schema valdiation check we can add. It's called RequiredWith and it takes an array of strings and makes sure that if one of those attributes is specified, than all of the attributes in that array must also be specified. It'll error if any aren't which should remove the logic you wrote out down below!
There was a problem hiding this comment.
We'll also probably want do add ConflictsWith: []string{"use_managed_identity"} to all of these attributes
| @@ -58,21 +58,27 @@ func resourceArmDataFactoryLinkedServiceDataLakeStorageGen2() *schema.Resource { | |||
| ValidateFunc: validation.IsURLWithHTTPS, | |||
| }, | |||
|
|
|||
| "use_managed_identity": { | |||
| Type: schema.TypeBool, | |||
| Optional: true, | |||
There was a problem hiding this comment.
We'll want to add ConflictsWith here too to make sure we don't specify it with the other service principal attributes.
There was a problem hiding this comment.
And to round it all out we should throw in AtLeastOneOf with all of these attributes to make sure at least one of them gets specified when creating this resource.
|
@mbfrahry Thanks for the pointers on the schema validation, I'll make those changes. IMO making the managed identity opt-in rather the the default was clearer, since the managed identity has to be explicitly enabled in the parent ADF resource too. |
|
That works for me! I'll look forward to the schema validation changes |
mbfrahry
changed the title
azurerm_data_factory_linked_service_data_lake_storage_gen2 - Supports managed identity auth through use_managed_identity
|
Thanks for this @dnlbunting. It should make it into the next release! |
|
This has been released in version 2.35.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.35.0"
}
# ... other configuration ... |
|
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 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked as resolved and limited conversation to collaborators
Closes #6501
Adds a use_managed_identity flag to azurerm_data_factory_linked_service_data_lake_storage_gen2.
If both use_managed_identity=true and service_principal_key or service_principal_id are specified raises an error.
First contribution so feel free to add feedback