-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fetching AKS Cluster credentials unfortunately does still not work. PR #20927 did NOT solve the issue #21183
Comments
Tagging here @lonegunmanb and @browley86 refering also to the issue in the k8s provider repo: hashicorp/terraform-provider-kubernetes#1964 |
This comment was marked as off-topic.
This comment was marked as off-topic.
@lonegunmanb FYI I'm marking your comment as off-topic since this isn't an approach we'd recommend, the Data Source should support limited permissions, whereas the Resource requires that we have CRUD permissions (including the ability to List the Admin and User Credentials here). |
@tombuildsstuff thanks for the correction, I'll submit a new pr to solve this issue |
In my opinion, the data source:
should use the https://management.azure.com/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerService/managedClusters/{resourceName}/listClusterUserCredential?api-version=2023-01-01 (the provider should assume the authenticated role of the service principal context, terraform is running in) And the data source:
should use the (the provider should assume the authenticated role of the service principal context, terraform is running in) that would be consistent. |
There's a lot flying around here, I think it's worth a summary: there are 3 API endpoints one can call to get cluster creds:
The change here moved from the deprecated
vs
This would be defaulted to the |
I would be anytime available testing the implementation. Just write me a PM @lonegunmanb |
I really am not sure this is going in the right direction. Is the problem clearly understood? There need to be two ways for the data object to get credentials. 1. Admin mode, 2. User mode. So the proposed solution by @browley86 would be the preferred one. See here:
But to suspend the error message when Admin mode is not working because the tf user context has no Cluster Admin Role is not the right approach. The whole K8s part later on, also the Helm chart provider, etc. will all fail because the credentials have not been obtained. See PR here: #21209 |
As mentioned in this comment when using the Data Source we should support limited permissions (that is, access to the AKS Cluster itself, but not necessarily the credentials endpoint) - however the Resource requires that we have CRUD access to the relevant APIs. In this instance whilst both the Data Source and the Resource retrieve the Admin and User Credentials, at present the Data Source isn't correctly handling the limited permissions for either the Admin or User Credential endpoints - which #21129 will fix. Since the provider needs to call both of these APIs to be able to expose these credentials, unfortunately we aren't planning to add a feature-toggle to the provider to select which API to use - however #21129 will solve this issue by supporting limited permissions to both the Thanks! |
Thanks. How will this data then be accessed? Will the limited permissions be provided by calling: and the admin permissions by: How do you handle if the tf user context does not have the "Azure Kubernetes Service Cluster Admin Role"? Will the data structure be empty? Will the tf run fail or will it just ignore silently? |
Correct.
These fields will be empty if there's limited permissions, as in other data sources. So, if the user has access to the User Credentials endpoint, then that'll be populated, else it'll be empty - ditto with the Admin Credentials endpoint. Hope that helps. |
Great, that sounds like a viable solution. It basically reflects all options in one data source depending on the combination of permissions. Thanks! |
This functionality has been released in v3.51.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
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. |
Is there an existing issue for this?
Community Note
Terraform Version
1.4.2
AzureRM Provider Version
3.49.0
Affected Resource(s)/Data Source(s)
azurerm_kubernetes_cluster
Terraform Configuration Files
Debug Output/Panic Output
Expected Behaviour
The implementation should work as documented in the provider:
https://registry.terraform.io/providers/hashicorp/kubernetes/latest/docs/guides/getting-started
Actual Behaviour
Despite the merge of PR:
#20927
the issue:
#20843
is not solved. The whole problem is described here: #20843
We have the following problem now: We dont try to fetch the admin credentials but the USER credentials as you can see here:
Using data.azurerm_kubernetes_cluster.aks_provider_config.kube_config.0.host as OPPOSED to when you fetch the admin config which is not protected by RBAC:
data.azurerm_kubernetes_cluster.aks_provider_config.kube_admin_config.0.host
The implementation here #20927
is buggy because under it seems that it tries to fetch ALWAYS the Admin credentials even when the request is the USER credentials: kube_config versus kube_admin_config. This usecase was not implemented.
Steps to Reproduce
Create a service principal.
Create a security group and add the service principal:
Create a Kubernetes Namespace.
Give the service principal security group the following roles:
Test if the service principal is able to fetch the credentials via:
Which WORKS! The SP gets the proper credentials and is only allowed to manage the specific Namespace it has RBAC Admin permissions on.
It can NOT though fetch the cluster credentials from the proper endpoint by the kubernetes-provider. The issue was not solved with the code change here:
#20927
Important Factoids
No response
References
No response
The text was updated successfully, but these errors were encountered: