canonical / traefik-k8s-operator

This charmed operator automates the operational procedures of running Traefik, an open-source application proxy.
https://charmhub.io/traefik-k8s
Apache License 2.0
11 stars 24 forks source link

possible tls issue with traefik-route #328

Open PietroPasotti opened 4 months ago

PietroPasotti commented 4 months ago

Bug Description

in charm._update_dynamic_config_route, we inject tls config regardless of whether TLS is enabled. It's unclear whether that's a bug and if so what the consequences are.

To Reproduce

read the code

Environment

n/a

Relevant log output

n/a

Additional context

we should minimally add some documentation and find out what's going on and why

PietroPasotti commented 2 months ago

Alessandro Cabbia has found that in some cases we generate duplicate tls routers. It may be an instance of this issue:

juju-test-kratos-admin-api-router-tls:
      entryPoints:
      - websecure
      rule: PathRegexp(`^/test-kratos/(admin|schemas)`)
      service: juju-test-kratos-admin-api-service
      tls: {}
    juju-test-kratos-admin-api-router-tls-tls:
      entryPoints:
      - websecure
      rule: PathRegexp(`^/test-kratos/(admin|schemas)`)
      service: juju-test-kratos-admin-api-service
      tls: {}
shipperizer commented 2 months ago

to reproduce (using grafana in this case)

juju deploy grafana-k8s --channel edge
juju deploy traefik-k8s --channel edge

juju integrate grafana-k8s traefik-k8s

juju ssh --container traefik traefik-k8s/0 "cat /opt/traefik/juju/juju_ingress_traefik-route*"

then inspect the grafana file where there are 3 routers instead of 2

http:
  middlewares:
    juju-sidecar-noprefix-test-grafana-k8s:
      stripPrefix:
        forceSlash: false
        prefixes:
        - /test-grafana-k8s
  routers:
    juju-test-grafana-k8s-router:
      entryPoints:
      - web
      middlewares:
      - juju-sidecar-noprefix-test-grafana-k8s
      rule: PathPrefix(`/test-grafana-k8s`)
      service: juju-test-grafana-k8s-service
    juju-test-grafana-k8s-router-tls:
      entryPoints:
      - websecure
      rule: PathPrefix(`/test-grafana-k8s`)
      service: juju-test-grafana-k8s-service
      tls: {}
    juju-test-grafana-k8s-router-tls-tls:
      entryPoints:
      - websecure
      rule: PathPrefix(`/test-grafana-k8s`)
      service: juju-test-grafana-k8s-service
      tls: {}
  services:
    juju-test-grafana-k8s-service:
      loadBalancer:
        servers:
        - url: http://grafana-k8s-0.grafana-k8s-endpoints.test.svc.cluster.local:3000

the above comes from https://github.com/canonical/grafana-k8s-operator/blob/main/src/charm.py#L1325-L1380

shipperizer commented 2 months ago

another diff compared to other ingresses, traefik_route fails to introduce middlewares in each router, see an example below and compare with the above

http:
  middlewares:
    juju-sidecar-noprefix-test-prometheus-k8s-0:
      stripPrefix:
        forceSlash: false
        prefixes:
        - /test-prometheus-k8s-0
  routers:
    juju-test-prometheus-k8s-0-router:
      entryPoints:
      - web
      middlewares:
      - juju-sidecar-noprefix-test-prometheus-k8s-0
      rule: PathPrefix(`/test-prometheus-k8s-0`)
      service: juju-test-prometheus-k8s-0-service
    juju-test-prometheus-k8s-0-router-tls:
      entryPoints:
      - websecure
      middlewares:
      - juju-sidecar-noprefix-test-prometheus-k8s-0
      rule: PathPrefix(`/test-prometheus-k8s-0`)
      service: juju-test-prometheus-k8s-0-service
      tls: {}
  services:
    juju-test-prometheus-k8s-0-service:
      loadBalancer:
        servers:
shipperizer commented 2 months ago

@PietroPasotti https://github.com/canonical/traefik-k8s-operator/blob/main/src/charm.py#L812-L814 seems middlewares key is missing in there

mmkay commented 2 months ago

Duplicate -tls and -tls-tls entries might be a clash between _update_dynamic_config_route generating the config here https://github.com/canonical/traefik-k8s-operator/blame/main/src/charm.py#L806 and traefik generating it too here: https://github.com/canonical/traefik-k8s-operator/blame/main/src/traefik.py#L442

In tempo we had no web entrypoints so this must've stayed overlooked. Might be worth writing a test case for both scenarios.