Skip to content
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

Inconvenient coroot ingress helm chart settings #37

Open
gecube opened this issue Oct 9, 2024 · 0 comments
Open

Inconvenient coroot ingress helm chart settings #37

gecube opened this issue Oct 9, 2024 · 0 comments

Comments

@gecube
Copy link

gecube commented Oct 9, 2024

Good day Sirs!

I am again demonstrating coroot to my customers explaining them that it is a good solution. To do it I prepared FluxCD manifests. Everything fine for the exception that Coroot CE does not look like a thing that could be installed in one go. Let's say as a DevOps I want to publish coroot outside to be accessible by users. I need to set up ingress to do it. And what do I see? Completely different semantics than I'm used to see before.

I see the next default values:

  ingress:
    enabled: false
    className: ""
    hostname:
    path: /
    pathType: ImplementationSpecific
    annotations: {}
      # kubernetes.io/ingress.class: nginx
      # kubernetes.io/tls-acme: "true"
    hosts: []
    #  - host: chart-example.local
    #    paths:
    #      - path: /
    #        pathType: ImplementationSpecific
    tls: []
    #  - secretName: chart-example-tls
    #    hosts:
    #      - chart-example.local

so I am just filling hostname and hosts (no idea why, because charts of other application use this semantics) and.... nothing.

I think it would be great to change templates and values to the next as project is on early stage of development and in future it would be much harder to do it:

   ingress: # GOOD!
    enabled: false # GOOD!
    # className: "" # change to ingressClassName
    ingressClassName: ""
    # hostname: # throwing away
    # path: / # throwing away
    # pathType: ImplementationSpecific # throwing away
    annotations: {} # GOOD!
      # kubernetes.io/ingress.class: nginx
      # kubernetes.io/tls-acme: "true"
    hosts: [] # GOOD!
    #  - host: chart-example.local
    #    paths:
    #      - path: /
    #        pathType: ImplementationSpecific
    tls: [] # GOOD!
    #  - secretName: chart-example-tls
    #    hosts:
    #      - chart-example.local

so main changes are

  • remove className in favour of ingressClassName. The logic behind it that there is no className field in Ingress spec and many charts use ingressClassName (examples: sentry, nixys universal chart, victoria stack). It is clear and standard. I don't think that it is good to be different
  • remove dedicated hostname/path/pathType keys. If user of the chart wants, it would be easy to him to populate hosts lists like it is done in other charts. Now the two settings creates confusion. I may guess that authors of the coroot chart thought that user may want to add additional hosts to the default one. But in fact managing of single list is simpler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant