pmint93 / helm-charts

My helm charts
https://pmint93.github.io/helm-charts/
Apache License 2.0
73 stars 72 forks source link

[charts/metabase]Allow additional ingress paths to be configured for the ingress #125

Closed MuriukiPM closed 1 month ago

MuriukiPM commented 1 month ago

Allow additional ingress paths to be configured for the ingress by utilising a extraPaths config e.g when one needs to set up a path for ssl-redirect

MuriukiPM commented 1 month ago

@pmint93

pmint93 commented 1 month ago

@MuriukiPM I don't think you'll need extra path and another service to do ssl redirection. It can be done directly via ingress annotation like so:

ingress:
  enabled: true
  # ...
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/force-ssl-redirect: "true" # <- here
MuriukiPM commented 1 month ago

@MuriukiPM I don't think you'll need extra path and another service to do ssl redirection. It can be done directly via ingress annotation like so:

ingress:
  enabled: true
  # ...
  annotations:
    kubernetes.io/ingress.class: nginx
    nginx.ingress.kubernetes.io/force-ssl-redirect: "true" # <- here

I'm familiar with that setting and if I am not wrong it depends on the use case. For example with AWS ALBs using an aws-load-balancer-controller, this won't work unless you use the annotations provided by the controller and add the ssl-redirect path. This PR allows for such a requirement.

Additionally, the example I gave probably is not the only use case for extraPaths, though at the moment I cannot think of any other.

pmint93 commented 1 month ago

@MuriukiPM it seem you misunderstanding aws-loadbalancer-controller document for version 2.2 which use serviceName: ssl-redirect as an example, so that you think it need extra path and a service named ssl-redirect to do redirection. The fact is: you don't

You can see less-misleading document in version 2.3 and later (note that the annotation value also changed - much simpler)

In short, here's the way to do ssl redirection with aws-load-balancer-controller

ingress:
  ...
  path: /*
  pathType: ImplementationSpecific
  annotations:
    kubernetes.io/ingress.class: alb
    alb.ingress.kubernetes.io/certificate-arn: arn:aws:acm:us-west-2:xxxx:certificate/xxxxxx
    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80}, {"HTTPS":443}]'
    alb.ingress.kubernetes.io/actions.ssl-redirect: '{"Type": "redirect", "RedirectConfig": { "Protocol": "HTTPS", "Port": "443", "StatusCode": "HTTP_301"}}'
ingress:
  ...
  path: /*
  pathType: ImplementationSpecific
  annotations:
    kubernetes.io/ingress.class: alb
    alb.ingress.kubernetes.io/certificate-arn: arn:aws:acm:us-west-2:xxxx:certificate/xxxxxx
    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80}, {"HTTPS":443}]'
    alb.ingress.kubernetes.io/ssl-redirect: '443'

you can test it on your cluster to see if i'm correct

MuriukiPM commented 1 month ago

@MuriukiPM it seem you misunderstanding aws-loadbalancer-controller document for version 2.2 which use serviceName: ssl-redirect as an example, so that you think it need extra path and a service named ssl-redirect to do redirection. The fact is: you don't

You can see less-misleading document in version 2.3 and later (note that the annotation value also changed - much simpler)

In short, here's the way to do ssl redirection with aws-load-balancer-controller

  • v2.2 and prior:
ingress:
  ...
  path: /*
  pathType: ImplementationSpecific
  annotations:
    kubernetes.io/ingress.class: alb
    alb.ingress.kubernetes.io/certificate-arn: arn:aws:acm:us-west-2:xxxx:certificate/xxxxxx
    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80}, {"HTTPS":443}]'
    alb.ingress.kubernetes.io/actions.ssl-redirect: '{"Type": "redirect", "RedirectConfig": { "Protocol": "HTTPS", "Port": "443", "StatusCode": "HTTP_301"}}'
  • v2.3 and later
ingress:
  ...
  path: /*
  pathType: ImplementationSpecific
  annotations:
    kubernetes.io/ingress.class: alb
    alb.ingress.kubernetes.io/certificate-arn: arn:aws:acm:us-west-2:xxxx:certificate/xxxxxx
    alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80}, {"HTTPS":443}]'
    alb.ingress.kubernetes.io/ssl-redirect: '443'

you can test it on your cluster to see if i'm correct

Nice, that works! It seems I was still using the old configuration for ssl redirect, thanks for pointing out the new config. I will close the PR since this is not necessary for this use case.