Skip to content
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

data source azurerm_kubernetes_cluster - Bypass reading admin config when encounter 403 error #21209

Closed
wants to merge 1 commit into from

Conversation

lonegunmanb
Copy link
Contributor

As @tombuildsstuff described:

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).

This patch should solve #21183 by bypass reading cluster admin config when API response 403 error.

@slzmruepp
Copy link

Why would you do that? If the Admin config can not be read, the context user is clearly not allowed to read it so the error should be thrown. Otherwise the tf fails when trying to access the cluster anyhow because the authentication was never completed. All subsequent providers like helm, etc using the kubeconfig will fail. This is not good practice. Why don't you implement the solution suggested by @browley86? #21183 (comment) ?
Still the problem is not solved by just ignoring the error. Also the option to read the cluster user config protected by RBAC is not addressed here.

@tombuildsstuff
Copy link
Contributor

hi @lonegunmanb

Unfortunately we need to take a different approach with this PR, 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 in favour of #21229 which solves this in a different manner.

Thanks!

@lonegunmanb
Copy link
Contributor Author

lonegunmanb commented Mar 31, 2023

Hi @tombuildsstuff I'm glad that your approach is apparently better than mine. Have a nice day!

@slzmruepp to be honest I didn't quite get @browley86's comment's point. In both resource and data source we call both listClusterUserCredential and listClusterAdminCredential, one for kube_config and one for kube_admin_config. What this deprecated pr tried to do is, set an empty value as data source's kube_admin_config output when the executing account doesn't have permission to read admin config. Since your account is lacking of such permission it won't block you from reading other properties of the cluster, you can still use kube_config if your executing account do have the permission to call listClusterUserCredential API.

@github-actions
Copy link

github-actions bot commented May 1, 2023

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants