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

HAProxy Router: Add option to use PROXY protocol #12271

Conversation

Miciah
Copy link
Contributor

@Miciah Miciah commented Dec 15, 2016

Configure HAProxy to expect incoming connections on port 443 to use the PROXY protocol if the ROUTER_USE_PROXY_PROTOCOL environment variable is set to "true" or "TRUE".

This commit fixes bug 1385421.

https://bugzilla.redhat.com/show_bug.cgi?id=1385421


openshift-bot, please [test]!

@Miciah
Copy link
Contributor Author

Miciah commented Dec 15, 2016

Does this count as a flake?

+ vagrant ssh -c 'sudo docker version'
Client:
 Version:         1.12.2-rc2
 API version:     1.24
 Package version: docker-common-1.12.2-5.el7.x86_64
 Go version:      go1.6.2
 Git commit:      19dbe35
 Built:           Mon Oct 17 11:44:51 2016
 OS/Arch:         linux/amd64
Cannot connect to the Docker daemon. Is the docker daemon running on this host?

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_integration/9479/console

Flake #11887.
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_conformance/9593/consoleFull

@Miciah
Copy link
Contributor Author

Miciah commented Dec 15, 2016

openshift-bot, could you please [test] again?

@Miciah
Copy link
Contributor Author

Miciah commented Dec 15, 2016

@knobunc, we are looking to get this into 3.4 and backport to 3.3 for a customer. Does the change look reasonable?

@Miciah
Copy link
Contributor Author

Miciah commented Dec 15, 2016

Blocked on #12282:

hack/build-base-images.sh took 0 seconds
bash: line 7: hack/build-dind-images.sh: Permission denied
Error while running ssh/sudo command: 
echo "Building base images..."
set -e
pushd /data/src/github.com/openshift/origin
  hack/build-base-images.sh
  hack/build-dind-images.sh
popd
The SSH command responded with a non-zero exit status. Vagrant
assumes that this means the command failed. The output for this command
should be in the log above. Please read the output to determine what
went wrong.
Build step 'Execute shell' marked build as failure

https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_integration/9505/console

Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@knobunc
Copy link
Contributor

knobunc commented Dec 15, 2016

@openshift/networking PTAL

@stevekuznetsov
Copy link
Contributor

re[test]

@Miciah
Copy link
Contributor Author

Miciah commented Dec 15, 2016

Flake #11452:

Error: Failed to synchronize cache for repo 'updates'
The command '/bin/sh -c dnf -y update && dnf -y install docker glibc-langpack-en iptables openssh-clients openssh-server && dnf -y remove docker-v1.10-migrator glibc-all-langpacks selinux-policy-minimum && dnf clean all' returned a non-zero code: 1

Copy link
Contributor

@ramr ramr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add the same for the http bind on port 80.

@ramr
Copy link
Contributor

ramr commented Dec 15, 2016

@Miciah change for the https part looks good but could you please add the same for the http part (bind on port 80). Thx

@Miciah
Copy link
Contributor Author

Miciah commented Dec 15, 2016

@ramr, same configuration variable or an independent one?

@ramr
Copy link
Contributor

ramr commented Dec 15, 2016

@Miciah same env variable ROUTER_USE_PROXY_PROTOCOL would be good. Thx

Configure HAProxy to expect incoming connections to use the PROXY protocol
if the ROUTER_USE_PROXY_PROTOCOL environment variable is set to "true" or
"TRUE".

This commit fixes bug 1385421.

https://bugzilla.redhat.com/show_bug.cgi?id=1385421
@Miciah Miciah force-pushed the bz1385421-template-router-support-proxy-protocol branch from 24c5ba0 to 9db4fe7 Compare December 16, 2016 08:08
@Miciah
Copy link
Contributor Author

Miciah commented Dec 16, 2016

Updated.

Note that using the PROXY protocol for unencrypted HTTP with an Amazon ELB requires changing the listener from HTTP to TCP. Testing the change took a while because it took me way too long to figure this out. 😀

@Miciah
Copy link
Contributor Author

