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

[nginx] TCP configmap should allow listen proxy_protocol per service #659

Closed
donaldguy opened this issue Apr 27, 2017 · 3 comments · Fixed by #930
Closed

[nginx] TCP configmap should allow listen proxy_protocol per service #659

donaldguy opened this issue Apr 27, 2017 · 3 comments · Fixed by #930

Comments

@donaldguy
Copy link

We run a (special cased, locked down) ssh server for some of our production sites. It is is our desire for there to be hosts with :443 TLS terminated and passing into an upstream HTTP service and another port for the same hostname to pass to a (the same for all hostnames—ssh has no vhost concept) sshd in another pod

We run in AWS.

Previously we have achieved this by using a setup much like https://github.com/kubernetes/ingress/blob/b01dc68e30d935bc7845e40e85bc6b99ddb320a0/examples/aws/nginx/nginx-ingress-controller.yaml
and then additionally manually opening SG rules and a listener to a NodePort service for the sshd pod

This works most of the time. But occasionally we find suddenly that the manually added rules are reverted

Today, I tried doing as https://github.com/kubernetes/ingress/tree/b01dc68e30d935bc7845e40e85bc6b99ddb320a0/examples/tcp/nginx

and also adding the port mappings through the type LoadBalancer service

This worked for one other raw TCP service we run in the cluster; for SSH it just gets us Bad protocol version identification 'PROXY TCP4 $SOURCE_IP 172.21.74.176 57359 $SSH_PORT' from 100.96.5.223 port 50992 in our sshd logs before the sshd drops the connection.

As reported in kubernetes/kubernetes#42616 there does not appear to be a way to prevent the Service type LoadBalancer from enabling AWS proxy protocol on all ports. I could manually remove, but I suspect that like my manual listeners might be reverted by the controller-manager at a later time.

Additionally as reported in kubernetes-retired/contrib#2258 there isn't currently a way to add proxy_protocol to the listen in server blocks inside the stream; this does seem to be supported https://nginx.org/en/docs/stream/ngx_stream_core_module.html#listen so I believe this should be a change to https://github.com/kubernetes/ingress/blob/b01dc68e30d935bc7845e40e85bc6b99ddb320a0/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl#L505 and the tcp configmap parser

My ideal would actually also be for --publish-service to handle the additional port mappings, but I can live without that.

@donaldguy donaldguy changed the title [nginx] TCP configmap should allow stream proxy_protocol per service [nginx] TCP configmap should allow listen proxy_protocol per service Apr 27, 2017
@aledbf aledbf added this to TODO in nginx 0.9-beta.6 Apr 28, 2017
@donaldguy
Copy link
Author

It might be good to include $proxy_protocol_addr and $proxy_protocol_port in log messages for services with proxy protocol enabled

Because some TCP services (like ssh) create long lived connections, it would also be nice to have the option to have them logged access_log style at socket open time rather than close time. I haven't figured out a way to do this, but I imagine folks here are probably better at nginx config than I am (and/or prometheus metrics about active connections would be neat; idk if the vts stuff can support stream at all)

@aledbf aledbf removed this from TODO in nginx 0.9-beta.6 May 25, 2017
@aledbf aledbf added this to TODO in nginx 0.9-beta.8 May 25, 2017
@donaldguy
Copy link
Author

I notice this didn't make the jump to the 0.9-beta.9 board; is that a wontfix or just incidental?

@aledbf
Copy link
Member

aledbf commented Jun 16, 2017

just incidental?

Yes (moving issues a manual task)

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

Successfully merging a pull request may close this issue.

2 participants