Improve identity related variables and output.#197
Conversation
…dation and precondition for identity-related variables. Bump Terraform required version to 1.2.0 since we've used precondition.
change `cluster_identity` type from list to object so the user won't need use index anymore.
| output "system_assigned_identity" { | ||
| value = azurerm_kubernetes_cluster.main.identity | ||
| output "cluster_identity" { | ||
| value = try(azurerm_kubernetes_cluster.main.identity[0], null) |
There was a problem hiding this comment.
Why the try block ? Is there a condition when the cluster has null identity ?
There was a problem hiding this comment.
Why we output in the value only the first element, rather than making available the all list ?
If we export the all list the user of the module can use the output with a index and consume all the identities of the cluster in other resources.
This would be also consistent with how we export the kubelet identity, example:
https://github.com/zioproto/istio-aks-example/blob/main/registry.tf#L10
There was a problem hiding this comment.
Hi, according to the resource schema and identity schema it referenced, the identity in aks resource is a list with maximum length 1. So in this case the identity list has three cases: null, an empty list and a list with exactly one item.
There was a problem hiding this comment.
What happens when you use SystemAssigned, UserAssigned ? Dont you have 2 cluster identities ?
There was a problem hiding this comment.
Should we also patch the CHANGLOG.md to indicate that the output name changed, so folks using the module must update their code ?
|
|
||
| lifecycle { | ||
| precondition { | ||
| condition = (var.client_id != "" && var.client_secret != "") || (var.identity_type != "") |
There was a problem hiding this comment.
identity_type should have nullable = false in the variable definition ?
There was a problem hiding this comment.
identity_type has a validation block, null won't pass the validation so I think it's ok for this case.
| @@ -328,9 +330,14 @@ variable "ingress_application_gateway_subnet_id" { | |||
| } | |||
|
|
|||
| variable "identity_type" { | |||
There was a problem hiding this comment.
Also identity_type should be nullable = false to make sure the condition in main.tf L158 works all the times.
There was a problem hiding this comment.
identity_type has a validation block, null won't pass the validation so I think it's ok for this case.
3 participants
Rename
var.system_assigned_identitytocluster_identity, add validation and precondition for identity-related variables. Bump Terraform required version to 1.2.0 since we've used precondition. #196 related.