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
diff --git a/src/maascli/snappy.py b/src/maascli/snappy.py
index 07bde83..08ff34b 100755
--- a/src/maascli/snappy.py
+++ b/src/maascli/snappy.py
@@ -15,6 +15,7 @@ import argparse
15from collections import OrderedDict15from collections import OrderedDict
16from contextlib import contextmanager16from contextlib import contextmanager
17import grp17import grp
18import json
18import os19import os
19import pwd20import pwd
20import random21import random
@@ -631,6 +632,25 @@ class cmd_init(SnappyCommand):
631 help=(632 help=(
632 "Skip the admin creation when initializing in 'all' mode."))633 "Skip the admin creation when initializing in 'all' mode."))
633 parser.add_argument(634 parser.add_argument(
635 '--enable-idm', default=False, action="store_true",
636 help=("Enable configuring the use of an external IDM server. "
637 "This feature is currently experimental. "
638 "If this isn't enabled, all --idm-* arguments "
639 "will be ignored."))
640 parser.add_argument(
641 '--idm-url', default=None, metavar='IDM_URL',
642 help=("The URL to the external IDM server to use for "
643 "authentication."))
644 parser.add_argument(
645 '--idm-user', default=None,
646 help="The username to access the IDM server API.")
647 parser.add_argument(
648 '--idm-key', default=None,
649 help="The private key to access the IDM server API.")
650 parser.add_argument(
651 '--idm-agent-file', type=argparse.FileType('r'),
652 help="Agent file containing IDM authentication information")
653 parser.add_argument(
634 '--admin-username', default=None, metavar='USERNAME',654 '--admin-username', default=None, metavar='USERNAME',
635 help="Username for the admin account.")655 help="Username for the admin account.")
636 parser.add_argument(656 parser.add_argument(
@@ -773,7 +793,13 @@ class cmd_init(SnappyCommand):
773 "Performing database migrations",793 "Performing database migrations",
774 migrate_db, capture=sys.stdout.isatty())794 migrate_db, capture=sys.stdout.isatty())
775 clear_line()795 clear_line()
776 if not options.skip_admin:796 if options.enable_idm:
797 print_msg('Configuring authentication')
798 configure_authentication(options)
799 auth_config = self._get_current_auth_config()
800 skip_create_admin = (
801 options.skip_admin or auth_config['external_auth_url'])
802 if not skip_create_admin:
777 self._create_admin_account(options)803 self._create_admin_account(options)
778 elif mode in ['region', 'region+rack']:804 elif mode in ['region', 'region+rack']:
779 # When in 'region' or 'region+rack' the migrations for the database805 # When in 'region' or 'region+rack' the migrations for the database
@@ -805,6 +831,28 @@ class cmd_init(SnappyCommand):
805 cmd.extend(['--ssh-import', options.admin_ssh_import])831 cmd.extend(['--ssh-import', options.admin_ssh_import])
806 subprocess.call(cmd)832 subprocess.call(cmd)
807833
834 def _get_current_auth_config(self):
835 cmd = [
836 os.path.join(os.environ['SNAP'], 'bin', 'maas-region'),
837 'configauth', '--json']
838 output = subprocess.check_output(cmd)
839 return json.loads(output)
840
841
842def configure_authentication(options):
843 cmd = [
844 os.path.join(os.environ['SNAP'], 'bin', 'maas-region'),
845 'configauth']
846 if options.idm_url is not None:
847 cmd.extend(['--idm-url', options.idm_url])
848 if options.idm_user is not None:
849 cmd.extend(['--idm-user', options.idm_user])
850 if options.idm_key is not None:
851 cmd.extend(['--idm-key', options.idm_key])
852 if options.idm_agent_file is not None:
853 cmd.extend(['--idm-agent-file', options.idm_agent_file.name])
854 subprocess.call(cmd)
855
808856
809class cmd_config(SnappyCommand):857class cmd_config(SnappyCommand):
810 """View or change controller configuration."""858 """View or change controller configuration."""
diff --git a/src/maascli/tests/test_snappy.py b/src/maascli/tests/test_snappy.py
index f2adf70..2af352e 100644
--- a/src/maascli/tests/test_snappy.py
+++ b/src/maascli/tests/test_snappy.py
@@ -9,11 +9,16 @@ import os
9import random9import random
10import signal10import signal
11import subprocess11import subprocess
12import tempfile
12from textwrap import dedent13from textwrap import dedent
13import time14import time
14from unittest.mock import MagicMock15from unittest.mock import (
16 MagicMock,
17 patch,
18)
1519
16from maascli import snappy20from maascli import snappy
21from maascli.parser import ArgumentParser
17from maastesting.factory import factory22from maastesting.factory import factory
18from maastesting.matchers import MockCalledOnceWith23from maastesting.matchers import MockCalledOnceWith
19from maastesting.testcase import MAASTestCase24from maastesting.testcase import MAASTestCase
@@ -138,6 +143,96 @@ class TestHelpers(MAASTestCase):
138 self.assertEqual('none', snappy.get_current_mode())143 self.assertEqual('none', snappy.get_current_mode())
139144
140145
146class TestConfigureAuthentication(MAASTestCase):
147
148 def setUp(self):
149 super().setUp()
150 self.maas_bin_path = 'snap-path/bin/maas-region'
151 self.mock_subprocess = self.patch(snappy, 'subprocess')
152 self.mock_environ = patch.dict(
153 snappy.os.environ, {'SNAP': 'snap-path'}, clear=True)
154 self.mock_environ.start()
155 self.parser = ArgumentParser()
156 snappy.cmd_init(self.parser)
157
158 def tearDown(self):
159 self.mock_subprocess.stop()
160 self.mock_environ.stop()
161 super().tearDown()
162
163 def test_no_options(self):
164 options = self.parser.parse_args([])
165 snappy.configure_authentication(options)
166 [config_call] = self.mock_subprocess.mock_calls
167 method, args, kwargs = config_call
168 self.assertEqual('call', method)
169 self.assertEqual(([self.maas_bin_path, 'configauth'],), args)
170 self.assertEqual({}, kwargs)
171
172 def test_idm_url(self):
173 config_auth_args = ['--idm-url', 'http://idm.example.com/']
174 options = self.parser.parse_args(config_auth_args)
175 snappy.configure_authentication(options)
176 [config_call] = self.mock_subprocess.mock_calls
177 method, args, kwargs = config_call
178 self.assertEqual('call', method)
179 self.assertEqual(
180 ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
181 self.assertEqual({}, kwargs)
182
183 def test_idm_user(self):
184 config_auth_args = ['--idm-user', 'some-user']
185 options = self.parser.parse_args(config_auth_args)
186 snappy.configure_authentication(options)
187 [config_call] = self.mock_subprocess.mock_calls
188 method, args, kwargs = config_call
189 self.assertEqual('call', method)
190 self.assertEqual(
191 ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
192 self.assertEqual({}, kwargs)
193
194 def test_idm_key(self):
195 config_auth_args = ['--idm-key', 'some-key']
196 options = self.parser.parse_args(config_auth_args)
197 snappy.configure_authentication(options)
198 [config_call] = self.mock_subprocess.mock_calls
199 method, args, kwargs = config_call
200 self.assertEqual('call', method)
201 self.assertEqual(
202 ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
203 self.assertEqual({}, kwargs)
204
205 def test_idm_agent_file(self):
206 _, agent_file_path = tempfile.mkstemp()
207 self.addCleanup(os.remove, agent_file_path)
208 config_auth_args = ['--idm-agent-file', agent_file_path]
209 options = self.parser.parse_args(config_auth_args)
210 snappy.configure_authentication(options)
211 [config_call] = self.mock_subprocess.mock_calls
212 method, args, kwargs = config_call
213 self.assertEqual('call', method)
214 self.assertEqual(
215 ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
216 self.assertEqual({}, kwargs)
217
218 def test_full(self):
219 _, agent_file = tempfile.mkstemp()
220 self.addCleanup(os.remove, agent_file)
221 config_auth_args = [
222 '--idm-url', 'http://idm.example.com/',
223 '--idm-user', 'idm-user',
224 '--idm-key', 'idm-key',
225 '--idm-agent-file', agent_file]
226 options = self.parser.parse_args(config_auth_args)
227 snappy.configure_authentication(options)
228 [config_call] = self.mock_subprocess.mock_calls
229 method, args, kwargs = config_call
230 self.assertEqual('call', method)
231 self.assertEqual(
232 ([self.maas_bin_path, 'configauth'] + config_auth_args,), args)
233 self.assertEqual({}, kwargs)
234
235
141class TestRenderSupervisord(MAASTestCase):236class TestRenderSupervisord(MAASTestCase):
142237
143 TEST_TEMPLATE = dedent("""\238 TEST_TEMPLATE = dedent("""\
diff --git a/src/maasserver/management/commands/configauth.py b/src/maasserver/management/commands/configauth.py
index 2e8df6c..5d7c7c4 100644
--- a/src/maasserver/management/commands/configauth.py
+++ b/src/maasserver/management/commands/configauth.py
@@ -58,10 +58,16 @@ def is_valid_auth_url(auth_url):
58 return True58 return True
5959
6060
61def config_auth(config_manager, auth_url, auth_user, auth_key):61def get_auth_config(config_manager):
62 config_manager.set_config('external_auth_url', auth_url)62 config_keys = [
63 config_manager.set_config('external_auth_user', auth_user)63 'external_auth_url', 'external_auth_user', 'external_auth_key']
64 config_manager.set_config('external_auth_key', auth_key)64 return {key: config_manager.get_config(key) for key in config_keys}
65
66
67def set_auth_config(config_manager, auth_url, auth_user, auth_key):
68 config_manager.set_config('external_auth_url', auth_url)
69 config_manager.set_config('external_auth_user', auth_user)
70 config_manager.set_config('external_auth_key', auth_key)
6571
6672
67class Command(BaseCommand):73class Command(BaseCommand):
@@ -82,16 +88,23 @@ class Command(BaseCommand):
82 parser.add_argument(88 parser.add_argument(
83 '--idm-agent-file', type=argparse.FileType('r'),89 '--idm-agent-file', type=argparse.FileType('r'),
84 help="Agent file containing IDM authentication information")90 help="Agent file containing IDM authentication information")
91 parser.add_argument(
92 '--json', action='store_true', default=False,
93 help="Return the current authentication configuration as JSON")
8594
86 def handle(self, *args, **options):95 def handle(self, *args, **options):
87 config_manager = Config.objects.db_manager(DEFAULT_DB_ALIAS)96 config_manager = Config.objects.db_manager(DEFAULT_DB_ALIAS)
8897
98 if options.get('json'):
99 print(json.dumps(get_auth_config(config_manager)))
100 return
101
89 auth_url, auth_user, auth_key = None, '', ''102 auth_url, auth_user, auth_key = None, '', ''
90103
91 agent_file = options.get('idm_agent_file')104 agent_file = options.get('idm_agent_file')
92 if agent_file:105 if agent_file:
93 auth_url, auth_user, auth_key = read_agent_file(agent_file)106 auth_url, auth_user, auth_key = read_agent_file(agent_file)
94 config_auth(config_manager, auth_url, auth_user, auth_key)107 set_auth_config(config_manager, auth_url, auth_user, auth_key)
95 return108 return
96109
97 auth_url = options.get('idm_url')110 auth_url = options.get('idm_url')
@@ -111,4 +124,4 @@ class Command(BaseCommand):
111 if not auth_key:124 if not auth_key:
112 auth_key = read_input("Private key for IDM API access: ")125 auth_key = read_input("Private key for IDM API access: ")
113126
114 config_auth(config_manager, auth_url, auth_user, auth_key)127 set_auth_config(config_manager, auth_url, auth_user, auth_key)
diff --git a/src/maasserver/tests/test_commands_configauth.py b/src/maasserver/tests/test_commands_configauth.py
index e4ab929..f3fb896 100644
--- a/src/maasserver/tests/test_commands_configauth.py
+++ b/src/maasserver/tests/test_commands_configauth.py
@@ -145,6 +145,35 @@ class TestChangeAuthCommand(MAASServerTestCase):
145 'private-key', Config.objects.get_config('external_auth_key'))145 'private-key', Config.objects.get_config('external_auth_key'))
146 self.read_input.assert_not_called()146 self.read_input.assert_not_called()
147147
148 def test_configauth_json_empty(self):
149 mock_print = self.patch(configauth, 'print')
150 call_command('configauth', json=True)
151 self.read_input.assert_not_called()
152 [print_call] = mock_print.mock_calls
153 _, [output], kwargs = print_call
154 self.assertEqual({}, kwargs)
155 self.assertEqual(
156 {'external_auth_url': '', 'external_auth_user': '',
157 'external_auth_key': ''},
158 json.loads(output))
159
160 def test_configauth_json_full(self):
161 Config.objects.set_config(
162 'external_auth_url', 'http://idm.example.com/')
163 Config.objects.set_config('external_auth_user', 'maas')
164 Config.objects.set_config('external_auth_key', 'secret maas key')
165 mock_print = self.patch(configauth, 'print')
166 call_command('configauth', json=True)
167 self.read_input.assert_not_called()
168 [print_call] = mock_print.mock_calls
169 _, [output], kwargs = print_call
170 self.assertEqual({}, kwargs)
171 self.assertEqual(
172 {'external_auth_url': 'http://idm.example.com/',
173 'external_auth_user': 'maas',
174 'external_auth_key': 'secret maas key'},
175 json.loads(output))
176
148177
149class TestIsValidAuthSource(unittest.TestCase):178class TestIsValidAuthSource(unittest.TestCase):
150179
diff --git a/src/metadataserver/models/scriptresult.py b/src/metadataserver/models/scriptresult.py
index d817a0e..9cd657f 100644
--- a/src/metadataserver/models/scriptresult.py
+++ b/src/metadataserver/models/scriptresult.py
@@ -145,7 +145,7 @@ class ScriptResult(CleanSave, TimestampedModel):
145 # Get an estimated runtime from previous runs.145 # Get an estimated runtime from previous runs.
146 for script_result in self.history.only(146 for script_result in self.history.only(
147 'status', 'started', 'ended', 'script_id', 'script_name',147 'status', 'started', 'ended', 'script_id', 'script_name',
148 'script_set_id', 'physical_blockdevice_id'):148 'script_set_id', 'physical_blockdevice_id', 'created'):
149 # Only look at passed results when calculating an estimated149 # Only look at passed results when calculating an estimated
150 # runtime. Failed results may take longer or shorter than150 # runtime. Failed results may take longer or shorter than
151 # average. Don't use self.history.filter for this as the now151 # average. Don't use self.history.filter for this as the now

Subscribers

People subscribed via source and target branches