Helm Chart Finalization

[acelinkio]

Feature description

Follow up from #560 MR. Here is a checklist of items to improve the helm chart further and prepare it for prime time!

• Add CI workflows to test chart
• Add CI workflows to publish chart to a Helm/OCI repository
• Add common ingress controller annotations for rewrite needed on ingress (Will add docs if Add prefix every route to kyoo_back #653 is not added before then)
• Add installation documentation (chart needs to be published)
• Uniformly quote environment variables throughout the chart
• Improve values.yaml comments/documentation

[acelinkio]

Having some troubles with common ingress controller annotations for rewrite as there is no standard implementations across controllers.

nginx-ingress: Most common ingress controller

• rewrite via configuration-snippet annotation is the commonly used method by many people. However it is no longer enabled by default due to security concerns. https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/annotations.md#configuration-snippet
• rewrite via rewrite-target is the alternative method. However it requires that each path be split into its own ingress resource. https://kubernetes.github.io/ingress-nginx/examples/rewrite/#rewrite-target

haproxy: 2nd most common

• rewrite via path-rewrite annotation. The most flexible to implement because you just repeat the path rule. https://www.haproxy.com/documentation/kubernetes-ingress/community/configuration-reference/ingress/#path-rewrite

nginx inc:

• rewrite via rewrites annotation. Requires that you use the Kubernetes servicename in the annotation. That is annoying because template functions can't be used inside of values.yaml https://github.com/nginxinc/kubernetes-ingress/tree/main/examples/ingress-resources/rewrites

traefik:

• requires its own stripprefix custom resource. https://doc.traefik.io/traefik/middlewares/http/stripprefix/#prefixes

caddy:

• does not appear to be a widespread enough ingress controller. Docs lack examples for rewrite annotations. https://github.com/caddyserver/ingress/blob/main/internal/caddy/ingress/annotations.go#L7-L8

The next api replacing ingresses, GatewayAPI, handles rewrites in a standardized way

spec:
  rules:
  - matches:
    - path:
        type: PathPrefix
        value: /api
    backendRefs:
    - name: kyoo-back
      port: 5000
    filters:
      - type: URLRewrite
        urlRewrite:
          path:
            type: ReplacePrefixMatch
            replacePrefixMatch: /
  - matches:
    - path:
        type: PathPrefix
        value: /
    backendRefs:
    - name: kyoo-front
      port: 8901

Honestly did not realize how painful it is to handle rewrites. @onedr0p / @joryirving / @bo0tzz any thoughts on how to approach templating ingresses that support rewrite rules?

[bo0tzz]

I'm not a fan of rewrites for these sort of reasons tbh. How feasible is it to just get rid of the need on an app level, eg by having kyoo-back listen directly on /api @zoriya?

[zoriya]

I could add a env var to prefix every routes if needed yes

[acelinkio]

I could add a env var to prefix every routes if needed yes

I think that is that is the solution. Until that feature is added, can add in docs the above links on how to manage the rewrite rules. Created for tracking #653.

[qjoly]

New Kyoo user here 👋, I discovered yesterday how installation works on Kubernetes. I proposed a small PR for the Helm chart and discovered this ingress problem while testing it.

I have the same conclusions as you (kyoo_back which doesn't require rewrite rules would just be gold.

• Using the Gateway API is the long term solution, I think we should support it (but it's not the priority, knowing that 99% of Kubernetes users are still on ingress)
• We could try to support several ingress-controller syntaxes (I saw your research about traefik/haproxy/nginx, very interesting!), but we risk vendor-locking (or even maintenance difficulties, I use Istio on my side, but it's not common, so should we really support it ? Who decide which ingress we will have to maintain ?), this is a complex problem.

Until #653 is implemented (and the release of GatewayAPI for most commons Ingress controller), we could just add another component inside the cluster. This would be an equivalent to the Traefik in the docker setup. I suggest using HAProxy, which is IMO the easiest (and most flexible) middleware to configure, but it's open to debate (see #708)

[acelinkio]

Adding that ingress complexity in the helm chart means eventually removing it and breaking those that rely upon it. I think just offering a basic ingress definition would be better than trying to address every edge case. Most helm charts do not address these ingress complexities.

I plan on adding some documentation to highlight ingress and provide users a way to address it. Users can use the extraObject to include any additional manifests that they want.

I'll reply to your #708 MR later today. Wish you would have started a discussion before suggesting a large amount of changes.

[qjoly]

Users can use the extraObject to include any additional manifests that they want.

In my opinion, the extraObject is an anti-pattern or should only be used in situations where the chart cannot handle all cases, the ingress subject should not be managed here (or only for edge cases, like with my Istio)

Sorry for not discussing this here before proposing the changes. Depending on your review, I can split in two merge requests to separate the two topics (value management and 'route' component)