Merge lp:~dpb/landscape-charm/vhost-config-relation into lp:~landscape/landscape-charm/trunk
- vhost-config-relation
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | David Britton |
Approved revision: | 198 |
Merged at revision: | 181 |
Proposed branch: | lp:~dpb/landscape-charm/vhost-config-relation |
Merge into: | lp:~landscape/landscape-charm/trunk |
Diff against target: |
523 lines (+316/-27) 7 files modified
Makefile (+14/-2) config/landscape-deployments.yaml (+3/-2) hooks/hooks.py (+96/-17) hooks/lib/util.py (+32/-6) hooks/test_hooks.py (+167/-0) metadata.yaml (+2/-0) tests/01-begin (+2/-0) |
To merge this branch: | bzr merge lp:~dpb/landscape-charm/vhost-config-relation |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andreas Hasenack | Approve | ||
Adam Collard (community) | Approve | ||
Review via email: mp+222878@code.launchpad.net |
Commit message
We can now link apache2 and landscape in a relation. This will allow the rather awkward step of "specify this random vhost base64 blob in a juju set command" to be removed from our instructions and bundle.yaml. It also gets us one step closer to a real bundle that can go into the charm store.
Also:
* ssl cert stored from apache
* get apache2 servername and use it to set the root_url of landscape
* some deve cleanups
Description of the change
In this exciting change, we can now link apache2 and landscape in a relation. This will allow the rather awkward step of "specify this random vhost base64 blob in a juju set command" to be removed from our instructions and bundle.yaml.
It also gets us one step closer to a real bundle that can go into the charm store.
Instead, landsacpe will specify it over a relation to apache2 directly. The admin is still in control of this behavior with add-relation/
Major Features:
- vhost-config relation added (need latest apache2 charm to test)
- base64 blobs removed from deployer yaml.
- self-signed-SSL cert and apache "servername" auto-discovered on the same relation, cert stored in the usual place, root_url updated automatically with servername
- unit test cases
- I'm going to add an integration test case to check that the ssl cert and root url are set correctly, but maybe in a follow-on branch, I'll see.
To test:
- make lint test integration-test
- deploy and of the landscape-bundles and see if the ssl cert is passed correctly and stored on the server in the right place (landscape/0).
David Britton (dpb) wrote : | # |
Thanks Adam, please look again, I fixed all those.
David Britton (dpb) wrote : | # |
BTW, I committed r184 and tested it, and it hit a very weird error in the locking code. Not only did it hit an error, but the error was swallowed and the charm just got stuck by exiting 0 and blocking other relations.
I need to investigate not only why the change was bad, but also why it didn't get an error back to juju (exit 1)
David Britton (dpb) wrote : | # |
> BTW, I committed r184 and tested it, and it hit a very weird error in the
> locking code. Not only did it hit an error, but the error was swallowed and
> the charm just got stuck by exiting 0 and blocking other relations.
>
> I need to investigate not only why the change was bad, but also why it didn't
> get an error back to juju (exit 1)
So... I've reverted that for now.
David Britton (dpb) wrote : | # |
OK, remerged, and ready for a second review. Sorry, got distracted with the documentation earlier today and when I pushed it was not ready/tested.
During testing, I addressed a couple things that I noticed:
1) I added a make target to deploy from the current branch. Simplifies testing a bit.
2) I removed a couple of try/except blocks that were trapping errors in the lock_exclusive code path. That code shouldn't fail unless it legitimately *should* fail the hook. It blocks getting the lock by design, that try/execpt block was inserted chasing another bug and was never removed. It finally bit me during testing, and I figured it was time to remove it. Also added a test to check this case.
Adam Collard (adam-collard) wrote : | # |
Looks good! A couple of possible cleanups inline below. Successfully deployed dense MAAS target (after fighting AMT for a bit).
Andreas Hasenack (ahasenack) wrote : | # |
I deployed max, and all is good. I then destroyed the single apache2 unit (juju destroy-unit apache2/0), and added a new one, and as a new machine (not lxc). It got the host bohr.beretstack. I logged in, saw LDS as normal, but in the settings page, root-url was still set to the IP of the unit I destroyed.
Is this scenario something this branch should handle?
The certificate was regenerated, as expected, since it's a whole new unit, and landscape/0's /etc/ssl/
root@juju-
subject= /CN=bohr.beretstack
AFAICT, the only thing that didn't get updated was the root-url in the landscape DB.
Andreas Hasenack (ahasenack) wrote : | # |
Hmm, wait a second. The DB has the right root-url:
landscape-
key | value
-------
landscape.
landscape.root_url | u24:https:/
(2 rows)
system_email_sender is still wrong, btw.
What is odd is that the landscape UI is showing the wrong root_url in the standalone-settings page. There it is https:/
Andreas Hasenack (ahasenack) wrote : | # |
The root url in the settings page was the correct one only after I restarted the app services. This looks to be a bug in landscape code.
David Britton (dpb) wrote : | # |
Finalized feedback. Also added a relation between landscape-app and apache2 (vhost-config). There may yet be more bugs with the max deployment, but will fix those in follow-ups if need be.
🤖 Landscape Builder (landscape-builder) wrote : | # |
The attempt to merge lp:~davidpbritton/landscape-charm/vhost-config-relation into lp:landscape-charm/trunk failed. Below is the output from the failed tests.
trial hooks
hooks.test_hooks
TestHooksService
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
test_
- 198. By David Britton
-
remove lint, fix broken test, add JUJU_ENV to -p in make integration-test
Preview Diff
1 | === modified file 'Makefile' | |||
2 | --- Makefile 2014-06-06 19:57:27 +0000 | |||
3 | +++ Makefile 2014-06-14 03:50:22 +0000 | |||
4 | @@ -16,8 +16,16 @@ | |||
5 | 16 | $(EXTRA_UPDATE_ARGUMENTS) \ | 16 | $(EXTRA_UPDATE_ARGUMENTS) \ |
6 | 17 | apache2 postgresql juju-gui haproxy rabbitmq-server nfs | 17 | apache2 postgresql juju-gui haproxy rabbitmq-server nfs |
7 | 18 | 18 | ||
10 | 19 | integration-test: verify-juju-test config/repo-file config/license-file config/vhostssl.tmpl config/vhost.tmpl | 19 | test-depends: verify-juju-test config/repo-file config/license-file config/vhostssl.tmpl config/vhost.tmpl |
11 | 20 | juju test --set-e -p SKIP_SLOW_TESTS,DEPLOYER_TARGET,JUJU_HOME -v --timeout 3000s | 20 | |
12 | 21 | integration-test: test-depends | ||
13 | 22 | juju test --set-e -p SKIP_SLOW_TESTS,DEPLOYER_TARGET,JUJU_HOME,JUJU_ENV -v --timeout 3000s | ||
14 | 23 | |||
15 | 24 | deploy-dense-maas: test-depends | ||
16 | 25 | SKIP_TESTS=1 DEPLOYER_TARGET=landscape-dense-maas tests/01-begin | ||
17 | 26 | |||
18 | 27 | deploy-max-dense-maas: test-depends | ||
19 | 28 | SKIP_TESTS=1 DEPLOYER_TARGET=landscape-max-dense-maas tests/01-begin | ||
20 | 21 | 29 | ||
21 | 22 | lint: | 30 | lint: |
22 | 23 | flake8 --exclude=charmhelpers hooks | 31 | flake8 --exclude=charmhelpers hooks |
23 | @@ -28,9 +36,13 @@ | |||
24 | 28 | clean: clean-integration-test | 36 | clean: clean-integration-test |
25 | 29 | 37 | ||
26 | 30 | .PHONY: lint \ | 38 | .PHONY: lint \ |
27 | 39 | test-depends \ | ||
28 | 40 | deploy-dense-maas \ | ||
29 | 41 | deploy-max-dense-maas \ | ||
30 | 31 | integration-test \ | 42 | integration-test \ |
31 | 32 | verify-juju-test \ | 43 | verify-juju-test \ |
32 | 33 | test \ | 44 | test \ |
33 | 34 | clean \ | 45 | clean \ |
34 | 35 | clean-integration-test \ | 46 | clean-integration-test \ |
35 | 36 | update-charm-revision-numbers | 47 | update-charm-revision-numbers |
36 | 48 | |||
37 | 37 | 49 | ||
38 | === modified file 'config/landscape-deployments.yaml' | |||
39 | --- config/landscape-deployments.yaml 2014-06-12 10:38:38 +0000 | |||
40 | +++ config/landscape-deployments.yaml 2014-06-14 03:50:22 +0000 | |||
41 | @@ -25,8 +25,6 @@ | |||
42 | 25 | ssl_cert: SELFSIGNED | 25 | ssl_cert: SELFSIGNED |
43 | 26 | ssl_certlocation: apache2.cert | 26 | ssl_certlocation: apache2.cert |
44 | 27 | ssl_keylocation: apache2.key | 27 | ssl_keylocation: apache2.key |
45 | 28 | vhost_https_template: include-base64://vhostssl.tmpl | ||
46 | 29 | vhost_http_template: include-base64://vhost.tmpl | ||
47 | 30 | 28 | ||
48 | 31 | landscape: | 29 | landscape: |
49 | 32 | inherits: _common | 30 | inherits: _common |
50 | @@ -53,6 +51,7 @@ | |||
51 | 53 | relations: | 51 | relations: |
52 | 54 | - [landscape, rabbitmq-server] | 52 | - [landscape, rabbitmq-server] |
53 | 55 | - [landscape, haproxy] | 53 | - [landscape, haproxy] |
54 | 54 | - ["landscape:vhost-config", "apache2:vhost-config"] | ||
55 | 56 | - ["landscape:db-admin", "postgresql:db-admin"] | 55 | - ["landscape:db-admin", "postgresql:db-admin"] |
56 | 57 | - ["haproxy:website", "apache2:reverseproxy"] | 56 | - ["haproxy:website", "apache2:reverseproxy"] |
57 | 58 | - [landscape-msg, rabbitmq-server] | 57 | - [landscape-msg, rabbitmq-server] |
58 | @@ -119,6 +118,7 @@ | |||
59 | 119 | relations: | 118 | relations: |
60 | 120 | - [landscape, rabbitmq-server] | 119 | - [landscape, rabbitmq-server] |
61 | 121 | - [landscape, haproxy] | 120 | - [landscape, haproxy] |
62 | 121 | - ["landscape:vhost-config", "apache2:vhost-config"] | ||
63 | 122 | - ["landscape:db-admin", "postgresql:db-admin"] | 122 | - ["landscape:db-admin", "postgresql:db-admin"] |
64 | 123 | - ["haproxy:website", "apache2:reverseproxy"] | 123 | - ["haproxy:website", "apache2:reverseproxy"] |
65 | 124 | - [landscape-msg, rabbitmq-server] | 124 | - [landscape-msg, rabbitmq-server] |
66 | @@ -130,6 +130,7 @@ | |||
67 | 130 | - [landscape-app, rabbitmq-server] | 130 | - [landscape-app, rabbitmq-server] |
68 | 131 | - [landscape-app, haproxy] | 131 | - [landscape-app, haproxy] |
69 | 132 | - ["landscape-app:db-admin", "postgresql:db-admin"] | 132 | - ["landscape-app:db-admin", "postgresql:db-admin"] |
70 | 133 | - ["landscape-app:vhost-config", "apache2:vhost-config"] | ||
71 | 133 | 134 | ||
72 | 134 | landscape-max-dense-maas: | 135 | landscape-max-dense-maas: |
73 | 135 | inherits: landscape-max | 136 | inherits: landscape-max |
74 | 136 | 137 | ||
75 | === modified file 'hooks/hooks.py' | |||
76 | --- hooks/hooks.py 2014-06-09 20:33:41 +0000 | |||
77 | +++ hooks/hooks.py 2014-06-14 03:50:22 +0000 | |||
78 | @@ -9,15 +9,15 @@ | |||
79 | 9 | from lib import util | 9 | from lib import util |
80 | 10 | from lib.juju import Juju | 10 | from lib.juju import Juju |
81 | 11 | 11 | ||
83 | 12 | from base64 import b64encode | 12 | from base64 import b64encode, b64decode |
84 | 13 | from configobj import ConfigObj, ConfigObjError | 13 | from configobj import ConfigObj, ConfigObjError |
85 | 14 | from contextlib import closing | ||
86 | 14 | from copy import deepcopy | 15 | from copy import deepcopy |
87 | 15 | import cStringIO | 16 | import cStringIO |
88 | 16 | import datetime | 17 | import datetime |
89 | 17 | import grp | 18 | import grp |
90 | 18 | import os | 19 | import os |
91 | 19 | import psutil | 20 | import psutil |
92 | 20 | import psycopg2 | ||
93 | 21 | import pwd | 21 | import pwd |
94 | 22 | import pycurl | 22 | import pycurl |
95 | 23 | import re | 23 | import re |
96 | @@ -27,6 +27,9 @@ | |||
97 | 27 | from subprocess import check_call, check_output, CalledProcessError, call | 27 | from subprocess import check_call, check_output, CalledProcessError, call |
98 | 28 | 28 | ||
99 | 29 | 29 | ||
100 | 30 | SSL_CERT_LOCATION = "/etc/ssl/certs/landscape_server_ca.crt" | ||
101 | 31 | |||
102 | 32 | |||
103 | 30 | def _get_installed_version(name): | 33 | def _get_installed_version(name): |
104 | 31 | """Returns the version string of name using dpkg-query or returns None""" | 34 | """Returns the version string of name using dpkg-query or returns None""" |
105 | 32 | try: | 35 | try: |
106 | @@ -95,6 +98,29 @@ | |||
107 | 95 | services=yaml.safe_dump(_get_services_haproxy())) | 98 | services=yaml.safe_dump(_get_services_haproxy())) |
108 | 96 | 99 | ||
109 | 97 | 100 | ||
110 | 101 | def notify_vhost_config_relation(relation_id=None): | ||
111 | 102 | """ | ||
112 | 103 | Notify the vhost-config relation. | ||
113 | 104 | |||
114 | 105 | This will mark it "ready to proceed". If relation_id is specified | ||
115 | 106 | use that as the relation context, otherwise look up and notify all | ||
116 | 107 | vhost-config relations. | ||
117 | 108 | """ | ||
118 | 109 | vhosts = [] | ||
119 | 110 | with open("%s/config/vhostssl.tmpl" % ROOT, 'r') as handle: | ||
120 | 111 | vhosts.append({ | ||
121 | 112 | "port": "443", "template": b64encode(handle.read())}) | ||
122 | 113 | with open("%s/config/vhost.tmpl" % ROOT, 'r') as handle: | ||
123 | 114 | vhosts.append({ | ||
124 | 115 | "port": "80", "template": b64encode(handle.read())}) | ||
125 | 116 | |||
126 | 117 | relation_ids = [relation_id] | ||
127 | 118 | if relation_id is None: | ||
128 | 119 | relation_ids = juju.relation_ids("vhost-config") | ||
129 | 120 | for relation_id in relation_ids: | ||
130 | 121 | juju.relation_set(relation_id=relation_id, vhosts=yaml.dump(vhosts)) | ||
131 | 122 | |||
132 | 123 | |||
133 | 98 | def db_admin_relation_joined(): | 124 | def db_admin_relation_joined(): |
134 | 99 | pass | 125 | pass |
135 | 100 | 126 | ||
136 | @@ -150,20 +176,13 @@ | |||
137 | 150 | "password": password}, | 176 | "password": password}, |
138 | 151 | "schema": {"store_user": admin, "store_password": admin_password}}) | 177 | "schema": {"store_user": admin, "store_password": admin_password}}) |
139 | 152 | 178 | ||
154 | 153 | try: | 179 | with closing(util.connect_exclusive(host, admin, admin_password)): |
155 | 154 | # Name as lock so we don't try to reuse it as a database connection | 180 | util.create_user(user, password, host, admin, admin_password) |
156 | 155 | lock = util.connect_exclusive(host, admin, admin_password) | 181 | _create_maintenance_user(password, host, admin, admin_password) |
157 | 156 | except psycopg2.Error: | 182 | check_call("setup-landscape-server") |
158 | 157 | # Another unit is performing database configuration. | 183 | juju.juju_log("Landscape database initialized!") |
159 | 158 | pass | 184 | |
160 | 159 | else: | 185 | notify_vhost_config_relation() |
147 | 160 | try: | ||
148 | 161 | util.create_user(user, password, host, admin, admin_password) | ||
149 | 162 | _create_maintenance_user(password, host, admin, admin_password) | ||
150 | 163 | check_call("setup-landscape-server") | ||
151 | 164 | finally: | ||
152 | 165 | juju.juju_log("Landscape database initialized!") | ||
153 | 166 | lock.close() | ||
161 | 167 | 186 | ||
162 | 168 | try: | 187 | try: |
163 | 169 | # Handle remove-relation db-admin. This call will fail because | 188 | # Handle remove-relation db-admin. This call will fail because |
164 | @@ -316,6 +335,65 @@ | |||
165 | 316 | config_changed() | 335 | config_changed() |
166 | 317 | 336 | ||
167 | 318 | 337 | ||
168 | 338 | def vhost_config_relation_changed(): | ||
169 | 339 | """Relate to apache to configure a vhost. | ||
170 | 340 | |||
171 | 341 | This hook will supply vhost configuration as well as read simple data | ||
172 | 342 | out of apache (servername, certificate). This data is necessary for | ||
173 | 343 | informing clients of the correct URL and cert to use when connecting | ||
174 | 344 | to the server. | ||
175 | 345 | """ | ||
176 | 346 | notify_vhost_config_relation(os.environ.get("JUJU_RELATION_ID", None)) | ||
177 | 347 | |||
178 | 348 | config_obj = _get_config_obj(LANDSCAPE_SERVICE_CONF) | ||
179 | 349 | try: | ||
180 | 350 | section = config_obj["stores"] | ||
181 | 351 | database = section["main"] | ||
182 | 352 | host = section["host"] | ||
183 | 353 | user = section["user"] | ||
184 | 354 | password = section["password"] | ||
185 | 355 | except KeyError: | ||
186 | 356 | juju.juju_log("Database not ready yet, deferring call") | ||
187 | 357 | sys.exit(0) | ||
188 | 358 | |||
189 | 359 | relids = juju.relation_ids("vhost-config") | ||
190 | 360 | if relids: | ||
191 | 361 | relid = relids[0] | ||
192 | 362 | apache2_unit = juju.relation_list(relid)[0] | ||
193 | 363 | apache_servername = juju.relation_get( | ||
194 | 364 | "servername", unit_name=apache2_unit, relation_id=relid) | ||
195 | 365 | else: | ||
196 | 366 | apache_servername = juju.relation_get("servername") | ||
197 | 367 | |||
198 | 368 | if not apache_servername: | ||
199 | 369 | juju.juju_log("Waiting for data from apache, deferring") | ||
200 | 370 | sys.exit(0) | ||
201 | 371 | apache_url = "https://%s/" % apache_servername | ||
202 | 372 | |||
203 | 373 | if not _is_db_up(): | ||
204 | 374 | juju.juju_log("Waiting for database to become available, deferring.") | ||
205 | 375 | sys.exit(0) | ||
206 | 376 | |||
207 | 377 | with closing(util.connect_exclusive(host, user, password)): | ||
208 | 378 | juju.juju_log("Updating Landscape root_url: %s" % apache_url) | ||
209 | 379 | util.change_root_url(database, user, password, host, apache_url) | ||
210 | 380 | |||
211 | 381 | # This data may or may not be present, dependeing on if cert is self | ||
212 | 382 | # signed from apache. | ||
213 | 383 | ssl_cert = juju.relation_get( | ||
214 | 384 | "ssl_cert", unit_name=apache2_unit, relation_id=relid) | ||
215 | 385 | if ssl_cert: | ||
216 | 386 | juju.juju_log("Writing new SSL cert: %s" % SSL_CERT_LOCATION) | ||
217 | 387 | with open(SSL_CERT_LOCATION, 'w') as f: | ||
218 | 388 | f.write(str(b64decode(ssl_cert))) | ||
219 | 389 | else: | ||
220 | 390 | if os.path.exists(SSL_CERT_LOCATION): | ||
221 | 391 | os.remove(SSL_CERT_LOCATION) | ||
222 | 392 | |||
223 | 393 | # only starts services again if is_db_up and _is_amqp_up | ||
224 | 394 | config_changed() | ||
225 | 395 | |||
226 | 396 | |||
227 | 319 | def config_changed(): | 397 | def config_changed(): |
228 | 320 | """Update and restart services based on config setting changes. | 398 | """Update and restart services based on config setting changes. |
229 | 321 | 399 | ||
230 | @@ -743,7 +821,8 @@ | |||
231 | 743 | "data-relation-changed": data_relation_changed, | 821 | "data-relation-changed": data_relation_changed, |
232 | 744 | "db-admin-relation-joined": db_admin_relation_joined, | 822 | "db-admin-relation-joined": db_admin_relation_joined, |
233 | 745 | "db-admin-relation-changed": db_admin_relation_changed, | 823 | "db-admin-relation-changed": db_admin_relation_changed, |
235 | 746 | "website-relation-joined": website_relation_joined} | 824 | "website-relation-joined": website_relation_joined, |
236 | 825 | "vhost-config-relation-changed": vhost_config_relation_changed} | ||
237 | 747 | hook = os.path.basename(sys.argv[0]) | 826 | hook = os.path.basename(sys.argv[0]) |
238 | 748 | # If the hook is unsupported, let it raise a KeyError and exit with error. | 827 | # If the hook is unsupported, let it raise a KeyError and exit with error. |
239 | 749 | hooks[hook]() | 828 | hooks[hook]() |
240 | 750 | 829 | ||
241 | === modified file 'hooks/lib/util.py' | |||
242 | --- hooks/lib/util.py 2014-01-31 20:38:36 +0000 | |||
243 | +++ hooks/lib/util.py 2014-06-14 03:50:22 +0000 | |||
244 | @@ -2,8 +2,9 @@ | |||
245 | 2 | Utility library for juju hooks | 2 | Utility library for juju hooks |
246 | 3 | """ | 3 | """ |
247 | 4 | 4 | ||
249 | 5 | from psycopg2 import connect | 5 | from psycopg2 import connect, Error as psycopg2Error |
250 | 6 | from juju import Juju | 6 | from juju import Juju |
251 | 7 | from contextlib import closing | ||
252 | 7 | 8 | ||
253 | 8 | juju = Juju() | 9 | juju = Juju() |
254 | 9 | 10 | ||
255 | @@ -38,7 +39,7 @@ | |||
256 | 38 | password=admin_password) | 39 | password=admin_password) |
257 | 39 | try: | 40 | try: |
258 | 40 | cur = conn.cursor() | 41 | cur = conn.cursor() |
260 | 41 | cur.execute("SELECT usename FROM pg_user WHERE usename='%s'" % user) | 42 | cur.execute("SELECT usename FROM pg_user WHERE usename=%s", (user,)) |
261 | 42 | result = cur.fetchall() | 43 | result = cur.fetchall() |
262 | 43 | if not result: | 44 | if not result: |
263 | 44 | juju.juju_log("Creating postgres db user: %s" % user) | 45 | juju.juju_log("Creating postgres db user: %s" % user) |
264 | @@ -48,6 +49,32 @@ | |||
265 | 48 | conn.close() | 49 | conn.close() |
266 | 49 | 50 | ||
267 | 50 | 51 | ||
268 | 52 | def change_root_url(database, user, password, host, url): | ||
269 | 53 | """Change the root url in the database.""" | ||
270 | 54 | url = "u%s:%s" % (len(url), url) | ||
271 | 55 | with closing(connect(database=database, host=host, | ||
272 | 56 | user=user, password=password)) as conn: | ||
273 | 57 | cur = conn.cursor() | ||
274 | 58 | cur.execute("SELECT encode(key, 'escape'),encode(value, 'escape') " | ||
275 | 59 | "FROM system_configuration " | ||
276 | 60 | "WHERE key='landscape.root_url' FOR UPDATE") | ||
277 | 61 | result = cur.fetchall() | ||
278 | 62 | if not result: | ||
279 | 63 | juju.juju_log("Setting new root_url: %s" % url) | ||
280 | 64 | cur.execute( | ||
281 | 65 | "INSERT INTO system_configuration " | ||
282 | 66 | "VALUES (decode('landscape.root_url', 'escape'), " | ||
283 | 67 | " decode(%s, 'escape'))", (url,)) | ||
284 | 68 | else: | ||
285 | 69 | juju.juju_log("Updating root_url %s => %s" % (result, url)) | ||
286 | 70 | cur.execute( | ||
287 | 71 | "UPDATE system_configuration " | ||
288 | 72 | "SET key=decode('landscape.root_url', 'escape')," | ||
289 | 73 | " value=decode(%s, 'escape') " | ||
290 | 74 | "WHERE encode(key, 'escape')='landscape.root_url'", (url,)) | ||
291 | 75 | conn.commit() | ||
292 | 76 | |||
293 | 77 | |||
294 | 51 | def is_db_up(database, host, user, password): | 78 | def is_db_up(database, host, user, password): |
295 | 52 | """ | 79 | """ |
296 | 53 | Return True if the database relation is configured with write permission, | 80 | Return True if the database relation is configured with write permission, |
297 | @@ -59,10 +86,9 @@ | |||
298 | 59 | # Ensure we are user with write access, to avoid hot standby dbs | 86 | # Ensure we are user with write access, to avoid hot standby dbs |
299 | 60 | cur.execute( | 87 | cur.execute( |
300 | 61 | 'CREATE TEMP TABLE "write_access_test_%s" (id serial PRIMARY KEY) ' | 88 | 'CREATE TEMP TABLE "write_access_test_%s" (id serial PRIMARY KEY) ' |
305 | 62 | "ON COMMIT DROP" | 89 | "ON COMMIT DROP" % juju.local_unit().replace("/", "_")) |
306 | 63 | % juju.local_unit().replace("/", "_")) | 90 | except psycopg2Error as e: |
307 | 64 | except Exception as e: | 91 | juju.juju_log("Database not yet up: %s" % e) |
304 | 65 | juju.juju_log(str(e)) | ||
308 | 66 | return False | 92 | return False |
309 | 67 | else: | 93 | else: |
310 | 68 | return True | 94 | return True |
311 | 69 | 95 | ||
312 | === modified file 'hooks/test_hooks.py' | |||
313 | --- hooks/test_hooks.py 2014-05-23 20:51:15 +0000 | |||
314 | +++ hooks/test_hooks.py 2014-06-14 03:50:22 +0000 | |||
315 | @@ -3,9 +3,11 @@ | |||
316 | 3 | import hooks | 3 | import hooks |
317 | 4 | import mocker | 4 | import mocker |
318 | 5 | import os | 5 | import os |
319 | 6 | import psycopg2 | ||
320 | 6 | import pycurl | 7 | import pycurl |
321 | 7 | import stat | 8 | import stat |
322 | 8 | import subprocess | 9 | import subprocess |
323 | 10 | import tempfile | ||
324 | 9 | import yaml | 11 | import yaml |
325 | 10 | 12 | ||
326 | 11 | 13 | ||
327 | @@ -1482,6 +1484,171 @@ | |||
328 | 1482 | baseline = (("services", yaml.safe_dump(self.all_services)),) | 1484 | baseline = (("services", yaml.safe_dump(self.all_services)),) |
329 | 1483 | self.assertEqual(baseline, hooks.juju._outgoing_relation_data) | 1485 | self.assertEqual(baseline, hooks.juju._outgoing_relation_data) |
330 | 1484 | 1486 | ||
331 | 1487 | def test_notify_vhost_config_relation_specify_id(self): | ||
332 | 1488 | """ | ||
333 | 1489 | notify the vhost-config relation on a separate ID. | ||
334 | 1490 | """ | ||
335 | 1491 | hooks.notify_vhost_config_relation("foo/0") | ||
336 | 1492 | with open("%s/config/vhostssl.tmpl" % hooks.ROOT, 'r') as f: | ||
337 | 1493 | vhostssl_template = f.read() | ||
338 | 1494 | with open("%s/config/vhost.tmpl" % hooks.ROOT, 'r') as f: | ||
339 | 1495 | vhost_template = f.read() | ||
340 | 1496 | baseline = yaml.dump( | ||
341 | 1497 | [{"port": "443", "template": base64.b64encode(vhostssl_template)}, | ||
342 | 1498 | {"port": "80", "template": base64.b64encode(vhost_template)}]) | ||
343 | 1499 | self.assertEqual( | ||
344 | 1500 | (("vhosts", baseline),), hooks.juju._outgoing_relation_data) | ||
345 | 1501 | |||
346 | 1502 | def test_notify_vhost_config_relation(self): | ||
347 | 1503 | """notify the vhost-config relation on the "current" ID.""" | ||
348 | 1504 | hooks.notify_vhost_config_relation() | ||
349 | 1505 | with open("%s/config/vhostssl.tmpl" % hooks.ROOT, 'r') as f: | ||
350 | 1506 | vhostssl_template = f.read() | ||
351 | 1507 | with open("%s/config/vhost.tmpl" % hooks.ROOT, 'r') as f: | ||
352 | 1508 | vhost_template = f.read() | ||
353 | 1509 | baseline = yaml.dump( | ||
354 | 1510 | [{"port": "443", "template": base64.b64encode(vhostssl_template)}, | ||
355 | 1511 | {"port": "80", "template": base64.b64encode(vhost_template)}]) | ||
356 | 1512 | self.assertEqual( | ||
357 | 1513 | (("vhosts", baseline),), hooks.juju._outgoing_relation_data) | ||
358 | 1514 | |||
359 | 1515 | def test_vhost_config_relation_changed_exit_no_configuration(self): | ||
360 | 1516 | """Ensure vhost_relation_changed deferrs if db is not up.""" | ||
361 | 1517 | self.assertRaises(SystemExit, hooks.vhost_config_relation_changed) | ||
362 | 1518 | self.assertEquals(len(hooks.juju._logs), 1) | ||
363 | 1519 | self.assertIn('Database not ready yet', hooks.juju._logs[0]) | ||
364 | 1520 | |||
365 | 1521 | def test_vhost_config_relation_changed_wait_apache_servername(self): | ||
366 | 1522 | """Ensure vhost_relation_changed deferrs if db is not up.""" | ||
367 | 1523 | _get_config_obj = self.mocker.replace(hooks._get_config_obj) | ||
368 | 1524 | _get_config_obj(hooks.LANDSCAPE_SERVICE_CONF) | ||
369 | 1525 | self.mocker.result({ | ||
370 | 1526 | "stores": { | ||
371 | 1527 | "main": "database", | ||
372 | 1528 | "host": "host", | ||
373 | 1529 | "user": "user", | ||
374 | 1530 | "password": "password"}}) | ||
375 | 1531 | notify_vhost = self.mocker.replace(hooks.notify_vhost_config_relation) | ||
376 | 1532 | notify_vhost(None) | ||
377 | 1533 | self.mocker.replay() | ||
378 | 1534 | self.assertRaises(SystemExit, hooks.vhost_config_relation_changed) | ||
379 | 1535 | self.assertIn('Waiting for data from apache', hooks.juju._logs[-1]) | ||
380 | 1536 | |||
381 | 1537 | def test_vhost_config_relation_changed_fail_root_url(self): | ||
382 | 1538 | """Ensure vhost_relation_changed deferrs if db is not up.""" | ||
383 | 1539 | _get_config_obj = self.mocker.replace(hooks._get_config_obj) | ||
384 | 1540 | _get_config_obj(hooks.LANDSCAPE_SERVICE_CONF) | ||
385 | 1541 | hooks.juju._incoming_relation_data += (("servername", "foobar"),) | ||
386 | 1542 | self.mocker.result({ | ||
387 | 1543 | "stores": { | ||
388 | 1544 | "main": "database", | ||
389 | 1545 | "host": "host", | ||
390 | 1546 | "user": "user", | ||
391 | 1547 | "password": "password"}}) | ||
392 | 1548 | notify_vhost = self.mocker.replace(hooks.notify_vhost_config_relation) | ||
393 | 1549 | notify_vhost(None) | ||
394 | 1550 | is_db_up = self.mocker.replace(hooks._is_db_up) | ||
395 | 1551 | is_db_up() | ||
396 | 1552 | self.mocker.result(False) | ||
397 | 1553 | self.mocker.replay() | ||
398 | 1554 | self.assertRaises(SystemExit, hooks.vhost_config_relation_changed) | ||
399 | 1555 | self.assertIn( | ||
400 | 1556 | 'Waiting for database to become available, deferring', | ||
401 | 1557 | hooks.juju._logs[-1]) | ||
402 | 1558 | |||
403 | 1559 | def test_vhost_config_relation_changed_fail_root_url_db_update(self): | ||
404 | 1560 | """vhost_config_relation_changed should error if db update fails""" | ||
405 | 1561 | _get_config_obj = self.mocker.replace(hooks._get_config_obj) | ||
406 | 1562 | _get_config_obj(hooks.LANDSCAPE_SERVICE_CONF) | ||
407 | 1563 | hooks.juju._incoming_relation_data += (("servername", "foobar"),) | ||
408 | 1564 | self.mocker.result({ | ||
409 | 1565 | "stores": { | ||
410 | 1566 | "main": "database", | ||
411 | 1567 | "host": "host", | ||
412 | 1568 | "user": "user", | ||
413 | 1569 | "password": "password"}}) | ||
414 | 1570 | notify_vhost = self.mocker.replace(hooks.notify_vhost_config_relation) | ||
415 | 1571 | notify_vhost(None) | ||
416 | 1572 | is_db_up = self.mocker.replace(hooks._is_db_up) | ||
417 | 1573 | is_db_up() | ||
418 | 1574 | self.mocker.result(True) | ||
419 | 1575 | self.mocker.replay() | ||
420 | 1576 | self.assertRaises(psycopg2.Error, hooks.vhost_config_relation_changed) | ||
421 | 1577 | |||
422 | 1578 | def test_vhost_config_relation_changed_cert_not_provided(self): | ||
423 | 1579 | """ | ||
424 | 1580 | Ensure vhost_relation_changed runs to completion. | ||
425 | 1581 | |||
426 | 1582 | Existing cert should be removed. | ||
427 | 1583 | """ | ||
428 | 1584 | hooks.SSL_CERT_LOCATION = tempfile.NamedTemporaryFile().name | ||
429 | 1585 | _get_config_obj = self.mocker.replace(hooks._get_config_obj) | ||
430 | 1586 | _get_config_obj(hooks.LANDSCAPE_SERVICE_CONF) | ||
431 | 1587 | hooks.juju._incoming_relation_data += (("servername", "foobar"),) | ||
432 | 1588 | self.mocker.result({ | ||
433 | 1589 | "stores": { | ||
434 | 1590 | "main": "database", | ||
435 | 1591 | "host": "host", | ||
436 | 1592 | "user": "user", | ||
437 | 1593 | "password": "password"}}) | ||
438 | 1594 | notify_vhost = self.mocker.replace(hooks.notify_vhost_config_relation) | ||
439 | 1595 | notify_vhost(None) | ||
440 | 1596 | mock_conn = self.mocker.mock() | ||
441 | 1597 | mock_conn.close() | ||
442 | 1598 | connect_exclusive = self.mocker.replace(hooks.util.connect_exclusive) | ||
443 | 1599 | connect_exclusive("host", "user", "password") | ||
444 | 1600 | self.mocker.result(mock_conn) | ||
445 | 1601 | change_root_url = self.mocker.replace(hooks.util.change_root_url) | ||
446 | 1602 | change_root_url( | ||
447 | 1603 | "database", "user", "password", "host", "https://foobar/") | ||
448 | 1604 | config_changed = self.mocker.replace(hooks.config_changed) | ||
449 | 1605 | config_changed() | ||
450 | 1606 | is_db_up = self.mocker.replace(hooks._is_db_up) | ||
451 | 1607 | is_db_up() | ||
452 | 1608 | self.mocker.result(True) | ||
453 | 1609 | self.mocker.replay() | ||
454 | 1610 | hooks.vhost_config_relation_changed() | ||
455 | 1611 | self.assertFalse(os.path.exists(hooks.SSL_CERT_LOCATION)) | ||
456 | 1612 | |||
457 | 1613 | def test_vhost_config_relation_changed_ssl_cert_provided(self): | ||
458 | 1614 | """ | ||
459 | 1615 | Ensure vhost_relation_changed runs to completion. | ||
460 | 1616 | |||
461 | 1617 | Cert passed in to other side of relation should be written on disk. | ||
462 | 1618 | """ | ||
463 | 1619 | hooks.SSL_CERT_LOCATION = tempfile.NamedTemporaryFile().name | ||
464 | 1620 | _get_config_obj = self.mocker.replace(hooks._get_config_obj) | ||
465 | 1621 | _get_config_obj(hooks.LANDSCAPE_SERVICE_CONF) | ||
466 | 1622 | hooks.juju._incoming_relation_data += (("servername", "foobar"),) | ||
467 | 1623 | hooks.juju._incoming_relation_data += ( | ||
468 | 1624 | ("ssl_cert", base64.b64encode("foobar")),) | ||
469 | 1625 | self.mocker.result({ | ||
470 | 1626 | "stores": { | ||
471 | 1627 | "main": "database", | ||
472 | 1628 | "host": "host", | ||
473 | 1629 | "user": "user", | ||
474 | 1630 | "password": "password"}}) | ||
475 | 1631 | notify_vhost = self.mocker.replace(hooks.notify_vhost_config_relation) | ||
476 | 1632 | notify_vhost(None) | ||
477 | 1633 | is_db_up = self.mocker.replace(hooks._is_db_up) | ||
478 | 1634 | is_db_up() | ||
479 | 1635 | self.mocker.result(True) | ||
480 | 1636 | mock_conn = self.mocker.mock() | ||
481 | 1637 | mock_conn.close() | ||
482 | 1638 | connect_exclusive = self.mocker.replace(hooks.util.connect_exclusive) | ||
483 | 1639 | connect_exclusive("host", "user", "password") | ||
484 | 1640 | self.mocker.result(mock_conn) | ||
485 | 1641 | change_root_url = self.mocker.replace(hooks.util.change_root_url) | ||
486 | 1642 | change_root_url( | ||
487 | 1643 | "database", "user", "password", "host", "https://foobar/") | ||
488 | 1644 | config_changed = self.mocker.replace(hooks.config_changed) | ||
489 | 1645 | config_changed() | ||
490 | 1646 | self.mocker.replay() | ||
491 | 1647 | hooks.vhost_config_relation_changed() | ||
492 | 1648 | self.assertTrue(os.path.exists(hooks.SSL_CERT_LOCATION)) | ||
493 | 1649 | with open(hooks.SSL_CERT_LOCATION, 'r') as f: | ||
494 | 1650 | self.assertEqual("foobar", f.read()) | ||
495 | 1651 | |||
496 | 1485 | 1652 | ||
497 | 1486 | class TestHooksUtils(TestHooks): | 1653 | class TestHooksUtils(TestHooks): |
498 | 1487 | def test__setup_apache(self): | 1654 | def test__setup_apache(self): |
499 | 1488 | 1655 | ||
500 | === added symlink 'hooks/vhost-config-relation-changed' | |||
501 | === target is u'hooks.py' | |||
502 | === modified file 'metadata.yaml' | |||
503 | --- metadata.yaml 2014-04-25 15:04:36 +0000 | |||
504 | +++ metadata.yaml 2014-06-14 03:50:22 +0000 | |||
505 | @@ -23,3 +23,5 @@ | |||
506 | 23 | optional: true | 23 | optional: true |
507 | 24 | website: | 24 | website: |
508 | 25 | interface: http | 25 | interface: http |
509 | 26 | vhost-config: | ||
510 | 27 | interface: apache-vhost-config | ||
511 | 26 | 28 | ||
512 | === modified file 'tests/01-begin' | |||
513 | --- tests/01-begin 2014-06-11 12:50:56 +0000 | |||
514 | +++ tests/01-begin 2014-06-14 03:50:22 +0000 | |||
515 | @@ -191,6 +191,8 @@ | |||
516 | 191 | self.service_name = service_name | 191 | self.service_name = service_name |
517 | 192 | 192 | ||
518 | 193 | 193 | ||
519 | 194 | @unittest.skipIf( | ||
520 | 195 | getenv("SKIP_TESTS", None), "Requested to skip all tests.") | ||
521 | 194 | class BaseLandscapeTests(unittest.TestCase): | 196 | class BaseLandscapeTests(unittest.TestCase): |
522 | 195 | """ | 197 | """ |
523 | 196 | Base class with some commonality between all test classes. | 198 | Base class with some commonality between all test classes. |
SQL injection vulnerabilities, see inline comments.