keephq / helm-charts

Charts for Keep https://github.com/keephq/keep
MIT License
7 stars 7 forks source link

[BUG]: Helm chart missing websocket ingress template #11

Closed jackrh closed 6 months ago

jackrh commented 6 months ago

Description of the bug

If you add an ingress to websocket in your values.yaml it will never render the necessary yaml because the websocket template file is missing.

Steps To Reproduce

  1. Add ingress to values.yaml file for websocket
websocket:
  enabled: true
  ingress:
    enabled: true
    hosts:
      - host: keep-websocket.${SECRET_DOMAIN}
        paths:
          - path: /
            pathType: Prefix
    tls:
      - hosts:
          - keep-websocket.${SECRET_DOMAIN}
  1. Install helm chart
  2. Verify no ingress is created

Additional Information

No response

pehlicd commented 6 months ago

I don't think websocket service ever need to have ingress object at all. Instead of adding template for it, removing ingress part from values.yaml would be better. @talboren @shahargl what do you guys think?

talboren commented 6 months ago

I don't think websocket service ever need to have ingress object at all. Instead of adding template for it, removing ingress part from values.yaml would be better. @talboren @shahargl what do you guys think?

Hmm, but we do need ingress for the websocket service if we want FE clients to be able to get push notifications about newly added alerts (real-time updates) -- lmk if I miss understood something

pehlicd commented 6 months ago

I don't think websocket service ever need to have ingress object at all. Instead of adding template for it, removing ingress part from values.yaml would be better. @talboren @shahargl what do you guys think?

Hmm, but we do need ingress for the websocket service if we want FE clients to be able to get push notifications about newly added alerts (real-time updates) -- lmk if I miss understood something

Actually you don't need ingress for that if you are not going to access your websocket service from another cluster. If both websocket and fe services are in same cluster it is kind of best practice to communicate them over services. like: keep-websocket.<namespace>.svc.cluster.local:6001

talboren commented 6 months ago

Contributor

gotcha! thanks for the detailed explanation! @pehlicd do you want to craft a PR to issue that?

pehlicd commented 6 months ago

yeah sure