Miciah commented Dec 17, 2016

I reliably see these tests fail on this commit in a devenv with Go 1.6 and reliably pass in a devenv (even the same devenv) updated to Go 1.7. I also see the parent commit fail the same tests on a devenv with Go 1.6 and pass in a devenv with Go 1.7. Is this a situation for which I should open a flake, or is there an existing issue to which I can refer?

@Miciah
Copy link
Contributor Author

Miciah commented Dec 17, 2016

Scratch that (or change the word "reliably" to "usually")—I just saw the test fail in a devenv with Go 1.7 too.

@Miciah
Copy link
Contributor Author

Miciah commented Dec 19, 2016

I'm pretty sure this is flake #8502.

@Miciah
Copy link
Contributor Author

Miciah commented Dec 20, 2016

openshift-bot, please [test] again!

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 9db4fe7

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12550/) (Base Commit: c1b78d5)

@Miciah
Copy link
Contributor Author

Miciah commented Dec 20, 2016

@knobunc, I think Ram is out, so would you mind re-reviewing the PR? Is there anyone else I should ping?

@knobunc
Copy link
Contributor

knobunc commented Dec 20, 2016

@rajatchopra @ramr @JacobTanenbaum PTAL

Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New changes seem perfectly reasonable

Copy link
Contributor

@ramr ramr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@knobunc
Copy link
Contributor

knobunc commented Dec 21, 2016

[merge]

@knobunc
Copy link
Contributor

knobunc commented Dec 21, 2016

@Miciah can you add the environment variable to the docs in architecture/core_concepts/routes.adoc please and submit a PR.

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 9db4fe7

@openshift-bot
Copy link
Contributor

openshift-bot commented Dec 21, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12565/) (Base Commit: 2dd7247) (Image: devenv-rhel7_5583)

@openshift-bot openshift-bot merged commit 05e4f0e into openshift:master Dec 21, 2016
@andrewklau
Copy link
Contributor

If I use this commit as a haproxy config overwrite will this be valid for 1.4 or is there an image dependency? My test environment won't route traffic with this enabled

@Miciah
Copy link
Contributor Author

Miciah commented Jan 22, 2017

It's purely an HAProxy configuration change. You can inject the template change into a container running an old image, and it should work fine.

What is your test environment? I tested HAProxy with AWS ELB configured to use PROXY protocol as follows:

lb='infra-lb'
instances=( 'i-0123456789abcdef0' )
secgroups=( 'sg-01234567' )
subnets=( 'subnet-01234567' )

# Create the ELB with the appropriate listeners, security groups, and subnets.
# Note that the listeners must all be configured to use the TCP protocol, not
# the HTTP protocol:
aws elb create-load-balancer --load-balancer-name "$lb" \
 --listeners \
  'Protocol=TCP,LoadBalancerPort=80,InstanceProtocol=TCP,InstancePort=80' \
  'Protocol=TCP,LoadBalancerPort=443,InstanceProtocol=TCP,InstancePort=443' \
  'Protocol=TCP,LoadBalancerPort=5000,InstanceProtocol=TCP,InstancePort=5000' \
 --security-groups $secgroups \
 --subnets $subnets

# Expected output:
# {
#     "DNSName": "infra-lb-2006263232.us-east-1.elb.amazonaws.com"
# }

# Register your router host instance or instances with the ELB:
aws elb register-instances-with-load-balancer --load-balancer-name "$lb" \
 --instances $instances

# Expected output:
# {
#     "Instances": [
#         {
#             "InstanceId": "i-0123456789abcdef0"
#         }
#     ]
# }

# Configure the ELB's health check:
aws elb configure-health-check --load-balancer-name "$lb" \
 --health-check 'Target=HTTP:1936/healthz,Interval=30,UnhealthyThreshold=2,HealthyThreshold=2,Timeout=5'

# Expected output:
# {
#     "HealthCheck": {
#         "HealthyThreshold": 2,
#         "Interval": 30,
#         "Target": "HTTP:1936/healthz",
#         "Timeout": 5,
#         "UnhealthyThreshold": 2
#     }
# }

