[traefik-route] middleware in router tls is removed

hemanthnakkina commented 2 months ago

Bug Description

The charm integrates with traefik via traefik-route interface. And the charm updates relation data with the dynamic traefik configuration that includes middlewares. The middlewares added in router non-tls are retained in the file /opt/traefik/juju/ where as the middlewares specified in router tls section are removed.

This is easily reproducible by unit tests. Patch file with unit test attached

To Reproduce

Clone the latest traefik-k8s-operator code
Apply patch file attached test-traefik-route-middleware-patch.txt
Run unit test cases tox -e unit

Environment

Juju 3.4 + microk8s v1.28.7 traefik charm : 1.0/stable 164

However this is reproducible on latest charm as well.

Relevant log output

Unit test assert failure output:

  Differing items:
  {'http': {'middlewares': {'custom-headers-http': {'headers': {'customRequestHeaders': {'X-Forwarded-Proto': 'http'}}},...foo-service', 'tls': {'domains': [{...}]}}}, 'services': {'juju-foo-service': {'loadBalancer': {'servers': [{...}]}}}}} != {'http': {'middlewares': {'custom-headers-http': {'headers': {'customRequestHeaders': {'X-Forwarded-Proto': 'http'}}},...th`)', 'service': 'juju-foo-service', ...}}, 'services': {'juju-foo-service': {'loadBalancer': {'servers': [{...}]}}}}}

  Full diff:
    {
        'http': {
            'middlewares': {
                'custom-headers-http': {
                    'headers': {
                        'customRequestHeaders': {
                            'X-Forwarded-Proto': 'http',
                        },
                    },
                },
                'custom-headers-https': {
                    'headers': {
                        'customRequestHeaders': {
                            'X-Forwarded-Proto': 'https',
                        },
                    },
                },
            },
            'routers': {
                'juju-foo-router': {
                    'entryPoints': [
                        'web',
                    ],
                    'middlewares': [
                        'custom-headers-http',
                    ],
                    'rule': 'PathPrefix(`/path`)',
                    'service': 'juju-foo-service',
                },
                'juju-foo-router-tls': {
                    'entryPoints': [
                        'websecure',
                    ],
  -                 'middlewares': [
  -                     'custom-headers-https',
  -                 ],
                    'rule': 'PathPrefix(`/path`)',
                    'service': 'juju-foo-service',
                    'tls': {
                        'domains': [
                            {
                                'main': 'testhostname',
                                'sans': [
                                    '*.testhostname',
                                ],
                            },
                        ],
                    },
                },
            },
            'services': {
                'juju-foo-service': {
                    'loadBalancer': {
                        'servers': [
                            {
                                'url': 'http://foo.testmodel-endpoints.local:8080',
                            },
                        ],
                    },
                },
            },
        },
    }

Additional context

test-traefik-route-middleware-patch.txt

PietroPasotti commented 2 months ago

So this happens because traefik charm generates a tls config for the route and overwrites the one you passed.

A fix could be: if the user provided a tls config, respect that instead of overwriting. Only issue is: how can traefik know that a router definition is a tls one?

For now, I suggest you try a workaround: rename your tls router to a different prefix. Traefik uses "{router_name}-tls" for the tls router, if you name your router "{router_name}-https" or something else, traefik will not overwrite it. However, it will configure traefik with three routers, of which two for tls, and only one of them will have the middlewares you passed, and I'm not sure that will work.

PietroPasotti commented 2 months ago

a somewhat more sustainable solution could be to extend the route interface with a tls-middlewares arg so that traefik knows what middlewares to inject in the tls definition. Which still means you leave to traefik the task of configuring a tls route for your entrypoint, which I'm not sure it's a good practice either. Perhaps we should say: if you're using traefik route, you know what you're doing and we're not going to touch your config. Why are we injecting tls stuff in it?

@simskij or @sed-i do you have an opinion here?

hemanthnakkina commented 2 months ago

There is another problem with traefik_route interface itself.

The related charm is unaware of the traefik configurations but constructing traefik config based on assumptions. I believe the interface itself needs to pass the relevant traefik configuration to the charm via the interface if it expects the charm to provide complete configuration

PietroPasotti commented 2 months ago

that's not scalable though, you incur precedence issues if you have multiple charms using traefik-route I think the interface is fine: a requirer charm gives traefik a valid dynamic config file. The question is: should traefik patch that file or take it as-is?

hemanthnakkina commented 2 months ago

@PietroPasotti It works with the workaround you mentioned if i set a priority for "{router_name}-https"

sed-i commented 2 months ago

In traefik, would we be able to easily include user middlewares, @PietroPasotti?
@hemanthnakkina, what are you using traefik route for? The only use case I am aware of is "ingress-to-leader" for grafana, which we want to replace with a proper charm lib in the near future. According to charmhub, there are no other uses.

hemanthnakkina commented 2 months ago

@sed-i I am not sure how the charmhub populates its list but its definitely missing some charms. I looked at ingress interface as well and it does not list any charms owned by OpenStack Charmers.

Couple of sunbeam charms (nova-k8s, heat-k8s) are using traefik route interface predominantly for 2 reasons:

Multiple ports per charm need to be exposed via traefik ( ingress interface allows only single app single port )
Use middleware in traefik configuration

Note: Sunbeam charms starts multiple apps as separate containers within a pod

simskij commented 2 months ago

We're seeing this in other places as well, even in our own charms, and I think we might need to start thinking about how to properly support multi-route or multi-port ingressing over the ingress interface. @gregory-schiano, do you have a similar concept in nginx?

gregory-schiano commented 2 months ago

Well we don't have multi-port usecase for now, but multi-route is one. The way I intend to solve the multi-route is by using multiple ingress relation on the requirer charm (one per route) and the custom-url-routing we've discussed here https://github.com/canonical/charm-relation-interfaces/issues/79 and in Riga Note that multi-port might resolved using multiple ingress relation with the requirer charm since the port attribute is already supported.

My overall though is I think summed up is the following comment https://github.com/canonical/charm-relation-interfaces/issues/79#issuecomment-1624950433

canonical / traefik-k8s-operator