crossplane-contrib / provider-gcp

Crossplane GCP provider
Apache License 2.0
119 stars 102 forks source link

Support regional static external IP addresses #407

Closed hferentschik closed 2 years ago

hferentschik commented 2 years ago

What problem are you facing?

I create a GKE cluster using Crossplane. On this cluster, I install via the Helm provider the NGINX Ingress controller. In order to be able to then create a DNS record (via ResourceRecordSet) I need to know the load balancer IP for the Ingress controller.

Ideally, I would create the IP upfront (using GlobalAddress) and then pass it to the Helm install via --set controller.service.loadBalancerIP=<IP> as well as using it in the ResourceRecordSet.

I created the Composition for the above scenario, but it does not work. The load balancer for NGINX stays in the pending state. The problem is that Google Cloud distinguishes between regional and global static external IP addresses. In the case of a Network LoadBalander a regional static external IP needs to be used. See also https://stackoverflow.com/questions/48763805/does-gke-support-nginx-ingress-with-static-ip.

The problem is that the provided GlobalAddress resource in this provider only created global external IP addresses. In fact, the naming might be a bit misleading since external IP addresses from the point of the user are always "global". The distinction between global and regional seems to be more of an implementation detail. ExternalAddress might be a better resource name. Not sure how feasible it is to rename a resource.

Alternative solutions are to either add a region parameter to the GlobalAddress resource or create a new [Regional]Address resource. Both of these options might increase the ambiguities in the naming.

How could Crossplane help solve your problem?

Support regional static external IP addresses via the GCP provider. Possible solutions are:

hferentschik commented 2 years ago

If there is agreement that the name should be changed, one could deprecate GlobalAddress and introduce ExternalAddress and then eventually remove GlobalAddress.

hferentschik commented 2 years ago

This is also relevant in this context. The Google Docs documentation about static external IP addresses - https://cloud.google.com/compute/docs/ip-addresses/reserve-static-external-ip-address

Feggah commented 2 years ago

Thanks for raising this issue, @hferentschik !

I agree with you that the parameter region could be misleading for the name GlobalAddress, and in my opinion, the best solution that we can get would be creating another resource or renaming GlobalAddress.

I know that Address and GlobalAddress are almost identical (the only difference in the SDK is the HTTP methods, where you need to send or not the region parameter) and it makes sense to reuse the implementation to both types, but in Crossplane we always try to reflect the external API, so I would suggest to create a new Address managed resource instead of modifying the GlobalAddress.

You can also check that the terraform provider and terrajet GCP provider have these resources separated from each other and I think it makes more sense to follow these defaults instead of creating a new arbitrary managed resource that represents both of them.

hferentschik commented 2 years ago

I agree with you that the parameter region could be misleading for the name GlobalAddress, and in my opinion, the best solution that we can get would be creating another resource or renaming GlobalAddress.

Cool. IMO renaming would be the best. Independent from the missing region, I have my issue with GlobalAddress or _google_compute_globaladdress (as it is called in Terraform). As a user trying to create infrastructure I want an external IP address. IP addresses are for me per definition "global", so I have been scratching my head over this naming before.

I know that Address and GlobalAddress are almost identical (the only difference in the SDK is the HTTP methods, where you need to send or not the region parameter) and it makes sense to reuse the implementation to both types, but in Crossplane we always try to reflect the external API, so I would suggest to create a new Address managed resource instead of modifying the GlobalAddress.

Right, the underlying REST API makes the address vs global address distinction as well - https://cloud.google.com/compute/docs/reference/rest/v1/addresses and https://cloud.google.com/compute/docs/reference/rest/v1/globalAddresses as well. Aligning with the REST API is a valid strategy I guess. Maybe the code can be generified a bit to have some reuse, otherwise one needs to duplicate the code. I can have a look and update my pull request.