MaikuMori / helm-charts

Maiku's Helm charts
https://artifacthub.io/packages/search?user=MaikuMori&sort=relevance
MIT License
18 stars 11 forks source link

Ingress for gotenberg chart #7

Closed MatthiasLohr closed 1 year ago

MatthiasLohr commented 1 year ago

Hi @MaikuMori,

would you be open to add the possibility for an Ingress object for the gotenberg chart?

Best regards Matthias

MaikuMori commented 1 year ago

Hi, I could, but there is a reason why I haven't added it.

According to docs, Gotenberg should be treated the same as a database and therefore not exposed publicly.

Using Service Gotenberg can be accessed locally within the cluster.

Could you describe a bit more the use-case for an Ingress?

There's a workaround you can use right now where you can just write your own Ingress definition with correct selectors and deploy it to the same namespace as the Helm chart.

MatthiasLohr commented 1 year ago

In my opinion, there is a difference between having an Ingress and allowing public access. Ingresses can do authentication etc.

For example, I have a small Kubernetes cluster running for private things (mostly overkill, but I like to play around with Kubernetes). For writing my PhD thesis, I need a gotenberg instance for converting things, some within a CI/CD pipeline, some from my laptop. Setting up two gotenberg instances in a different way (Kubernetes for CI/CD, Docker for local use) does not feel right. Therefore, I would like to use the Kubernetes instance for all purposes - with authentication of course.

Yes, I could definitely add the Ingress manually - but that's (in my opinion) not the purpose of an helm chart that, despite using a chart, still Kubernetes manifests must be written manually. Furthermore, anyone who installs a helm chart has (IMHO) the responsibility to carefully read the documentation and check available options - such as checking if the service should be exposed to public or not, and setting the values accordingly.

In this case, I would suggest and therefore like to ask you to kindly add an Ingress resource, but e.g. disabled by default. So a responsible administrator could decide if the Ingress should be created after all, but instead writing a custom manifest (which also could lead to problems if you e.g. change labels in upcoming versions, so the custom ingress has to be checked on each upgrade) it's just some values (enabled, hostname, tls configuration, and maybe additional annotations to require authentication).

I would also be open to create a pull request, if you don't have the time for writing the changes.

MaikuMori commented 1 year ago

I don't mind disabled by default ingress. That was the solution I was thinking of. If I have time, I'll add it.

MatthiasLohr commented 1 year ago

Wow, thanks!