-
Notifications
You must be signed in to change notification settings - Fork 797
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
Refactor: reference ports by name instead of repeating the number #1645
Conversation
port: 8080 | ||
# nginx /healthz | ||
- protocol: TCP | ||
port: 10254 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to allow ingress to the deploy/proxy's pod, but that pod doesn't expose these ports anyhow. This is not relevant any more.
{{- else }} | ||
targetPort: 8000 | ||
{{- end }} | ||
targetPort: http |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No matter if this goes to the deploy/autohttps or deploy/proxy, both have a port named http even if it is exposed on different port numbers.
{{- else }} | ||
targetPort: 443 | ||
targetPort: https |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No matter if this goes to the deploy/autohttps or deploy/proxy, it should always go to the https
port unless the https type is "offload", then apparently it should still go through the 443 entrypoint of this service but end up at the http port as it actually is http traffic.
I think the main reason for this, has been to trick anyone using the variables PROXY_PUBLIC_SERVICE_PORT to go to the right location or similar, because that port is the first port exposed by the service. But, anyhow, this is an offtopic discussion with regards to this PR.
dc3ad68
to
ef29792
Compare
protocol: TCP
is no longer explicitly setThis should not influence anything.