# Finally, create a load-balancer policy with the ProxyProtocol attribute
# enabled, and configure it on the ELB's TCP ports 80 and 443:

aws elb create-load-balancer-policy --load-balancer-name "$lb" \
 --policy-name "${lb}-ProxyProtocol-policy" \
 --policy-type-name 'ProxyProtocolPolicyType' \
 --policy-attributes 'AttributeName=ProxyProtocol,AttributeValue=true'

for port in 80 443
do
  aws elb set-load-balancer-policies-for-backend-server \
   --load-balancer-name "$lb" \
   --instance-port "$port" \
   --policy-names "${lb}-ProxyProtocol-policy"
done

aws elb describe-load-balancers --load-balancer-name "$lb" |
  jq '.LoadBalancerDescriptions| [.[]|.ListenerDescriptions]'

# Expected output:
# [
#   [
#     {
#       "Listener": {
#         "InstancePort": 80,
#         "LoadBalancerPort": 80,
#         "Protocol": "TCP",
#         "InstanceProtocol": "TCP"
#       },
#       "PolicyNames": ["infra-lb-ProxyProtocol-policy"]
#     },
#     {
#       "Listener": {
#         "InstancePort": 443,
#         "LoadBalancerPort": 443,
#         "Protocol": "TCP",
#         "InstanceProtocol": "TCP"
#       },
#       "PolicyNames": ["infra-lb-ProxyProtocol-policy"]
#     },
#     {
#       "Listener": {
#         "InstancePort": 5000,
#         "LoadBalancerPort": 5000,
#         "Protocol": "TCP",
#         "InstanceProtocol": "TCP"
#       },
#       "PolicyNames": []
#     }
#   ]
# ]

# Alternatively, if you already have an ELB configured, but it is not configured
# to use PROXY protocol, you will need to change the existing listener for TCP
# port 80 to use the TCP protocol instead of HTTP (TCP port 443 should already
# be using the TCP protocol):

aws elb delete-load-balancer-listeners --load-balancer-name "$lb" \
 --load-balancer-ports 80
aws elb create-load-balancer-listeners --load-balancer-name "$lb" \
 --listeners 'Protocol=TCP,LoadBalancerPort=80,InstanceProtocol=TCP,InstancePort=80'
aws elb describe-load-balancers --load-balancer-name "$lb" |
 jq '[.LoadBalancerDescriptions[]|.ListenerDescriptions]'

# Expected output:
# [
#   [
#     {
#       "Listener": {
#         "InstancePort": 443,
#         "LoadBalancerPort": 443,
#         "Protocol": "TCP",
#         "InstanceProtocol": "TCP"
#       },
#       "PolicyNames": []
#     },
#     {
#       "Listener": {
#         "InstancePort": 5000,
#         "LoadBalancerPort": 5000,
#         "Protocol": "TCP",
#         "InstanceProtocol": "TCP"
#       },
#       "PolicyNames": []
#     },
#     {
#       "Listener": {
#         "InstancePort": 80,
#         "LoadBalancerPort": 80,
#         "Protocol": "TCP",
#         "InstanceProtocol": "TCP"
#       },
#       "PolicyNames": []
#     }
#   ]
# ]

# Then create a load-balancer policy and add it to the ELB as described above.

# Now configure the router to use PROXY protocol:

oc env dc/router ROUTER_USE_PROXY_PROTOCOL=true

@andrewklau
Copy link
Contributor

Thanks for the clarification.

I was doing something similar in my test environment however, for some reason it didn't like the fact I enabled ROUTER_USE_PROXY_PROTOCOL before enabling the ProxyPolicy on the ELB.

@knobunc
Copy link
Contributor

knobunc commented Jan 23, 2017

Unfortunately, until both ends have it set up correctly, it won't work at all.

@pecameron
Copy link
Contributor

@knobunc What is the PR in openshift-docs?

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

Successfully merging this pull request may close these issues.

7 participants