h3poteto / aws-global-accelerator-controller

A Kubernetes controller for Global Accelerators and Route53
Apache License 2.0
26 stars 8 forks source link

feat(route53): add support for zone filtering #217

Closed emaincourt closed 5 days ago

emaincourt commented 1 month ago

Hi,

Currently, the controller identifies the Route53 zone it needs to handle by selecting the first one that matches the hostnames. Since we have both a private and a public zone for each DNS name, this approach leads to random and unpredictable behavior.

This pull request introduces a route53-zone-id flag, allowing users to specify the exact zone they want to target. This enhancement is essential for us to reliably use the controller in our production environments and also helps to reduce the required IAM permissions.

This change does not address any existing PR. If the implementation does not meet your requirements or if you believe this use case is not relevant, please let me know.

Thank you for developing this controller—it has been invaluable in managing GA across our multi-region deployments.

h3poteto commented 1 month ago

Why do you add a controller parameter?

I suppose it's better to use an annotation like aws-global-accelerator-controller.h3poteto.dev/route53-hostname. If you use route53-zone-id parameter, this controller can handle only one route53 record. If we manage multiple accelerators by this controller, we can't use this flag.

emaincourt commented 1 month ago

I see what you mean. That’s definitely a possibility. It should be fairly easy to rewrite.

The main drawback with that approach is that each and every ingress that the controller manages the records for will need to be aware of the zone id, instead of centralizing the information. External DNS uses ingress annotation filters to match the ones it should manage. A deployment for both public and private zones should then be created.

What do you think of being able to filter at the controller level based on the type (private or public) of the zones, same as External DNS ? That way you could manage multiple domains with the same controller, while always targeting the right zone.

h3poteto commented 4 weeks ago

Ah, you mean

That's a good idea. If we don't specify --route53-zone-ids, please check all route53 zones. It means the default --route53-zone-ids is *.

emaincourt commented 4 weeks ago

I believe we can actually even omit the ids of the route53 zones. I created a POC branch here that I deployed to a development cluster to test the behavior: https://github.com/omi-lab/aws-global-accelerator-controller/commit/eb9a603261380729bb64106ea9448766aacd7895.

Basically it implements exactly the same logic as external dns:

To summarize:

If you prefer to go with a static list of zone ids, I can also implement that. What do you think ?

h3poteto commented 4 weeks ago

If you prefer to go with a static list of zone ids

Hmm, I prefer to go with a static list of zone ids, because it can only handle public or private route53 zones when we specify --route53-zone-type flag. Users will manage multiple accelerators and route53 records by one aws-global-accelerator-controller, so it should be able to handle public and private route53 zones simultaneously.

emaincourt commented 3 weeks ago

Got it. What about filtering the ingresses/services ?

h3poteto commented 3 weeks ago

What about filtering the ingresses/services ?

Isn't it enough to have a hosted-zone filter? I think it's better to manage it with annotation rather than controller flags.

emaincourt commented 2 weeks ago

Assuming you have records that are both public and private, and others that are exclusively private, or public, how would you handle it ? With hosted zone filter the controller will parse all ingresses matching the annotations, without any notion of ownership.

h3poteto commented 2 weeks ago

Ah, you mean that if I have

this controller can't decide which hosted zone should be used for an ingress. This controller doesn't know which hosted zone should public ingress use. Correct?

h3poteto commented 2 weeks ago

If so, is it easy to add annotation instead of controller flags? To archive this requirement, we need two controller flags:

However, it is enough if we use an annotation, for example

The controller can decide which hosted zone should be used for the Ingress/Service regardless of whether it is public or private. What do you think?

emaincourt commented 1 week ago

Adding the annotation is definitely an easy change. As I mentioned earlier in our conversation, the only drawback is that it requires an update of all the ingresses of the cluster, plus all ingresses to be aware of the zone id. Filtering based on the ingress class makes is more "agnostic" and allows for automatic discovery of the zones. It's up to you.

h3poteto commented 1 week ago

OK, I got it. Please proceed by adding annotations.