Merge ~bjornt/maas:bug-1926140-maas-url-db into maas:master

Proposed by Björn Tillenius
Status: Merged
Approved by: Björn Tillenius
Approved revision: be5103798af05ce9767f6ef85418c8bc843d3f99
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~bjornt/maas:bug-1926140-maas-url-db
Merge into: maas:master
Diff against target: 336 lines (+151/-63)
4 files modified
src/maasserver/start_up.py (+4/-0)
src/maasserver/tests/test_start_up.py (+26/-0)
src/maasserver/websockets/handlers/config.py (+27/-30)
src/maasserver/websockets/handlers/tests/test_config.py (+94/-33)
Reviewer Review Type Date Requested Status
Alberto Donato Approve
Review via email: mp+402337@code.launchpad.net

Commit message

LP #1926140: maas_url not returned to the UI

The UI gets the URL from the config websocket handler, but the code in
there to return the maas url was broken. The maas url was never stored
in the database, so the UI always got the default
http://localhost:5240/MAAS.

Now we sync the maas_url from regiond.conf to the database on startup.
Eventually we want to get rid of maas_url from regiond.conf completely
and store it only in the database.

To post a comment you must log in.
Revision history for this message
Björn Tillenius (bjornt) wrote :

The actual fix was very small, but then I went to the handler to make it less confusing, and discovered a bug there. So I took the opportunity to fix the handler as well.

Revision history for this message
Alberto Donato (ack) wrote :

