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

It's impossible to disable a port defined in the default config by setting it to null #890

Closed
2 tasks done
marekvospel opened this issue Jul 14, 2023 · 7 comments · Fixed by #898
Closed
2 tasks done
Labels
kind/bug/confirmed a confirmed bug (reproducible).

Comments

@marekvospel
Copy link

Welcome!

  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've searched similar issues on the Traefik community forum and didn't find any.

What version of the Traefik's Helm Chart are you using?

23.1.0

What version of Traefik are you using?

2.10.1

What did you do?

Traefik helm chart creates 4 entrypoints by default, web, websecure, traefik and prometheus. I only need web and websecure, so I've been trying to disable traefik and prometheus ports.

I've found #220, which said it is possible to use null, which had correct behavior, but fails during installation with an error in trying to expose the ports through kubernetes

What did you see instead?

Error: INSTALLATION FAILED: template: traefik/templates/_podtemplate.tpl:63:36: executing "traefik.podTemplate" at <.Values.ports.traefik.port>: nil pointer evaluating interface {}.port

What is your environment & configuration?

ports:
  web:
    port: 80
  websecure:
    port: 443
    http3:
      enabled: true
      advertisedPort: 443
  traefik: null
  prometheus: null

Additional Information

No response

@mloiseleur mloiseleur changed the title It's impossible to disable a port defined in the defualt config by setting it to null It's impossible to disable a port defined in the default config by setting it to null Jul 21, 2023
@mloiseleur
Copy link
Contributor

@marekvospel Thanks for your interest in Traefik and this report. I've made a PR that should give this capacity.

I won't recommend to disable traefik port on a production setup, though, since it means liveness and readiness probe will be directly on web port

@mloiseleur mloiseleur added kind/bug/confirmed a confirmed bug (reproducible). and removed status/0-needs-triage labels Jul 24, 2023
@mloiseleur
Copy link
Contributor

It's now possible in v23.2.0.

@mantoine96
Copy link

Hey @mloiseleur

It seems this is still an issue, at least for disabling web:

template: traefik/templates/_podtemplate.tpl:60:40: executing "traefik.podTemplate" at <.Values.ports.web.port>: nil pointer evaluating interface {}.port

This is because now ports.web.port is taken as the default healthcheck port before we evaluate whether the Traefik endpoint is available: https://github.com/traefik/traefik-helm-chart/pull/891/files#diff-c64349ddaeb8064687b0839b8b93e77fb6733c2dae67b8c0fface1cac0652fccR60-R65

I'm not sure what's the expected way forward here. Up until now we had always had web and websecure disabled as per #220 (using our own endpoints).

This is what we had and was working until v23.2.0:

                ports:
                  traefik:
                    port: 8082
                  admin:
                    port: 8444
                    tls:
                      enabled: true
                  http-internal:
                    port: 8443
                    tls:
                      enabled: true
                    expose: true
                  http-external:
                    port: 8445
                    tls:
                      enabled: true
                    expose: true
                  web: null
                  websecure: null
                  metrics: null

This is now what I need to have:

                ports:
                  traefik:
                    port: 8082
                  admin:
                    port: 8444
                    tls:
                      enabled: true
                  http-internal:
                    port: 8443
                    tls:
                      enabled: true
                    expose: true
                  http-external:
                    port: 8445
                    tls:
                      enabled: true
                    expose: true
                  web: {}
                  websecure: null
                  metrics: null

Let me know if you'd rather I open a new issue!

Thanks again!

@mloiseleur
Copy link
Contributor

mloiseleur commented Jul 28, 2023

Thanks for this report.

When a user wants to disable traefik port, then I guess he should precise on which port the healthcheck should go.
I've made PR #898, which is breaking, but should fix both use case.

@marekvospel @mantoine96 Would you please let me know if I missed something or if you have a better (non breaking) idea ?

@marekvospel
Copy link
Author

Thanks for this report.

When a user wants to disable traefik port, then I guess he should precise on which port the healthcheck should go.
I've made PR #898, which is breaking, but should fix both use case.

@marekvospel @mantoine96 Would you please let me know if I missed something or if you have a better (non breaking) idea ?

I'm in no way familiar with this helm chart enough to give a propper review, but at quick glance it looks good - makes sense and I don't think I would have done it sny other way.

@mloiseleur
Copy link
Contributor

You can see what it generates by pulling the PR and apply helm template locally, like this:

helm template traefik -f myvalues.yaml ./traefik

@mantoine96
Copy link

@mloiseleur Thanks for the PR. From my side it looks good to me. I don't think there's a way to solve this in a not breaking way for users that disable the traefik port. There are already a few things that need to be set in that case (such as configuring the ping to work on the new port) so I don't think it's too bad of a trade off.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug/confirmed a confirmed bug (reproducible).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants