Merge ~hloeung/content-cache-charm:master into content-cache-charm:master

Proposed by Haw Loeung
Status: Merged
Approved by: Tom Haddon
Approved revision: b274f7d4d4151212a5a9fa3752312d87f398c7b3
Merged at revision: 687d6d03ad15df2a82145e9744a202f11ec3d3f3
Proposed branch: ~hloeung/content-cache-charm:master
Merge into: content-cache-charm:master
Diff against target: 40 lines (+14/-1)
2 files modified
lib/haproxy.py (+11/-0)
tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output3.txt (+3/-1)
Reviewer Review Type Date Requested Status
Tom Haddon Approve
Canonical IS Reviewers Pending
Review via email: mp+388792@code.launchpad.net

Commit message

Fix default site being a redirect

Description of the change

HAProxy processes redirects before any backend configs (use_backend or
default_backend). With this, the 'use_backend' line in the example
below is never used:

| listen combined-80
| bind 0.0.0.0:80
| bind :::80
| redirect scheme https code 301 if { hdr(Host) -i site1.local } !{ ssl_fc }
| use_backend backend-site2-http if { hdr(Host) -i site2.local }
| redirect prefix https://site3.local

So any requests for site2.local will always hit the redirect
prefix. Instead, we need to create a new backend with the redirect
rule like so:

| listen combined-80
| bind 0.0.0.0:80
| bind :::80
| redirect scheme https code 301 if { hdr(Host) -i site1.local } !{ ssl_fc }
| use_backend backend-site2-http if { hdr(Host) -i site2.local }
| default backend-default-site
|
| backend backend-default-site
| redirect prefix https://site3.local

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
Tom Haddon (mthaddon) wrote :

Is there any risk of collision here since you've got a function called render_stanza_listen that was previously only rendering listen stanzas but can now potentially also render a "backend" stanza in this case?

review: Needs Information
Revision history for this message
Tom Haddon (mthaddon) wrote :

I think it's okay, because the render_stanza_backend is always rendering them as `backend backend-{name}` whereas this is using `backend default-redirect-site-{port}`.

Revision history for this message
Tom Haddon (mthaddon) wrote :

LGTM, thx

review: Approve
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 687d6d03ad15df2a82145e9744a202f11ec3d3f3

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/haproxy.py b/lib/haproxy.py
index bc77f62..15088f5 100644
--- a/lib/haproxy.py
+++ b/lib/haproxy.py
@@ -181,6 +181,17 @@ listen {name}
181 if address == '0.0.0.0':181 if address == '0.0.0.0':
182 bind_config += '\n{indent}bind :::{port}{tls}'.format(port=port, tls=tls_config, indent=INDENT)182 bind_config += '\n{indent}bind :::{port}{tls}'.format(port=port, tls=tls_config, indent=INDENT)
183183
184 # Redirects are always processed before use_backends so we
185 # need to convert default redirect sites to a backend.
186 if len(backend_config) > 1 and default_backend.startswith("{indent}redirect prefix".format(indent=INDENT)):
187 backend_name = self._generate_stanza_name("default-redirect-{}".format(name), exclude=stanza_names)
188 output = "backend {}\n".format(backend_name) + default_backend
189 default_backend = "{indent}default_backend {backend_name}\n".format(
190 backend_name=backend_name, indent=INDENT
191 )
192 rendered_output.append(output)
193 stanza_names.append(backend_name)
194
184 output = listen_stanza.format(195 output = listen_stanza.format(
185 name=name,196 name=name,
186 backend_config=''.join(backend_config),197 backend_config=''.join(backend_config),
diff --git a/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output3.txt b/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output3.txt
index e2614b3..d7d3110 100644
--- a/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output3.txt
+++ b/tests/unit/files/haproxy_config_rendered_listen_stanzas_test_output3.txt
@@ -1,10 +1,12 @@
1backend default-redirect-combined-80
2 redirect prefix https://site2.local
13
2listen combined-804listen combined-80
3 bind 0.0.0.0:805 bind 0.0.0.0:80
4 bind :::806 bind :::80
5 redirect scheme https code 301 if { hdr(Host) -i site1.local } !{ ssl_fc }7 redirect scheme https code 301 if { hdr(Host) -i site1.local } !{ ssl_fc }
6 redirect scheme https code 301 if { hdr(Host) -i site2.local } !{ ssl_fc }8 redirect scheme https code 301 if { hdr(Host) -i site2.local } !{ ssl_fc }
7 redirect prefix https://site2.local9 default_backend default-redirect-combined-80
810
9listen combined-44311listen combined-443
10 bind 0.0.0.0:443 ssl crt /var/lib/haproxy/certs alpn h2,http/1.112 bind 0.0.0.0:443 ssl crt /var/lib/haproxy/certs alpn h2,http/1.1

Subscribers

People subscribed via source and target branches