Add Azure AD authentication support

[younux]

Hello,

Here is a proposition to add Azure AD authentication support as suggested in #37

I want to add an acceptance test for it but an Azure subscription is needed.

Best regards,

[SemMulder]

Just to mention it here as well: I opened a PR with an alternative approach in #54. I use the same approach as the azurerm and azuread providers there, since the method used in this PR would not work for my use case (we use Azure CLI authentication).

@younux I definitely don't mean to diminish your work, but I had to fix this ASAP for ourselves anyway, so I thought to contribute our fix as well :).

[younux]

Hello @SemMulder

I just want to mention that this is not a fix, this is a new feature.

I had a look at your implementation, I think that it is overkill for something simple. In my approach I kept things very simple (in code and in how to use it).

I don't know your use case exactly, but I think that you can fallback to the case I implemented : Connecting with a service principal and client credentials.

[SemMulder]

I don't know your use case exactly, but I think that you can fallback to the case I implemented : Connecting with a service principal and client credentials.

We are running Terraform in a CI/CD agent (with a corresponding service principal) for which it is impossible to retrieve the client credentials due to compliance. That means I would have to create a new service principal just for connecting to PostgreSQL. So it's possible to work around, but it's a trade-off.

[younux]

@SemMulder Your service principal in the CI is using what to authenticate to Azure ? How do you authenticate against Azure in your CI ? I don't get your use case.

Something that could also help you if you want a solution ASAP is to build and push your provider to terraform registry under your namespace and start using it from now.

[SemMulder]

@SemMulder Your service principal in the CI is using what to authenticate to Azure ? How do you authenticate against Azure in your CI ? I don't get your use case.

Using Managed Identities and the Az CLI within Azure DevOps YAML pipelines: https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/guides/azure_cli

Something that could also help you if you want a solution ASAP is to build and push your provider to terraform registry under your namespace and start using it from now.

This is what I'm currently doing :): https://registry.terraform.io/providers/SemMulder/postgresql/latest

The "ASAP" was meant for the internal timeline, not for this PR ;).

[younux]

@SemMulder If I understand well. You have some VMs where you deployed Azure DevOps private agents (because I don't think managed identity is supported in the shared agents), and you are assigning managed identities to your VMs, then you use your managed identity to authenticate against Azure, right ?

So why don't you add to the case of a service principal I implemented, the case of a managed identity using https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#NewDefaultAzureCredential It would be simple and efficient.

In your PR #54, you are doing a lot of things and I don't understand why do we need all those changes (changing some functions signatures, checking many cases like azure gov ...) Could you explain why and put some examples of all the possibilities you implemented and where they are needed ? This will help understand your changes.

[SemMulder]

@SemMulder If I understand well. You have some VMs where you deployed Azure DevOps private agents (because I don't think managed identity is supported in the shared agents), and you are assigning managed identities to your VMs, then you use your managed identity to authenticate against Azure, right ?

That's correct. These build agents are managed by another team in the company though.

So why don't you add to the case of a service principal I implemented, the case of a managed identity using https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#NewDefaultAzureCredential It would be simple and efficient.

This might be a very good solution actually!

checking many cases like azure gov ...

I assume you are referring to the switch for the resource URI, correct? This is actually rather important, because the resource URI is different depending on which Azure Cloud environment you are in. See here: https://docs.microsoft.com/en-us/azure/postgresql/howto-configure-sign-in-aad-authentication#step-2-retrieve-azure-ad-access-token and here: https://github.com/Azure/azure-cli/blob/04b701947dcda475b42bf38550ebf3cde977621a/src/azure-cli-core/azure/cli/core/cloud.py#L294. I had to hardcode these values for now because the Azure Go SDK doesn't contain these values yet, as far as I could see.

changing some functions signatures

In general: some stuff might fail now, that's why I return an error in some cases. Which function signatures are you concerned about, specifically? Then I can better explain why they were changed :). That said, I think most complexity comes down to the fact that in #54 tokens are cached on a provider level, and in #52 a new token is requested for each new connection. Whether the caching is worth it depends on how many PostgreSQL resources the user is managing using Terraform of course.

I'm sure we can find a middle ground here: the NewDefaultAzureCredential you mentioned seems like a very good option!

I think if we adapt the #52 to use NewDefaultAzureCredential and add the support for the different Azure environments (Gov, Germany, etc.) it would still be KISS and also work for most use cases. I am curious what you think about the caching of tokens? Do you think this would be beneficial?

[cyrilgdn]

@younux @SemMulder Thanks to both for your work on this.

It's a bit hard to me to compare and test them as I almost never used Azure (I don't even have a personal account) and it would take me a long time to understand how Azure authentication is working.

Could you agree on what should be done and propose/adapt a PR that works for all your use cases?

Thanks in advance.

[younux]

I close the pull request because it is taking a lot of time to have a feedback.
Thanks