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

Refactor: reference ports by name instead of repeating the number #1645

Merged
merged 3 commits into from
Apr 25, 2020

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Apr 24, 2020

  • Port definitions protocol: TCP is no longer explicitly set
  • Ports are now referenced by name from services and network policy ingress
  • The deploy/hub and deploy/proxy pods got some ports renamed:
    • hub -> http
    • proxy-public -> http
    • proxy-https -> https
  • I removed legacy ports allowed by the deploy/proxy pod's network policy for kube-lego and nginx /health. They could not be accessed anyhow as they were not exposed by the pod.

This should not influence anything.

port: 8080
# nginx /healthz
- protocol: TCP
port: 10254
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

@consideRatio consideRatio Apr 24, 2020

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.

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

Successfully merging this pull request may close these issues.

1 participant