-
Notifications
You must be signed in to change notification settings - Fork 686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The "includes" function in HTTPProxy causes bad life-cycle management #2206
Comments
FYI when testing I was impressed that a http connection was "keep-alived" even when the route configuration was altered. Both for multiple Ingress and when updating the "includes:" array in |
Thanks for the detailed issue @uablrek! We've had some similar discussion in #2189 where folks would like the same type of self-service functionality. We will have to have some discussions at one of our community meeting calls about this I think to see if there's a mode that Contour could have to enable this, or utilize OPA, or an Admission controller, etc. The main reason HTTPProxy (and IngressRoute previously) was designed this way was to solve this problem of conflicts, but it does lessen the amount of self-service users might desire in certain environments. |
There are a number of ways to think about security policy and HTTPProxy can support many different policies. Fundamentally, vhosts are global to the cluster, and may even be global to an organization or exposed to the internet. So, we expect that many organizations will need controls around these. Because of this, the HTTPProxy requires that control over the proxy configuration be explicitly delegated from the root to the leaf objects. If we allowed the leaf to attach itself to the vhost, it would be harder to reason about and implement the organizational policy. Most likely, you would need some sort of admission control to ensure that only authorized routes attached themselves to the vhost. When you create a vhost (i.e. a root HTTPProxy), you can delegate the definition of all the routes if you want to. Using your example above:
Now, when you want to use includes to adjust the routes, you only need to edit the
You are right that there are some properties (specifically TLS and vhost policy) that can only me set on the root document. However, because includes can be nested to an arbitrary degree, it is possible to create a document tree where the top document rarely needs to be updated. |
But the problem is that you must alter any object not part of the application when the application is added or removed. To delegate so you don't have to alter the "top" object (defining the vhost) but instead alter the second (common) object in line (or the third,...) does not solve the fundamental problem at all. For instance in your example if I want to remove the "admin" application, it is not sufficient to just remove it, you must also update the "kahttp-delegate" object which is just as bad as updating the "top" object. |
I'd like to explain what I think you're asking for, @uablrek, please correct me if I'm wrong. Note that I'm not saying that we will do this, just wanting to ensure that I understand what you're asking for. In the following, I'm using 'vhost' to mean "An FQDN plus any associated TLS config". Currently, HTTPProxy objects pull config from other HTTPProxy objects into the root document (essentially). What you would like to be able to do is define a vhost somewhere, and then link to that vhost from an arbitrary number of other objects, the superset of which would define the config for that vhost. So, the routes pull in the vhost instead of the other way around. If I'm correct in my understanding, then the questions I have for you are:
Both of those questions were major concerns when we designed the current version of HTTPProxy, so I'd really appreciate your thoughts on how they would work in your proposed solution. |
Not exactly. What I request is a way to add an applucation (e.g with This is the requested function. I think it was a mistake to propose a solution. It might have stopped you from finding a better solution. An example is the "Ingress" object that have this functions since it allows the same vhost to be specified multiple times. This also works fine but there is no support for backend encryption. So another solution might be to allow backend encryption with a annotation in the Ingress object. |
I have not really found and conflicts that does not exist already with includes in a common object. But in general conflicting configs are rejected with some log message. And many sub-object requesting the same path is not a conflict, it is a feature 😄 See the "Canary aspect" above. |
About malicious application attaching to the vhost I have no solution, I don't know enough about RBAC in K8s which should be the mechanism. But I think that requires rights to do just about anything and there are ways to attack the current HTTPProxy setup also with malicious configs. |
So, your current request is:
Is that correct? I'd like to make sure I have the correct thing before we go any further. |
@uablrek Thank you for this clarification. I feel I owe you an explanation which might help explain our reluctance to engage with your request.
The problem with this feature is while it enables some teams to self service, at the same time it enables other teams to accidentally (or maliciously in the case where the tenants of a cluster are not well trusted -- think CI) stomp on each other's vhost configurations. I spoke about this a year ago, https://dave.cheney.net/paste/ingress-is-dead-long-live-ingressroute.pdf, sadly I don't think there is a recording. Put simply, the design of HTTPProxy was directly informed by difficulties our customers had using the existing networking.k8s.io ingress v1beta1 object in real life. HTTPProxy takes as its foundation the notion of delegation that gives the owner, the parent, of a piece of vhost configuration the explicit choice of delegating control to some or all of the configuration to someone else. The model we have in our heads when we discuss this internally is DNS delegation; someone can stand up a DNS server for www.microsoft.com but unless they can convince the owner of .com to change the NS records, their dns server is non canonical. I want to keep talking to make sure we understand your request, but I do need to make clear that the top down delegation model is inherent to the design of HTTPProxy and is unlikely to change at this point. |
I guess this is likely something that has been discussed before, but if not - would this approach be feasible: apiVersion: projectcontour.io/v1
kind: HTTPProxy
metadata:
name: example-root
spec:
virtualhost:
fqdn: example.domain.com
includes:
- name: *
namespace: application-foo-prod The administrator creates root resource This is a variant of delegating the definition of all routes which @jpeach mentioned but without the need to edit the |
I don't think this is a good idea. What if someone deploys two copies of the same httpproxy document to that namespace. Prior to this change the one that is not included directly will be ignored, after this change there will be a duplicate route definition and the entire httpproxy document will be declared invalid. |
True, it would become invalid, but I think similarly two copies of same root HTTPProxy documents will invalidate each other currently. Or explicitly included documents with duplicate routes. On the other hand, if the glob would include documents only from single namespace, problems would be fixable by the team who works in that namespace. |
May I add my two cents to this discussion. I actually started out in the same position as @uablrek, looking for an easier way to create/update/delete individual applications. |
Sorry to take a long time to come back to you @thprice. There's a few things to cover here. With respect to your broader ideas: The usual mechanism for doing something like you describe is in Kubernetes to have a selector of some sort, either "everything of a type in a namespace", or "everything of a type that matches a set of labels", some combination thereof, or similar. In this case, we'd have to do something like adding a selector of some kind to the That would mean that we would end up with a set of conditions (yes, I know, we used the wrong name, but we are stuck with it now), something like this: ...
includes:
- conditions:
- prefix: /teama
namespaceSelector: teama
- conditions:
- prefix: /teamb
namespaceSelector: teamb That would include all the HTTPProxy objects in namespace However, what happens when If they're not both to be rejected, what can we do?
Sadly, you can't do sub-object RBAC inside Kubernetes, so we can't make it so that only some people can edit the What we're running up against here is that HTTPProxy was originally designed to cover the use case of there being a centralised infrastructure team that was okay with parcelling out specific, named HTTPProxies to separate teams, that they were free to either use or further subdivide. Adding in more flexibility is not impossible, but requires careful thought and design. |
I'd point to Gloo as a good prior art when talking about delegating route configuration: https://docs.solo.io/gloo/latest/guides/traffic_management/destination_types/delegation/ It works pretty darn well in my experience, and IMHO provides the right balance of control between infra controllers and app operators. For specifically the case of conflicts, they have a pretty sensible strategy outlines for how conflicts are handled:
|
Thanks @RichiCoder1. I agree that that conflict resolution behavior seems reasonable - it's basically merging the configs, with safeguards around ensuring that all the config actually takes effect. The main other question there is what happens if there's a hard conflict - remember that HTTPProxy also allows inclusion with non-path matching conditions like headers, so it's possible for one to say "this header must be present" and another to say "this header must not be present", and we need a way to handle that case. Again, I should be clear that we're not opposed to this in principle, a design needs to be written up. |
This and Ext Auth support are about the only blockers for us considering Contour, so I'll see if I can do a more formal write up with some options! |
An update on this issue. HTTPProxy was designed for clusters where a cluster-admin wanted to enforce them being in the loop for changes made to the website. Bascially, its design is fundamentally incompatible with a self-service model without an extra controller being laid over the top to facilitate it. This problem has been mostly solved in Gateway API with the changing of HTTPRoutes so that they choose the Gateway that they attach to, so I'm going to mark this as closed now - we anticipate having an early cut of Contour 1.20 before the end of the calendar year with Gateway v1alpha2 support available. Please comment here or in one of our other channels (#contour on Kube Slack, community meetings, etc) if you would like to talk more. |
We could throw the same logic at HTTPProxy to solve the conflict issues. Change how these resources are processed in a cluster and have immediate success with a very simple, streamlined API. |
If there's a way to solve the fundamental incompatibility I noted above, I'd love to see a write up or design doc about it! |
I'd be interested. It still applies to our use case and I guess it may take some time for Gateway API CRDs to reach feature parity with the other functionality that we already have in |
The "includes" function in HTTPProxy causes bad life-cycle management
The "includes" can be used to delegate route definitions to sub-objects;
In a larger site where route definitions are updates regularly this can be used to allow applications to provide their own route definition and add them to the virtual host when loaded and remove them when un-loaded.
The problem is that the "top" object containing the vhost and the "includes" array must be updated for these operations.
This update operation of the top object requires coordination between otherwise independent applications. To update the top object manually is not feasible in a larger sites.
Use Case
As an operator I want to be able to deploy a applications that adds a route definitions to a vhost with a simple install, e.g. with "helm". When an application is removed it's route definition shall be removed automatically.
As an application owner I want to use "secure backend" to protect my traffic.
Comparison with the Ingress object
K8s allows "Ingress" objects to specify the same vhost in multiple instances;
Contour handles this nicely. An "admin" application can be deployed (and removed) independently and the route to "/admin" is updated automatically. The vhost can set with a value/parameter in a helm install.
But Contour supports backend encryption only for
HTTPProxy
.Proposal
Allow to specify the relation in the sub-objects, example;
The "from" field is an array to be compliant with the current implementation where multiple "top" objects can include the same sub-object.
There are of course misconfigurations that must be checked but I leave them for the moment because I can't think of anything unsolvable.
The Canary aspect
A spin-off I find really cool is the elegance of which canary testing can be made with this addition. To test a new
kahttp-admin
simply install a canary with something like;The canary will grab ~10% of the traffic to "/admin". After a test period the canary can simply be removed and traffic goes back to normal. If the canary is ok the normal backend application can be updated.
The text was updated successfully, but these errors were encountered: