Consul key/value collision by sberryman · Pull Request #47 · autopilotpattern/nginx

sberryman · 2017-04-11T22:00:53Z

Not sure if this is a common use-case but is something I ran into recently. I have several front end load balancers where I want to perform SSL termination. Since I don’t want to run several Consul clusters I ran into an issue where they all use the same keys. I’m also not sure if I have caught all the places where nginx is hard coded but this DOES work in production for me.

…a single consul cluster

sberryman · 2017-04-11T22:04:04Z

bin/acme

        ;;
    watch)
-        /usr/local/bin/consul-template -config /etc/acme/watch.hcl -consul $CONSUL_HOST:8500
+        /usr/local/bin/consul-template -config /etc/acme/watch.hcl -consul-addr $CONSUL_HOST:8500


This was changed as a warning was output to stdout by consul-template

sberryman · 2017-04-11T22:04:24Z

etc/acme/templates/cert.ctmpl

@@ -1 +1,2 @@
-{{if key "nginx/acme/cert"}}{{key "nginx/acme/cert"}}{{end}}
+{{ $service_name := env "SERVICE_NAME" }}


WARNING!!! No default SERVICE_NAME

We should have a default service name nginx.

Right, honestly I'm not sure how to do it and I didn't have time yesterday to look. Just needed it working to show off a few things

This appears fixed with {{ $service_name := or $service_name "nginx" }} below.

sberryman · 2017-04-11T22:04:33Z

@tgross Warning, I've never used golang and not sure how to add a default value for templating. Right now there is no default SERVICE_NAME for the environment variable on acme templates. However, it defaults to nginx in the two bash scripts that were changed.

See the example below:
https://github.com/sberryman/autopilotpattern-nginx/blob/91b5d21f689c1154bfc4bbe2a7021a6dd3c5cbaa/etc/acme/templates/privkey.ctmpl#L1

I haven't had time to look up adding defaults.

tgross · 2017-04-12T12:35:41Z

I've never used golang and not sure how to add a default value for templating.

Unfortunately default has to be added by every implementation of the rendered. ContainerPilot has it but consul-template does not. You want something along the lines of: {{ $myvar := or $myothervar "value" }}

sberryman · 2017-04-12T16:41:33Z

@tgross added a default service_name to the templates and tested on joyent. I've noticed curl showing an error when SSL is enabled though.

No matter what I end up using for the hostname I always get an error resolving the host.
2017/04/12 16:36:29 curl: (6) Could not resolve host: XYZ" XYZ being the hostname I picked. SSL works but that error shows up in the logs every 10 seconds.

Update:
I believe this is the command causing the error

/usr/bin/curl --insecure --fail --silent --show-error --output /dev/null --header \"HOST: {{ .ACME_DOMAIN }}\" https://localhost/nginx-health

Update 2:
The {{ .ACME_DOMAIN }} resolves to 127.0.0.1 inside the container.

tgross · 2017-04-12T17:03:17Z

I believe this is the command causing the error

/usr/bin/curl --insecure --fail --silent \
    --show-error --output /dev/null \
    --header \"HOST: {{ .ACME_DOMAIN }}\" \
    https://localhost/nginx-health

If that were the case I don't think we'd be seeing an attempt to resolve that hostname, right? Which hostname is the error Could not resolve host: XYZ" for: ACME_DOMAIN or CONSUL?

sberryman · 2017-04-12T17:09:06Z

@tgross - Here is what I'm doing:

ACME_DOMAIN=cms.dev.famishednow.net

Deployed my branch to joyent along with a backing service and consul
docker exec -it front_cms_nginx_1 /bin/bash
Output below

# export | grep ACME_DOMAIN
declare -x ACME_DOMAIN="cms.dev.famishednow.net"

# ping $ACME_DOMAIN
PING cms.dev.famishednow.net (127.0.0.1): 56 data bytes
64 bytes from 127.0.0.1: icmp_seq=0 ttl=255 time=0.054 ms
64 bytes from 127.0.0.1: icmp_seq=1 ttl=255 time=0.052 ms
64 bytes from 127.0.0.1: icmp_seq=2 ttl=255 time=0.060 ms

# /usr/bin/curl --insecure --fail --silent --show-error --output /dev/null --header \"HOST: $ACME_DOMAIN\" https://localhost/nginx-health
curl: (6) Could not resolve host: cms.dev.famishednow.net"
Active connections: 1 
server accepts handled requests
 1989 1989 2065 
Reading: 0 Writing: 1 Waiting: 0

Obviously the request is working and consul shows the test as passing. However I see that damn curl error in the logs every 10 seconds.

Update: Why are you even using the HOST header? The request passes without it, I'm guessing you are assuming the NGINX config is using server_name in the config?

tgross · 2017-04-13T13:01:43Z

PING cms.dev.famishednow.net (127.0.0.1)

This is the bit I'm confused by. Why does the name you're using resolve to localhost? That's not going to work with Let's Encrypt anyways, right?

