-
Notifications
You must be signed in to change notification settings - Fork 28
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
[httpd] Use absolute url with schema in redirect rule #396
[httpd] Use absolute url with schema in redirect rule #396
Conversation
/cherry-pick 18.0-fr1 |
@olliewalsh: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/test horizon-operator-build-deploy-kuttl |
@@ -65,17 +65,17 @@ def get_pod_ip(): | |||
import socket | |||
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) | |||
try: | |||
s.connect(("{{ .horizonEndpointUrl }}", 80)) | |||
s.connect(("{{ .horizonEndpointHost }}", 80)) |
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.
in case of tls, is this still port 80?
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.
IIUC this doesn't matter, just opens a socket to $something to determine the pod IP.
I guess if there is a firewall blocking port 80 on the ingress controller it would block this though so might be best to use the correct port
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.
Yeah, correct. I just needed a way to determine the pods IP address so that we could add it to ALLOWED_HOSTS
. The port should be ok, because port 80 is always open it just redirects to 443, and an open port of any kind is sufficient to pick up the IP address.
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.
However in this specific case port 80 is firewalled (not specified where/how though) so it may be safest to use the correct port for the endpoint
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.
Done in the latest patch. Maybe in a follow-up should look into replacing this with downward-api volume or env var passing status.podIP instead
Use the endpoint in the redirect rule to ensure it references the correct base url and schema which can be different to the current schema when TLS is terminated at the route Added horizonEndpoint template param with the full url, renamed the incorrectly named horizonEndpointUrl to horizonEndpointHost. Jira: OSPRH-12005
6ae59e9
to
c0f6c6d
Compare
/retest |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bshephar, olliewalsh, stuggi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
e006d13
into
openstack-k8s-operators:main
@olliewalsh: new pull request created: #398 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Use the endpoint in the redirect rule to ensure it references the correct base url and schema which can be different to the current schema when TLS is terminated at the route
Added horizonEndpoint template param with the full url, renamed the incorrectly named horizonEndpointUrl to horizonEndpointHost.
Jira: OSPRH-12005