petoju / terraform-provider-mysql

Terraform MySQL provider – unofficial fork
https://registry.terraform.io/providers/petoju/mysql
Mozilla Public License 2.0
63 stars 40 forks source link

Add Azure configuration block #148

Closed rneacsu5 closed 1 month ago

rneacsu5 commented 1 month ago

This pull request adds the azure_config provider configuration block which allows the user to pass Azure credentials and other configuration options.

This is useful when relying on the DefaultAzureCredential feature of the Azure SDK is not sufficient, such as when configuring multiple servers across multiple tenants from the same Terraform state.

# Configure the MySQL provider for Azure MySQL Server with specific credentials
provider "mysql" {
  endpoint = "azure://your-azure-instance-name.mysql.database.azure.com"
  username = "username@yourtenant.onmicrosoft.com"

  azure_config {
    tenant_id     = "your-tenant-id"
    client_id     = "your-client-id"
    client_secret = var.client_secret
  }
}

All variables are optional, falling back to their corresponding environment variables if absent. If the entire block is missing, the previous behaviour is preserved and DefaultAzureCredential is used.

Tested this manually. Unfortunately, I don't think this can be tested automatically without a test Azure account and an AAD enabled MySQL Server.

petoju commented 1 month ago

@kratkyzobak @frey0814 any opinions? I'm asking you as it may affect you, who are using this.

I'd possibly try to extract this whole azure auth to a separate function for readability, but it generally looks fine to me.

kratkyzobak commented 1 month ago

Hi @petoju, thanks for letting me know.

tl;dr I'm OK with this changes.

My opinion as user - until there is fallback to DefaultAzureCredential, it is not breaking change, so "I don't care".

Opinion as developer (remains same as I stated in PR introducing Entra ID authentication support) - there should be tons of configurations to provide all possible variants of Azure login (as seen in https://github.com/hashicorp/terraform-provider-azuread/blob/main/internal/provider/provider.go#L98-L235). I did not tried to implement any of them in advance (to not need to test them) and instead of it wait for someone who would need them enough to implement them. So subset of these configurations are now implemented to provider, others are still accessible using DefaultAzureCredential.

petoju commented 1 month ago

Thanks for the evaluation! This was released as v3.0.59.

rneacsu5 commented 1 month ago

Thank you all for reviewing and merging this change!

@petoju I'll try to remember to extract that code in a separate function on the next occasion 👍 @kratkyzobak Indeed the current Azure config options are far from complete, but more can be added as needed.