Support configuring Internal load balancer for Cloud Run for Anthos

[ZhiminXiang]

This PR enables Terraform to configure internal load balancer for Cloud Run for Anthos.

This PR won't change the default behavior of Cloud Run, i.e. if users do not set load_balancer_type, they will get external load balancer which is the default behavior.

If users want to use internal load balancer, they can set load_balancer_type: internal to get it.

[ZhiminXiang]

/assign @danawillow

[c2thorn]

Overall pretty solid, but I have a couple of concerns about the enum value. Also, do you mind adding a "The cloudrun_config block supports:" documentation bit here:
https://github.com/hashicorp/terraform-provider-google/blob/master/website/docs/r/container_cluster.html.markdown#argument-reference

[c2thorn]

I believe generally we pass the API accepted values through verbatim. Whenever additional settings come around, do you have an idea if they will follow the same naming convention?

I'd rather see LOAD_BALANCER_TYPE_INTERNAL here, or at the very least forcing all caps INTERNAL (for reasons below)

[ZhiminXiang]

Done. Switched to LOAD_BALANCER_TYPE_INTERNAL

[c2thorn]

It will be easier to add more values in the future if we pass through the addon["load_balancer_type"] value here and let the ValidateFunc handle validation.

[ZhiminXiang]

Done

[c2thorn]

Since we are hardcoding "internal" here, setting the balancer type to "INTERNAL" will result in a permanent diff. We should commit to one, or pass through the full LOAD_BALANCER_TYPE_INTERNAL which will allow us to get rid of the if condition.

[ZhiminXiang]

Good catch. Yea, changed it to LOAD_BALANCER_TYPE_INTERNAL

[c2thorn]

Suggested change

	cloudRunConfig := map[string]interface{}{
	"disabled": c.CloudRunConfig.Disabled,
	}
	if c.CloudRunConfig.LoadBalancerType == "LOAD_BALANCER_TYPE_INTERNAL" {
	// Currently we only allow setting load_balancer_type to LOAD_BALANCER_TYPE_INTERNAL
	cloudRunConfig["load_balancer_type"] = "LOAD_BALANCER_TYPE_INTERNAL"
	cloudRunConfig := map[string]interface{}{
	"disabled": c.CloudRunConfig.Disabled,
	"load_balancer_type": c.CloudRunConfig.LoadBalancerType,

now that we are passing the API value through, we don't need the if statement anymore

[ZhiminXiang]

I still want to keep the if check because the server side will return LOAD_BALANCER_TYPE_EXTERNAL for external case when not setting load_balancer_type which we don't want to return and store into state file.

[c2thorn]

Is it okay if we store it in state file? Terraform can handle this situation if we set the field schema as Computed: true
If the user doesn't explicitly state load_balancer_type and the API returns it, setting the field to computed will allow Terraform to default to the API value and not cause any diffs.

Otherwise, we can keep the if and ignore LOAD_BALANCER_TYPE_EXTERNAL. I'm fine either way

[ZhiminXiang]

Thanks @c2thorn. I will just keep it as it is.

[c2thorn]

I'll work on getting this upstreamed to Magic Modules

[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 👉 [email protected]. Thanks!