nice, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/start_up.py b/src/maasserver/start_up.py
2index 368f4c0..aa73a6f 100644
3--- a/src/maasserver/start_up.py
4+++ b/src/maasserver/start_up.py
5@@ -12,6 +12,7 @@ from twisted.internet import reactor
6 from twisted.internet.defer import inlineCallbacks
7
8 from maasserver import locks, security
9+from maasserver.config import RegionConfiguration
10 from maasserver.deprecations import sync_deprecation_notifications
11 from maasserver.fields import register_mac_type
12 from maasserver.models import (
13@@ -151,6 +152,9 @@ def inner_start_up(master=False):
14 ident="commissioning_release_deprecated",
15 )
16
17+ with RegionConfiguration.open() as config:
18+ Config.objects.set_config("maas_url", config.maas_url)
19+
20 # Update deprecation notifications if needed
21 sync_deprecation_notifications()
22
23diff --git a/src/maasserver/tests/test_start_up.py b/src/maasserver/tests/test_start_up.py
24index f9804ab..a2df245 100644
25--- a/src/maasserver/tests/test_start_up.py
26+++ b/src/maasserver/tests/test_start_up.py
27@@ -15,6 +15,7 @@ from maasserver.models.config import Config
28 from maasserver.models.node import RegionController
29 from maasserver.models.notification import Notification
30 from maasserver.models.signals import bootsources
31+from maasserver.testing.config import RegionConfigurationFixture
32 from maasserver.testing.eventloop import RegionEventLoopFixture
33 from maasserver.testing.factory import factory
34 from maasserver.testing.testcase import (
35@@ -152,6 +153,31 @@ class TestInnerStartUp(MAASServerTestCase):
36 ).exists()
37 )
38
39+ def test_sets_maas_url_master(self):
40+ Config.objects.set_config("maas_url", "http://default.example.com/")
41+ self.useFixture(
42+ RegionConfigurationFixture(maas_url="http://custom.example.com/")
43+ )
44+ with post_commit_hooks:
45+ start_up.inner_start_up(master=True)
46+
47+ self.assertEqual(
48+ "http://custom.example.com/", Config.objects.get_config("maas_url")
49+ )
50+
51+ def test_sets_maas_url_not_master(self):
52+ Config.objects.set_config("maas_url", "http://default.example.com/")
53+ self.useFixture(
54+ RegionConfigurationFixture(maas_url="http://my.example.com/")
55+ )
56+ with post_commit_hooks:
57+ start_up.inner_start_up(master=False)
58+
59+ self.assertEqual(
60+ "http://default.example.com/",
61+ Config.objects.get_config("maas_url"),
62+ )
63+
64 def test_generates_certificate_if_master(self):
65 with post_commit_hooks:
66 start_up.inner_start_up(master=True)
67diff --git a/src/maasserver/websockets/handlers/config.py b/src/maasserver/websockets/handlers/config.py
68index 7a0af79..61bc304 100644
69--- a/src/maasserver/websockets/handlers/config.py
70+++ b/src/maasserver/websockets/handlers/config.py
71@@ -7,11 +7,13 @@
72 from django.core.exceptions import ValidationError
73 from django.http import HttpRequest
74
75-from maasserver.config import RegionConfiguration
76 from maasserver.enum import ENDPOINT
77-from maasserver.forms.settings import CONFIG_ITEMS as FORM_CONFIG_ITEMS
78-from maasserver.forms.settings import get_config_field, get_config_form
79-from maasserver.models.config import Config, get_default_config
80+from maasserver.forms.settings import (
81+ CONFIG_ITEMS,
82+ get_config_field,
83+ get_config_form,
84+)
85+from maasserver.models.config import Config
86 from maasserver.websockets.base import (
87 Handler,
88 HandlerDoesNotExistError,
89@@ -20,11 +22,12 @@ from maasserver.websockets.base import (
90 HandlerValidationError,
91 )
92
93-CONFIG_ITEMS = dict(FORM_CONFIG_ITEMS)
94-for name in ("uuid", "rpc_shared_secret"):
95- CONFIG_ITEMS[name] = None
96-with RegionConfiguration.open() as config:
97- CONFIG_ITEMS["maas_url"] = config.maas_url
98+
99+def get_config_keys(user):
100+ config_keys = list(CONFIG_ITEMS.keys()) + ["uuid", "maas_url"]
101+ if user.is_superuser:
102+ config_keys.append("rpc_shared_secret")
103+ return config_keys
104
105
106 class ConfigHandler(Handler):
107@@ -46,37 +49,31 @@ class ConfigHandler(Handler):
108 self._include_choice(config_key)
109 return config_keys
110
111+ def dehydrate_configs(self, config_keys):
112+ return self._include_choices(
113+ [
114+ {"name": name, "value": value}
115+ for name, value in Config.objects.get_configs(
116+ config_keys
117+ ).items()
118+ ]
119+ )
120+
121 def list(self, params):
122 """List all the configuration values."""
123- defaults = get_default_config()
124- config_keys = CONFIG_ITEMS.keys()
125- config_objs = Config.objects.filter(name__in=config_keys)
126- config_objs = {obj.name: obj for obj in config_objs}
127+ config_keys = get_config_keys(self.user)
128 self.cache["loaded_pks"].update(config_keys)
129- config_keys = [
130- {
131- "name": key,
132- "value": (
133- config_objs[key].value
134- if key in config_objs
135- else defaults.get(key, "")
136- ),
137- }
138- for key in config_keys
139- ]
140- return self._include_choices(config_keys)
141+ return self.dehydrate_configs(config_keys)
142
143 def get(self, params):
144 """Get a config value."""
145 if "name" not in params:
146 raise HandlerPKError("Missing name in params")
147 name = params["name"]
148- if name not in CONFIG_ITEMS:
149+ if name not in get_config_keys(self.user):
150 raise HandlerDoesNotExistError(name)
151 self.cache["loaded_pks"].update({name})
152- return self._include_choice(
153- {"name": name, "value": Config.objects.get_config(name)}
154- )
155+ return self.dehydrate_configs([name])[0]
156
157 def _fix_validation_error(self, name, errors):
158 """Map the field name to the value field, which is what is used
159@@ -116,7 +113,7 @@ class ConfigHandler(Handler):
160 def on_listen(self, channel, action, pk):
161 """Override on_listen to always send the config values."""
162 config = Config.objects.get(id=pk)
163- if config.name not in CONFIG_ITEMS:
164+ if config.name not in get_config_keys(self.user):
165 return None
166 if config.name in self.cache["loaded_pks"]:
167 action = "update"
168diff --git a/src/maasserver/websockets/handlers/tests/test_config.py b/src/maasserver/websockets/handlers/tests/test_config.py
169index 2774982..de99882 100644
170--- a/src/maasserver/websockets/handlers/tests/test_config.py
171+++ b/src/maasserver/websockets/handlers/tests/test_config.py
172@@ -6,10 +6,9 @@
173
174 import random
175
176-from django.core.exceptions import ValidationError
177 from testtools import ExpectedException
178
179-from maasserver.forms.settings import get_config_field
180+from maasserver.forms.settings import CONFIG_ITEMS, get_config_field
181 from maasserver.models.config import Config, get_default_config
182 from maasserver.testing.factory import factory
183 from maasserver.testing.testcase import MAASServerTestCase
184@@ -19,34 +18,46 @@ from maasserver.websockets.base import (
185 HandlerPKError,
186 HandlerValidationError,
187 )
188-from maasserver.websockets.handlers.config import CONFIG_ITEMS, ConfigHandler
189+from maasserver.websockets.handlers.config import (
190+ ConfigHandler,
191+ get_config_keys,
192+)
193
194
195 class TestConfigHandler(MAASServerTestCase):
196- def dehydrated_all_configs(self):
197- defaults = get_default_config()
198- config_keys = CONFIG_ITEMS.keys()
199- config_objs = Config.objects.filter(name__in=config_keys)
200- config_objs = {obj.name: obj for obj in config_objs}
201- config_keys = [
202- {
203- "name": key,
204- "value": (
205- config_objs[key].value
206- if key in config_objs
207- else defaults.get(key, "")
208- ),
209- }
210- for key in config_keys
211- ]
212- for config_key in config_keys:
213- try:
214- config_field = get_config_field(config_key["name"])
215- if hasattr(config_field, "choices"):
216- config_key["choices"] = config_field.choices
217- except ValidationError:
218- pass
219- return config_keys
220+ def test_dehydrate_no_choice_config(self):
221+ no_choice_name = random.choice(
222+ list(
223+ name
224+ for name in CONFIG_ITEMS.keys()
225+ if not hasattr(get_config_field(name), "choices")
226+ )
227+ )
228+ Config.objects.set_config(no_choice_name, "myvalue")
229+ user = factory.make_User()
230+ handler = ConfigHandler(user, {}, None)
231+ dehydrated = handler.dehydrate_configs([no_choice_name])[0]
232+ self.assertEqual(no_choice_name, dehydrated["name"])
233+ self.assertEqual("myvalue", dehydrated["value"])
234+ self.assertNotIn("choices", dehydrated)
235+
236+ def test_dehydrate_choice_config(self):
237+ choice_name, choices = random.choice(
238+ list(
239+ (name, get_config_field(name).choices)
240+ for name in CONFIG_ITEMS.keys()
241+ if hasattr(get_config_field(name), "choices")
242+ )
243+ )
244+
245+ choice_value = random.choice([value for value, _ in choices])
246+ Config.objects.set_config(choice_name, choice_value)
247+ user = factory.make_User()
248+ handler = ConfigHandler(user, {}, None)
249+ dehydrated = handler.dehydrate_configs([choice_name])[0]
250+ self.assertEqual(choice_name, dehydrated["name"])
251+ self.assertEqual(choice_value, dehydrated["value"])
252+ self.assertEqual(choices, dehydrated["choices"])
253
254 def test_get(self):
255 user = factory.make_User()
256@@ -79,24 +90,74 @@ class TestConfigHandler(MAASServerTestCase):
257 )
258
259 def test_get_must_be_in_config_items(self):
260- allowed_keys = set(CONFIG_ITEMS.keys())
261+ admin = factory.make_admin()
262+ allowed_keys = set(get_config_keys(admin))
263 all_keys = set(get_default_config().keys())
264 not_allowed_keys = all_keys.difference(allowed_keys)
265 key = random.choice(list(not_allowed_keys))
266+ handler = ConfigHandler(admin, {}, None)
267+ self.assertRaises(HandlerDoesNotExistError, handler.get, {"name": key})
268+
269+ def test_list_admin_includes_all_config(self):
270+ admin = factory.make_admin()
271+ config_keys = list(CONFIG_ITEMS.keys()) + [
272+ "maas_url",
273+ "uuid",
274+ "rpc_shared_secret",
275+ ]
276+
277+ handler = ConfigHandler(admin, {}, None)
278+ self.assertCountEqual(
279+ config_keys, [item["name"] for item in handler.list({})]
280+ )
281+
282+ def test_list_user_excludes_secret(self):
283 user = factory.make_User()
284+ config_keys = list(CONFIG_ITEMS.keys()) + ["maas_url", "uuid"]
285+
286 handler = ConfigHandler(user, {}, None)
287- self.assertRaises(HandlerDoesNotExistError, handler.get, {"name": key})
288+ self.assertNotIn("rpc_shared_secret", config_keys)
289+ self.assertCountEqual(
290+ config_keys, [item["name"] for item in handler.list({})]
291+ )
292+
293+ def test_list_admin_includes_rpc_secret(self):
294+ Config.objects.set_config("rpc_shared_secret", "my-secret")
295+ admin = factory.make_admin()
296+ handler = ConfigHandler(admin, {}, None)
297+ config = {item["name"]: item["value"] for item in handler.list({})}
298+ self.assertEqual("my-secret", config["rpc_shared_secret"])
299+
300+ def test_get_admin_allows_rpc_secret(self):
301+ Config.objects.set_config("rpc_shared_secret", "my-secret")
302+ admin = factory.make_admin()
303+ handler = ConfigHandler(admin, {}, None)
304+ self.assertEqual(
305+ "my-secret", handler.get({"name": "rpc_shared_secret"})["value"]
306+ )
307
308- def test_list(self):
309+ def test_list_non_admin_hides_rpc_secret(self):
310+ Config.objects.set_config("rpc_shared_secret", "my-secret")
311 user = factory.make_User()
312 handler = ConfigHandler(user, {}, None)
313- self.assertItemsEqual(self.dehydrated_all_configs(), handler.list({}))
314+ config = {item["name"]: item["value"] for item in handler.list({})}
315+ self.assertNotIn("rpc_shared_secret", config)
316+
317+ def test_get_non_admin_disallows_rpc_secret(self):
318+ Config.objects.set_config("rpc_shared_secret", "my-secret")
319+ user = factory.make_User()
320+ handler = ConfigHandler(user, {}, None)
321+ self.assertRaises(
322+ HandlerDoesNotExistError,
323+ handler.get,
324+ {"name": "rpc_shared_secret"},
325+ )
326
327 def test_list_sets_loaded_pks_in_cache(self):
328 user = factory.make_User()
329 handler = ConfigHandler(user, {}, None)
330- handler.list({})
331- config_keys = {obj["name"] for obj in self.dehydrated_all_configs()}
332+ configs = handler.list({})
333+ config_keys = {obj["name"] for obj in configs}
334 self.assertItemsEqual(config_keys, handler.cache["loaded_pks"])
335
336 def test_update_as_non_admin_asserts(self):

Subscribers

People subscribed via source and target branches