MAAS

Merge ~bjornt/maas:maas-init-configauth into maas:master

  1. Git
  2. lp:~bjornt/maas
  3. maas-init-configauth
  4. Merge into master
Proposed by Björn Tillenius
Status: Merged
Approved by: Björn Tillenius
Approved revision: 931edae410e3d1af3c29f003c000969bf6b15df5
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~bjornt/maas:maas-init-configauth
Merge into: maas:master
Diff against target: 310 lines (+194/-9)
5 files modified
src/maascli/snappy.py (+49/-1)
src/maascli/tests/test_snappy.py (+96/-1)
src/maasserver/management/commands/configauth.py (+19/-6)
src/maasserver/tests/test_commands_configauth.py (+29/-0)
src/metadataserver/models/scriptresult.py (+1/-1)
Reviewer Review Type Date Requested Status
Alberto Donato (community) Approve
MAAS Lander Approve
Review via email: mp+341390@code.launchpad.net

Commit message

Configure external authentication in 'maas init'

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b maas-init-configauth lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 771106b90988a23be7aa43ff03884c26ec348d90

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

A question inline

review: Needs Information
Revision history for this message
Björn Tillenius (bjornt) :
Revision history for this message
Alberto Donato (ack) wrote :

an additional comment

Revision history for this message
Björn Tillenius (bjornt) wrote :

Addressed your comments. Probably makes sense to re-review it, since I did some changes so that I could add some tests as well.

After discussion with Andres, I also added an --enable-idm option as well, since we don't want to advertise it too much now when IDM is not even released.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b maas-init-configauth lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/1934/console
COMMIT: fd61026f4c1c33a174338ada91d4ae33708ddde1

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b maas-init-configauth lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: aa861fb67ee08b94b2dabb65583da948be9dbf2e

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

+1 looks good.

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b maas-init-configauth lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/1982/consoleText

Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b maas-init-configauth lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/1987/consoleText

~bjornt/maas:maas-init-configauth updated
931edae... by Björn Tillenius

Fix intermittent test failure.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maascli/snappy.py b/src/maascli/snappy.py
2index 07bde83..08ff34b 100755
3--- a/src/maascli/snappy.py
4+++ b/src/maascli/snappy.py
5@@ -15,6 +15,7 @@ import argparse
6 from collections import OrderedDict
7 from contextlib import contextmanager
8 import grp
9+import json
10 import os
11 import pwd
12 import random
13@@ -631,6 +632,25 @@ class cmd_init(SnappyCommand):
14 help=(
15 "Skip the admin creation when initializing in 'all' mode."))
16 parser.add_argument(
17+ '--enable-idm', default=False, action="store_true",
18+ help=("Enable configuring the use of an external IDM server. "
19+ "This feature is currently experimental. "
20+ "If this isn't enabled, all --idm-* arguments "
21+ "will be ignored."))
22+ parser.add_argument(
23+ '--idm-url', default=None, metavar='IDM_URL',
24+ help=("The URL to the external IDM server to use for "
25+ "authentication."))
26+ parser.add_argument(
27+ '--idm-user', default=None,
28+ help="The username to access the IDM server API.")
29+ parser.add_argument(
30+ '--idm-key', default=None,
31+ help="The private key to access the IDM server API.")
32+ parser.add_argument(
33+ '--idm-agent-file', type=argparse.FileType('r'),
34+ help="Agent file containing IDM authentication information")
35+ parser.add_argument(
36 '--admin-username', default=None, metavar='USERNAME',
37 help="Username for the admin account.")
38 parser.add_argument(
39@@ -773,7 +793,13 @@ class cmd_init(SnappyCommand):
40 "Performing database migrations",
41 migrate_db, capture=sys.stdout.isatty())
42 clear_line()
43- if not options.skip_admin:
44+ if options.enable_idm:
45+ print_msg('Configuring authentication')
46+ configure_authentication(options)
47+ auth_config = self._get_current_auth_config()
48+ skip_create_admin = (
49+ options.skip_admin or auth_config['external_auth_url'])
50+ if not skip_create_admin:
51 self._create_admin_account(options)
52 elif mode in ['region', 'region+rack']:
53 # When in 'region' or 'region+rack' the migrations for the database
54@@ -805,6 +831,28 @@ class cmd_init(SnappyCommand):
55 cmd.extend(['--ssh-import', options.admin_ssh_import])
56 subprocess.call(cmd)
57
58+ def _get_current_auth_config(self):
59+ cmd = [
60+ os.path.join(os.environ['SNAP'], 'bin', 'maas-region'),
61+ 'configauth', '--json']
62+ output = subprocess.check_output(cmd)
63+ return json.loads(output)
64+
65+
66+def configure_authentication(options):
67+ cmd = [
68+ os.path.join(os.environ['SNAP'], 'bin', 'maas-region'),
69+ 'configauth']
70+ if options.idm_url is not None:
71+ cmd.extend(['--idm-url', options.idm_url])
72+ if options.idm_user is not None:
73+ cmd.extend(['--idm-user', options.idm_user])
74+ if options.idm_key is not None:
75+ cmd.extend(['--idm-key', options.idm_key])
76+ if options.idm_agent_file is not None:
77+ cmd.extend(['--idm-agent-file', options.idm_agent_file.name])
78+ subprocess.call(cmd)
79+
80
81 class cmd_config(SnappyCommand):
82 """View or change controller configuration."""
83diff --git a/src/maascli/tests/test_snappy.py b/src/maascli/tests/test_snappy.py
84index f2adf70..2af352e 100644
85--- a/src/maascli/tests/test_snappy.py
86+++ b/src/maascli/tests/test_snappy.py
87@@ -9,11 +9,16 @@ import os
88 import random
89 import signal
90 import subprocess
91+import tempfile
92 from textwrap import dedent
93 import time
94-from unittest.mock import MagicMock
95+from unittest.mock import (
96+ MagicMock,
97+ patch,
98+)
99
100 from maascli import snappy
101+from maascli.parser import ArgumentParser
102 from maastesting.factory import factory
103 from maastesting.matchers import MockCalledOnceWith
104 from maastesting.testcase import MAASTestCase
105@@ -138,6 +143,96 @@ class TestHelpers(MAASTestCase):
106 self.assertEqual('none', snappy.get_current_mode())
107
108
109+class TestConfigureAuthentication(MAASTestCase):
110+
111+ def setUp(self):
112+ super().setUp()
113+ self.maas_bin_path = 'snap-path/bin/maas-region'
114+ self.mock_subprocess = self.patch(snappy, 'subprocess')
115+ self.mock_environ = patch.dict(
116+ snappy.os.environ, {'SNAP': 'snap-path'}, clear=True)
117+ self.mock_environ.start()
118+ self.parser = ArgumentParser()
119+ snappy.cmd_init(self.parser)
120+
121+ def tearDown(self):
122+ self.mock_subprocess.stop()
123+ self.mock_environ.stop()
124+ super().tearDown()
125+
126+ def test_no_options(self):
127+ options = self.parser.parse_args([])
128+ snappy.configure_authentication(options)
129+ [config_call] = self.mock_subprocess.mock_calls
130+ method, args, kwargs = config_call
131+ self.assertEqual('call', method)
132+ self.assertEqual(([self.maas_bin_path, 'configauth'],), args)
133+ self.assertEqual({}, kwargs)
134+
135+ def test_idm_url(self):
136+ config_auth_args = ['--idm-url', 'http://idm.example.com/']
137+ options = self.parser.parse_args(config_auth_args)
138+ snappy.configure_authentication(options)
139+ [config_call] = self.mock_subprocess.mock_calls
140+ method, args, kwargs = config_call
141+ self.assertEqual('call', method)
142+ self.assertEqual(
143+ ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
144+ self.assertEqual({}, kwargs)
145+
146+ def test_idm_user(self):
147+ config_auth_args = ['--idm-user', 'some-user']
148+ options = self.parser.parse_args(config_auth_args)
149+ snappy.configure_authentication(options)
150+ [config_call] = self.mock_subprocess.mock_calls
151+ method, args, kwargs = config_call
152+ self.assertEqual('call', method)
153+ self.assertEqual(
154+ ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
155+ self.assertEqual({}, kwargs)
156+
157+ def test_idm_key(self):
158+ config_auth_args = ['--idm-key', 'some-key']
159+ options = self.parser.parse_args(config_auth_args)
160+ snappy.configure_authentication(options)
161+ [config_call] = self.mock_subprocess.mock_calls
162+ method, args, kwargs = config_call
163+ self.assertEqual('call', method)
164+ self.assertEqual(
165+ ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
166+ self.assertEqual({}, kwargs)
167+
168+ def test_idm_agent_file(self):
169+ _, agent_file_path = tempfile.mkstemp()
170+ self.addCleanup(os.remove, agent_file_path)
171+ config_auth_args = ['--idm-agent-file', agent_file_path]
172+ options = self.parser.parse_args(config_auth_args)
173+ snappy.configure_authentication(options)
174+ [config_call] = self.mock_subprocess.mock_calls
175+ method, args, kwargs = config_call
176+ self.assertEqual('call', method)
177+ self.assertEqual(
178+ ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
179+ self.assertEqual({}, kwargs)
180+
181+ def test_full(self):
182+ _, agent_file = tempfile.mkstemp()
183+ self.addCleanup(os.remove, agent_file)
184+ config_auth_args = [
185+ '--idm-url', 'http://idm.example.com/',
186+ '--idm-user', 'idm-user',
187+ '--idm-key', 'idm-key',
188+ '--idm-agent-file', agent_file]
189+ options = self.parser.parse_args(config_auth_args)
190+ snappy.configure_authentication(options)
191+ [config_call] = self.mock_subprocess.mock_calls
192+ method, args, kwargs = config_call
193+ self.assertEqual('call', method)
194+ self.assertEqual(
195+ ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
196+ self.assertEqual({}, kwargs)
197+
198+
199 class TestRenderSupervisord(MAASTestCase):
200
201 TEST_TEMPLATE = dedent("""\
202diff --git a/src/maasserver/management/commands/configauth.py b/src/maasserver/management/commands/configauth.py
203index 2e8df6c..5d7c7c4 100644
204--- a/src/maasserver/management/commands/configauth.py
205+++ b/src/maasserver/management/commands/configauth.py
206@@ -58,10 +58,16 @@ def is_valid_auth_url(auth_url):
207 return True
208
209
210-def config_auth(config_manager, auth_url, auth_user, auth_key):
211- config_manager.set_config('external_auth_url', auth_url)
212- config_manager.set_config('external_auth_user', auth_user)
213- config_manager.set_config('external_auth_key', auth_key)
214+def get_auth_config(config_manager):
215+ config_keys = [
216+ 'external_auth_url', 'external_auth_user', 'external_auth_key']
217+ return {key: config_manager.get_config(key) for key in config_keys}
218+
219+
220+def set_auth_config(config_manager, auth_url, auth_user, auth_key):
221+ config_manager.set_config('external_auth_url', auth_url)
222+ config_manager.set_config('external_auth_user', auth_user)
223+ config_manager.set_config('external_auth_key', auth_key)
224
225
226 class Command(BaseCommand):
227@@ -82,16 +88,23 @@ class Command(BaseCommand):
228 parser.add_argument(
229 '--idm-agent-file', type=argparse.FileType('r'),
230 help="Agent file containing IDM authentication information")
231+ parser.add_argument(
232+ '--json', action='store_true', default=False,
233+ help="Return the current authentication configuration as JSON")
234
235 def handle(self, *args, **options):
236 config_manager = Config.objects.db_manager(DEFAULT_DB_ALIAS)
237
238+ if options.get('json'):
239+ print(json.dumps(get_auth_config(config_manager)))
240+ return
241+
242 auth_url, auth_user, auth_key = None, '', ''
243
244 agent_file = options.get('idm_agent_file')
245 if agent_file:
246 auth_url, auth_user, auth_key = read_agent_file(agent_file)
247- config_auth(config_manager, auth_url, auth_user, auth_key)
248+ set_auth_config(config_manager, auth_url, auth_user, auth_key)
249 return
250
251 auth_url = options.get('idm_url')
252@@ -111,4 +124,4 @@ class Command(BaseCommand):
253 if not auth_key:
254 auth_key = read_input("Private key for IDM API access: ")
255
256- config_auth(config_manager, auth_url, auth_user, auth_key)
257+ set_auth_config(config_manager, auth_url, auth_user, auth_key)
258diff --git a/src/maasserver/tests/test_commands_configauth.py b/src/maasserver/tests/test_commands_configauth.py
259index e4ab929..f3fb896 100644
260--- a/src/maasserver/tests/test_commands_configauth.py
261+++ b/src/maasserver/tests/test_commands_configauth.py
262@@ -145,6 +145,35 @@ class TestChangeAuthCommand(MAASServerTestCase):
263 'private-key', Config.objects.get_config('external_auth_key'))
264 self.read_input.assert_not_called()
265
266+ def test_configauth_json_empty(self):
267+ mock_print = self.patch(configauth, 'print')
268+ call_command('configauth', json=True)
269+ self.read_input.assert_not_called()
270+ [print_call] = mock_print.mock_calls
271+ _, [output], kwargs = print_call
272+ self.assertEqual({}, kwargs)
273+ self.assertEqual(
274+ {'external_auth_url': '', 'external_auth_user': '',
275+ 'external_auth_key': ''},
276+ json.loads(output))
277+
278+ def test_configauth_json_full(self):
279+ Config.objects.set_config(
280+ 'external_auth_url', 'http://idm.example.com/')
281+ Config.objects.set_config('external_auth_user', 'maas')
282+ Config.objects.set_config('external_auth_key', 'secret maas key')
283+ mock_print = self.patch(configauth, 'print')
284+ call_command('configauth', json=True)
285+ self.read_input.assert_not_called()
286+ [print_call] = mock_print.mock_calls
287+ _, [output], kwargs = print_call
288+ self.assertEqual({}, kwargs)
289+ self.assertEqual(
290+ {'external_auth_url': 'http://idm.example.com/',
291+ 'external_auth_user': 'maas',
292+ 'external_auth_key': 'secret maas key'},
293+ json.loads(output))
294+
295
296 class TestIsValidAuthSource(unittest.TestCase):
297
298diff --git a/src/metadataserver/models/scriptresult.py b/src/metadataserver/models/scriptresult.py
299index d817a0e..9cd657f 100644
300--- a/src/metadataserver/models/scriptresult.py
301+++ b/src/metadataserver/models/scriptresult.py
302@@ -145,7 +145,7 @@ class ScriptResult(CleanSave, TimestampedModel):
303 # Get an estimated runtime from previous runs.
304 for script_result in self.history.only(
305 'status', 'started', 'ended', 'script_id', 'script_name',
306- 'script_set_id', 'physical_blockdevice_id'):
307+ 'script_set_id', 'physical_blockdevice_id', 'created'):
308 # Only look at passed results when calculating an estimated
309 # runtime. Failed results may take longer or shorter than
310 # average. Don't use self.history.filter for this as the now

Subscribers

People subscribed via source and target branches