search

microsoft / reverse-proxy

A toolkit for developing high-performance HTTP reverse proxy applications.
https://microsoft.github.io/reverse-proxy
MIT License
8.55k stars 838 forks source link

Support Cluster Metadata K8s Annotation #1550

Open pinkfloydx33 opened 2 years ago

pinkfloydx33 commented 2 years ago

What should we add or change to make your life better?

The ability to add Metadata to clusters generated by the K8s controller YarpParser in addition to the recently added Route Metadata (#1518).

Why is this important to you?

We are using route/cluster metadata heavily in our project to reduce the amount of "static" configuration required. We use it within ITransformProvider/IProxyConfigFilter to build up preset configurations for both routes and clusters.

More importantly, I am also tracking backend clusters via IClusterChangeListerner. I look for clusters with specific Metadata to identify them as candidates for backends that support a swagger spec so that I can present a unified version hosted in my YARP gateway.

At the moment we are using the ConfigurationConfigProvider so I can just add the Metadata directly to the cluster in configuration. However, we hoped to switch over to a combination of the ConfigurationConfigProvider and the K8s provider once the changes from #1534/#979 are published. Our service/API routes/clusters would now be generated by the k8s provider, with our static/non-k8s setup remaining in config.

With #1518 the k8s parser now adds Route Metadata but does not add cluster Metadata. This leaves me with no way to to identify the generated cluster as a candidate in my change listener.

Since I currently must build the k8s controller myself, its simple enough to add this in, but it seems like something that should already be there.

I suppose there's the problem that multiple routes can (apparently?) end up with the same cluster, so you'd either overwrite the Metadata (as with other cluster options) or need a strategy for merging them.

Alternatives

Instead of introducing a new k8s annotation to split out route vs cluster metadata, the route Metadata could just be copied to the ClusterConfig as well.

If there were a way to retrieve all of the routes associated to a cluster, I could instead just loop over the routes and look for the Metadata there as part of my change listener processing.

Another option would be some way of allowing IProxyConfigFilter to make modifications to the cluster associated with the route in ConfigureRouteAsync. I could then just copy/append the annotations from route to cluster. This however probably has ordering concerns since ConfigureClusterAsync is called first. Plus I doubt you'd ever want to allow modifying anything about the cluster other than Metadata, and even that seems unique to my use case.

MihaZupan commented 2 years ago

I don't see a reason we couldn't add cluster-metadata to annotations, just like we did for route-metadata. My understanding is that the cluster id that is generated by YarpParser should be unique for a given ingress, so overwriting/merging shouldn't be a concern.

Would you be interested in contributing a change for this?

Instead of introducing a new k8s annotation to split out route vs cluster metadata, the route Metadata could just be copied to the ClusterConfig as well.

I was considering something like that in https://github.com/microsoft/reverse-proxy/pull/1518#discussion_r798671088, but we decided we'd prefer keeping the cluster/route metadata separate.

If there were a way to retrieve all of the routes associated to a cluster, I could instead just loop over the routes and look for the Metadata there as part of my change listener processing.

With #1538, you should be able to do that - given a cluster, loop through the routes.