microsoft / bedrock

Automation for Production Kubernetes Clusters with a GitOps Workflow
MIT License
130 stars 12 forks source link

Can we use the AKS resource's attributes instead of using aks_msi_client_id_query.sh? #1441

Open jpflueger opened 3 years ago

jpflueger commented 3 years ago

Topic: AKS module's output

Question: The aks_msi_client_id_query.sh script queries information that is already provided via the azurerm_kubernetes_cluster resource attributes. Would you accept a pull request that uses modifies the aks module to use those attributes and removes the dependency on the script?

Script -> Attribute Mapping aks_msi_client_id_query.sh attribute azurerm_kubernetes_cluster attribute Comment
msi_client_id identity.principal_id The term "MSI" has been replaced by "Managed identity", would it be acceptable for a possible rename to avoid confusion?
kubelet_client_id kubelet_identity.object_id The name of this property suggests it is the kubelet's client id but in the current script it is actually the kubelet managed identity's object_id
kubelet_id kubelet_identity.user_assigned_identity_id In the script this field and kubelet_resource_id are mapped to the same value
kubelet_resource_id kubelet_identity.user_assigned_identity_id In the script this field and kubelet_id are mapped to the same value
node_resource_group node_resource_group

Suggestion I would suggest that we just export the following attributes directly from the azurerm_kubernetes_cluster resource:

cc: @jmspring @andrebriggs

jmspring commented 3 years ago

Yes. The work that is in the repo we’ve been working on does away with the script. That code should be back ported to Bedrock.

Sent from my iPhone

On Sep 17, 2020, at 10:47 AM, Justin Pflueger notifications@github.com wrote:

 Topic: AKS module's output

Question: The aks_msi_client_id_query.sh script queries information that is already provided via the azurerm_kubernetes_cluster resource attributes. Would you accept a pull request that uses modifies the aks module to use those attributes and removes the dependency on the script?

Script -> Attribute Mapping

aks_msi_client_id_query.sh attribute azurerm_kubernetes_cluster attribute Comment msi_client_id identity.principal_id The term "MSI" has been replaced by "Managed identity", would it be acceptable for a possible rename to avoid confusion? kubelet_client_id kubelet_identity.object_id The name of this property suggests it is the kubelet's client id but in the current script it is actually the kubelet managed identity's object_id kubelet_id kubelet_identity.user_assigned_identity_id In the script this field and kubelet_resource_id are mapped to the same value kubelet_resource_id kubelet_identity.user_assigned_identity_id In the script this field and kubelet_id are mapped to the same value node_resource_group node_resource_group Suggestion I would suggest that we just export the following attributes directly from the azurerm_kubernetes_cluster resource:

node_resource_group kubelet_identity identity oms_agent_identity * this would be an addition to the current functionality but is necessary for a role assignment to support container insights cc: @jmspring @andrebriggs

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.