Update: Why are you even using the HOST header? The request passes without it, I'm guessing you are assuming the NGINX config is using server_name in the config?

Yes, which is a pretty safe assumption under TLS. I'm not sure why we're including --insecure on this request though.

sberryman · 2017-04-13T18:15:52Z

I was confused by it until I realized you are injecting $ACME_DOMAIN into /etc/hosts.
https://github.com/autopilotpattern/nginx/blob/master/bin/health-check#L8-L14

LetsEncrypt runs just fine, in fact I was blow away how well and quickly it worked. 100x easier than setting up certificates the old fashion manual way.

Right now the only issue I see is the error showing up in the logs. It doesn't cause any issues related to functionality. Even the consul health check shows as passing so it isn't that big of a deal.

misterbisson · 2017-06-07T02:45:07Z

etc/acme/templates/cert.ctmpl

@@ -1 +1,3 @@
-{{if key "nginx/acme/cert"}}{{key "nginx/acme/cert"}}{{end}}
+{{ $service_name := env "SERVICE_NAME" }}
+{{ $service_name := or $service_name "nginx" }}


I'm not saying the following with certainty, but there's a chance we'll also (or alternatively) want to set SERVICE_NAME to nginx as a default value in the Dockerfile. That would result in reliably getting a env var, even if the user doesn't supply one.

If I remember correctly, that syntax would look like:

ENV SERVICE_NAME =${SERVICE_NAME:-nginx}

misterbisson · 2017-06-07T02:45:39Z

bin/acme

 CONSUL_HOST=${CONSUL_HOST:-$CONSUL_HOST_DEFAULT}
 CONSUL_ROOT="http://${CONSUL_HOST}:8500/v1"
-CONSUL_KEY_ROOT="${CONSUL_ROOT}/kv/nginx"
+CONSUL_KEY_ROOT="${CONSUL_ROOT}/kv/${SERVICE_NAME}"


This is another place where we'd need a default value.

misterbisson · 2017-06-07T02:47:19Z

@sberryman I think this looks good. There's one place where SERVICE_NAME is being used without a default, but I've also got a question about a different or additional approach to setting that default in another comment.

sberryman · 2017-06-08T18:11:47Z

@misterbisson I have moved on past nginx and started using traefik for my project instead. I am more than happy to up the PR though. Still using the autopilot pattern for pretty much everything though which has been working out great!

misterbisson · 2017-06-08T23:23:19Z

@sberryman I think this was a useful change, so if you're up for it I'd love to update this PR. Also, I'd love to hear more about how you're using Traefik.

…ul-servicename # Conflicts: # README.md

sberryman · 2017-06-16T00:50:26Z

@misterbisson lets see what you think of those quick changes. We can move the Traefik conversation to another spot if you would like. I can go over what I'm using containerpilot for and the rest of my stack. All of which is hosted on Triton of course.

sberryman added 4 commits April 10, 2017 19:34

Use SERVICE_NAME to make it possible to run several nginx servers on …

c7572ee

…a single consul cluster

Need service name in 1 more location!

37f720e

[WARN] -consul has been renamed to -consul-addr

a2e6afb

Turns out the service name is hard coded everywhere!

91b5d21

sberryman commented Apr 11, 2017

View reviewed changes

Add some documentation for SERVICE_NAME

7029bef

Add default service_name of "nginx" to templates

55fd2e1

misterbisson reviewed Jun 7, 2017

View reviewed changes

Merge remote-tracking branch 'autopilotpattern/master' into acme-cons…

b2e346a

…ul-servicename # Conflicts: # README.md

		@@ -1 +1,2 @@
		{{if key "nginx/acme/cert"}}{{key "nginx/acme/cert"}}{{end}}
		{{ $service_name := env "SERVICE_NAME" }}

Conversation

sberryman commented Apr 11, 2017

Uh oh!

sberryman Apr 11, 2017

Choose a reason for hiding this comment

Uh oh!

sberryman Apr 11, 2017

Choose a reason for hiding this comment

Uh oh!

tgross Apr 12, 2017

Choose a reason for hiding this comment

Uh oh!

sberryman Apr 12, 2017

Choose a reason for hiding this comment

Uh oh!

misterbisson Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

sberryman commented Apr 11, 2017

Uh oh!

tgross commented Apr 12, 2017

Uh oh!

sberryman commented Apr 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross commented Apr 12, 2017

Uh oh!

sberryman commented Apr 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tgross commented Apr 13, 2017

Uh oh!

sberryman commented Apr 13, 2017

Uh oh!

misterbisson Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

misterbisson Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

misterbisson commented Jun 7, 2017

Uh oh!

sberryman commented Jun 8, 2017

Uh oh!

misterbisson commented Jun 8, 2017

Uh oh!

sberryman commented Jun 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sberryman commented Apr 12, 2017 •

edited

Loading

sberryman commented Apr 12, 2017 •

edited

Loading