service_principal in aks no longer required

[torresdal]

Fixes #6178. service_principal is no longer required, as Managed Identity is now GA: https://github.com/Azure/AKS/releases/tag/2020-03-16

Been a while since I've contributed here, so probably other files that needs to be updated?

Looks like version 2019-11-01/containerservice (which is the current in terraform) already support managed identities, but there are newer API versions available which I have not studied in detail.

[katbyte]

Hi @torresdal,

thanks for the PR, all it is missing is a test without the SP to confirm it works (and doesn't stop working in the future) and the docs to be updated.

[ghost]

According to the docs, when an AKS cluster is created with managed identity, the service_principal should be set like this:

  service_principal {
    client_id     = "msi"
    client_secret = null
  }

Therefore, client_secret may also need to be made optional when client_id=msi.

[anlutro]

The way I read the Azure docs, you shouldn't set the client ID/secret to that, it's just what Azure will set it to for you. If the whole service_principal block becomes optional I don't see why client_secret needs to be set to be optional.

[elsesiy]

@anlutro That's how I read the docs too. It says
A successful cluster creation using managed identities contains this service principal profile information which nowhere indicates the user should provide these values. What's the ETA change and how will this work for clusters that have been provisioned in the transitional period with a SP and MI?

[tombuildsstuff]

hey @torresdal

Thanks for this PR - apologies for the delayed review here, we've been trying to determine the intended behaviour of the AKS API to work out how to move forward here - since the API's changed behaviour several times during the past couple of weeks.

Prior to the recent changes in the Azure API there were 2 different use-cases for the AKS resource:

1. Creating an AKS Cluster using a Service Principal for Cluster Authentication
2. Creating an AKS Cluster using a Service Principal for Cluster Authentication with a Managed Identity attached

However the GA of the Managed Identity functionality introduced the third use-case was introduced:

3. Creating an AKS Cluster using a Managed Identity for Authentication

After spending some time digging into this, it appeared this (after chatting with the AKS Team, by design) broke use-case 2 here (creating a Cluster with a Service Principal & a Managed Identity attached), however it appeared that we could conditionally use the older API version to support use-case 2, meaning we'd support all 3 use-cases here.

Unfortunately after finishing up support for that (and confirming it works) the older API (2019-10-01) has been updated such that use-case 2 is no longer possible; as such unfortunately we're going to need to make some larger changes to the AKS resource.

Since AKS is a highly used and fast moving resource, there's currently several PR's open which change the functionality of this resource. In order to be able to make the larger changes needed for this issue - and consulate the other PR's which are open, I hope you don't mind but I'm going to close this PR in favour of #6095, which includes support for creating an AKS Cluster without a Service Principal, and the other changes mentioned above.

As such whilst I'd like to thank you for this contribution, I hope you don't mind but I'm going to close this PR in favour of #6095, which incorporates these changes and will ship in v2.5 of the Azure Provider.

Thanks!

[ghost]

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!