CausticLab / rgon-proxy

the base image of the rancher nginx-letsencrypt proxy
5 stars 1 forks source link

Mixed-service stacks duplicate upstream and server_name directives #48

Closed emcniece closed 6 years ago

emcniece commented 6 years ago

Just discovered that since we iterate services (and not domains specifically) it is possible to create duplicate (and conflicting) upstream and server directives.

Consider this:

Rancher mixed-service stack

Each of these services is tagged with rgon.domain = mysite.com, but looking at nginx.tmpl#L233 we are iterating services, which will run 3 times for this example: once for the Nginx services, and once for the dummy instance. This creates something like:

upstream mysite.com{ server 1.1.1.100:82 }
server{ server_name: mysite.com }

upstream mysite.com{ server 1.1.1.102:81 }
server{ server_name: mysite.com }

upstream mysite.com{ server 1.1.1.103 down }
server{ server_name: mysite.com }

I think that it may be possible to have multiple services referencing a single domain name. It may also be possible to have multiple stacks (maybe even with multiple services) referencing a domain name.

Should we abstract our domain iterator in such a way that makes it possible to multiple services across multiple stacks be included in a single upstream and server directive? If a user is tagging a container with a domain name, perhaps it would be best to allow them to do so successfully without producing an nginx.conf that errors immediately.

The solution may be simple - take the following block:

## SERVICES --------------
{{range $opt, $services := services | groupByLabel "rgon.domain" -}}
{{- $domains := (split (replace $opt " " "" -1) ",") -}}
{{- $domain := index $domains 0 -}}
{{range $serv := $services -}}
{{- $redirect_target := ($serv.Labels.GetValue "rgon.redirect" "http") -}}
{{- $has_certs := ( and (exists (printf "/etc/nginx/certs/%s/fullchain" $domain)) (exists (printf "/etc/nginx/certs/%s/privkey" $domain)) ) -}}
{{- $should_redirect := (and (eq $redirect_target "https") ($has_certs)) -}}

upstream {{$domain}} {
  {{- template "upstream" $serv}}
}

server {
  {{- template "httpServer" (dict "domains" $domains "should_redirect" $should_redirect) }}
}

The $services would be passed directly into the upstream template, and $redirect_target would need to either parse all labels in all services or just take the first one it finds (assuming that each service under this domain would be a similar behaviour):

## SERVICES --------------
{{range $opt, $services := services | groupByLabel "rgon.domain" -}}
{{- $domains := (split (replace $opt " " "" -1) ",") -}}
{{- $domain := index $domains 0 -}}
{{range $serv := index $services 0 -}}
{{- $redirect_target := ($serv.Labels.GetValue "rgon.redirect" "http") -}}
{{- $has_certs := ( and (exists (printf "/etc/nginx/certs/%s/fullchain" $domain)) (exists (printf "/etc/nginx/certs/%s/privkey" $domain)) ) -}}
{{- $should_redirect := (and (eq $redirect_target "https") ($has_certs)) -}}

upstream {{$domain}} {
  ## Upstream template will need modification
  {{- template "upstream" $services}}
}

server {
  {{- template "httpServer" (dict "domains" $domains "should_redirect" $should_redirect) }}
}

This should be sufficient for allowing cross-service and cross-stack domain name upstream association.

Munsio commented 6 years ago

All in all an good idea.

About the redirect-target, i don't know if rancher-gen sorts the services in the same way as they are exposed to the webinterface, if so i would use the normal programing rule, last definded is the one which is used, it would be easier in the template and also i think it wouldn't make sense if i use the same service multiple times that one of them had other labels for domains redirects etc. that sound like a job for docker-compose but startet multiple times. i hope this make sense.

But we shouldn't forget about the hosts - i think we should go the way host > service > container. so if an host with an domain is specified we shouldn't allow an service which has the same domain and so on, or we use the same as above and the last one counts.

emcniece commented 6 years ago

Resolved by https://github.com/CausticLab/rgon-proxy/pull/50