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

Ability to pass extra command line options to configurable-http-proxy #1302

Closed
ivan-gomes opened this issue Jun 3, 2019 · 9 comments · Fixed by #1813
Closed

Ability to pass extra command line options to configurable-http-proxy #1302

ivan-gomes opened this issue Jun 3, 2019 · 9 comments · Fixed by #1813

Comments

@ivan-gomes
Copy link

ivan-gomes commented Jun 3, 2019

Description

The z2jh-k8s deployment disables the managed configurable-http-proxy (CHP) in favor of running it in its own pod. This is perfectly fine, but we lose the ability to configure the proxy's command line options since using c.ConfigurableHTTPProxy.command in hub.extraConfig no longer has any effect. Digging deeper, https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/master/jupyterhub/templates/proxy/deployment.yaml builds a set of command line options based on Helm chart values, but it doesn't expose the breadth of options available in https://github.com/jupyterhub/configurable-http-proxy#command-line-options.

Proposal

I propose adding a proxy.extraConfig field in the Helm chart that appends to the CHP command line options that are derived. This would mirror hub.extraConfig conceptually, but could be specified as a list of command line options.

Use Cases:

  • Disabling TLS 1.0/1.1 - command line option exists in configurable-http-proxy but no way to set it when using the Helm chart.
  • Restricting allowed ciphers - command line option exists in configurable-http-proxy but no way to set it when using the Helm chart.
  • Adding Strict-Transport-Security response header - command line option to add custom headers implemented in PR Command line option for custom headers configurable-http-proxy#206 but even when/if it is merged there won't be a way to set it when using the Helm chart.
@manics
Copy link
Member

manics commented Jun 4, 2019

command:
- configurable-http-proxy
- --ip=0.0.0.0
- --api-ip=0.0.0.0
- --api-port=8001
- --default-target=http://$(HUB_SERVICE_HOST):$(HUB_SERVICE_PORT)
- --error-target=http://$(HUB_SERVICE_HOST):$(HUB_SERVICE_PORT)/hub/error
{{- if $manualHTTPS }}
- --port=8443
- --redirect-port=8000
- --ssl-key=/etc/chp/tls/tls.key
- --ssl-cert=/etc/chp/tls/tls.crt
{{- else if $manualHTTPSwithsecret }}
- --port=8443
- --redirect-port=8000
- --ssl-key=/etc/chp/tls/{{ .Values.proxy.https.secret.key }}
- --ssl-cert=/etc/chp/tls/{{ .Values.proxy.https.secret.crt }}
{{- else }}
- --port=8000
{{- end }}
{{- if .Values.debug.enabled }}
- --log-level=debug
{{- end }}

Do you think it's enough to append to those arguments, or should it be possible to completely override them?

@ivan-gomes
Copy link
Author

Just tested configurable-http-proxy by setting the the same command line option twice. The second option overrides the first, so implementing it as an append would allow overriding as well.

@ivan-gomes
Copy link
Author

@manics any chance you were interested in working on this?

@manics
Copy link
Member

manics commented May 17, 2020

I'm not planning to work on this. If you've already done some work feel free to open a PR!

@cslocum
Copy link

cslocum commented Oct 30, 2020

This it great! I was looking for ways to accomplish port forwarding, and found this. Thanks for implementing it!

I am however running into a problem, and I'm trying to figure out what I'm doing wrong. This is the relevant part of my configuration:

jupyterhub:
    proxy:
        secretToken: [REDACTED]
        https:
            enabled: true
            type: offload
        service:
            annotations:
                service.beta.kubernetes.io/aws-load-balancer-ssl-cert: [REDACTED]
                # The protocol to use on the backend, we use TCP since we're using websockets
                service.beta.kubernetes.io/aws-load-balancer-backend-protocol: tcp
                # Which ports should use SSL
                service.beta.kubernetes.io/aws-load-balancer-ssl-ports: https
                service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout: "3600"
        chp:
            extraCommandLineFlags:
            - --redirect-port 80
            - --redirect-to 443

A kubectl describe on the proxy pod shows these configurable-http-proxy commands, so I can see that they are at least being picked up:

  Command:
      configurable-http-proxy
      --ip=::
      --api-ip=::
      --api-port=8001
      --default-target=http://hub:$(HUB_SERVICE_PORT)
      --error-target=http://hub:$(HUB_SERVICE_PORT)/hub/error
      --port=8000
      --redirect-port 80
      --redirect-to 443

The proxy logs show this "error: unknown option '--redirect-port 80'". I believe it should be a valid flag, according to https://github.com/jupyterhub/configurable-http-proxy#command-line-options.

Any help would be appreciated. Thanks!

@consideRatio
Copy link
Member

@cslocum it believes --redirect-port 80 as a single string, is the flag when you write it like this, so just replace the space with a = and it will be fine.

        chp:
            extraCommandLineFlags:
            - --redirect-port=80
            - --redirect-to=443
        # this would also work I think.
        chp:
            extraCommandLineFlags:
            - --redirect-port
            - "80"
            - --redirect-to
            - "443"

@cslocum
Copy link

cslocum commented Oct 30, 2020

@consideRatio thank you for catching that! I modified it to this to make it work:

    chp:
        extraCommandLineFlags:
            - --redirect-port=80
            - --redirect-to=443

But now that I think about it, this approach might not work for me. I had been using https.type = manual, but now I'm offloading it to an AWS load balancer and using an ACM cert. That means that there is no cert on the proxy (I think), resulting in "[ConfigProxy] error: HTTPS redirection specified but certificates not provided"

The reason I pursued this approach in the first place is because it seems that when offloading, http requests are not redirected to https, which means users will land on an unencrypted page. This wasn't the case when http.type was manual.

@consideRatio
Copy link
Member

I tried solving automatic http->https redirects for proxy.https.type=offload in #1811, but it seems like we are forced to do it not based on port, but based on headers. Does CHP (which wraps node-proxy) support that? I'm not sure.

@consideRatio
Copy link
Member

@cslocum let's not continue the discussion in this issue as the issue is unrelated to the discussion.

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

Successfully merging a pull request may close this issue.

4 participants