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

feat(helm): added ingress pathType #1564

Merged
merged 2 commits into from
Sep 15, 2023
Merged

feat(helm): added ingress pathType #1564

merged 2 commits into from
Sep 15, 2023

Conversation

rathberm
Copy link
Contributor

As mentioned in #1534 the ingress pathType was hardcoded to ImplementationSpecific, which is not optimal in every environment. I tried to make it possible to change the pathType in the values file. What do you think?

I'm using the default template function here as to not break existing setups.

@rathberm
Copy link
Contributor Author

rathberm commented Aug 24, 2023

I just saw #1508 where this was discussed too and realized that $ingressPaths is a list of Paths which all need their separate pathType. As it was already mentioned here I don't see an easy way to solve that.

With my current solution multiple paths like this:

  paths:
    - /
    - /ui

will lead to a resource like this:

          - path: /
            pathType: ImplementationSpecific
            backend:
              service:
                name: release-name-akhq
                port:
                  name: http

          - path: /ui
            pathType: ImplementationSpecific
            backend:
              service:
                name: release-name-akhq
                port:
                  name: http

Every path will always have the same pathType, which is the same in the current stat on the master but with my fix you will be able to atleast change the pathType.

@tchiotludo tchiotludo merged commit 6e2f652 into tchiotludo:dev Sep 15, 2023
@tchiotludo
Copy link
Owner

let's try if it sufficient, since it don't break things.

THanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants