launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10389
[Merge] lp:~rvb/maas/store-nodegroup-key into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/store-nodegroup-key into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/store-nodegroup-key/+merge/117272
This branch adds a new column on the nodegroup objects to store the key used to communicate with the dhcp server.
I considered using a TextField but the key being a HMAC-MD5, a char field of 255 will do just fine and give us some space if the key get bigger. (With the size we use today, the size of the keys is 88.)
Note that I didn't set that field to be unique because, strictly speaking, that's not required: nothing prevents us from using the same key for two dhcp servers… but since these keys are randomly generated this is very unlikely.
--
https://code.launchpad.net/~rvb/maas/store-nodegroup-key/+merge/117272
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/store-nodegroup-key into lp:maas.
=== added file 'src/maasserver/migrations/0015_add_dhcp_key_to_nodegroup.py'
--- src/maasserver/migrations/0015_add_dhcp_key_to_nodegroup.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/migrations/0015_add_dhcp_key_to_nodegroup.py 2012-07-30 14:04:35 +0000
@@ -0,0 +1,158 @@
+# encoding: utf-8
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+class Migration(SchemaMigration):
+
+ def forwards(self, orm):
+
+ # Adding field 'NodeGroup.dhcp_key'
+ db.add_column(u'maasserver_nodegroup', 'dhcp_key', self.gf('django.db.models.fields.CharField')(default=u'zissygEDUel+1AVVQ95jG3GHSaSWLJfcArpbiRkgpz9em5jmZ5oASegWSFYkOaXUrY455KU7s6BqwB/PeeJ9zg==', max_length=255), keep_default=False)
+
+
+ def backwards(self, orm):
+
+ # Deleting field 'NodeGroup.dhcp_key'
+ db.delete_column(u'maasserver_nodegroup', 'dhcp_key')
+
+
+ models = {
+ 'auth.group': {
+ 'Meta': {'object_name': 'Group'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+ 'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+ },
+ 'auth.permission': {
+ 'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+ 'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+ 'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+ },
+ 'auth.user': {
+ 'Meta': {'object_name': 'User'},
+ 'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+ 'email': ('django.db.models.fields.EmailField', [], {'unique': 'True', 'max_length': '75', 'blank': 'True'}),
+ 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+ 'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+ 'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+ 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+ 'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+ 'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+ 'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+ },
+ 'contenttypes.contenttype': {
+ 'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+ 'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+ },
+ u'maasserver.config': {
+ 'Meta': {'object_name': 'Config'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+ 'value': ('maasserver.fields.JSONObjectField', [], {'null': 'True'})
+ },
+ u'maasserver.dhcplease': {
+ 'Meta': {'object_name': 'DHCPLease'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'ip': ('django.db.models.fields.IPAddressField', [], {'unique': 'True', 'max_length': '15'}),
+ 'mac': ('maasserver.fields.MACAddressField', [], {}),
+ 'nodegroup': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['maasserver.NodeGroup']"})
+ },
+ u'maasserver.filestorage': {
+ 'Meta': {'object_name': 'FileStorage'},
+ 'data': ('django.db.models.fields.files.FileField', [], {'max_length': '255'}),
+ 'filename': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '200'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+ },
+ u'maasserver.macaddress': {
+ 'Meta': {'object_name': 'MACAddress'},
+ 'created': ('django.db.models.fields.DateTimeField', [], {}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'mac_address': ('maasserver.fields.MACAddressField', [], {'unique': 'True'}),
+ 'node': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['maasserver.Node']"}),
+ 'updated': ('django.db.models.fields.DateTimeField', [], {})
+ },
+ u'maasserver.node': {
+ 'Meta': {'object_name': 'Node'},
+ 'after_commissioning_action': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+ 'architecture': ('django.db.models.fields.CharField', [], {'default': "u'i386'", 'max_length': '10'}),
+ 'created': ('django.db.models.fields.DateTimeField', [], {}),
+ 'error': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '255', 'blank': 'True'}),
+ 'hostname': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '255', 'blank': 'True'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'netboot': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+ 'nodegroup': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': u"orm['maasserver.NodeGroup']", 'null': 'True', 'blank': 'True'}),
+ 'owner': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}),
+ 'power_parameters': ('maasserver.fields.JSONObjectField', [], {'default': "u''", 'blank': 'True'}),
+ 'power_type': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '10', 'blank': 'True'}),
+ 'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '10'}),
+ 'system_id': ('django.db.models.fields.CharField', [], {'default': "u'node-01403e66-da4d-11e1-a1fa-9c4e363b1c94'", 'unique': 'True', 'max_length': '41'}),
+ 'token': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Token']", 'null': 'True'}),
+ 'updated': ('django.db.models.fields.DateTimeField', [], {})
+ },
+ u'maasserver.nodegroup': {
+ 'Meta': {'object_name': 'NodeGroup'},
+ 'api_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '18'}),
+ 'api_token': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Token']", 'unique': 'True'}),
+ 'broadcast_ip': ('django.db.models.fields.IPAddressField', [], {'default': "u''", 'max_length': '15', 'null': 'True', 'blank': 'True'}),
+ 'created': ('django.db.models.fields.DateTimeField', [], {}),
+ 'dhcp_key': ('django.db.models.fields.CharField', [], {'default': "u'zissygEDUel+1AVVQ95jG3GHSaSWLJfcArpbiRkgpz9em5jmZ5oASegWSFYkOaXUrY455KU7s6BqwB/PeeJ9zg=='", 'max_length': '255'}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'ip_range_high': ('django.db.models.fields.IPAddressField', [], {'default': "u''", 'max_length': '15', 'unique': 'True', 'null': 'True', 'blank': 'True'}),
+ 'ip_range_low': ('django.db.models.fields.IPAddressField', [], {'default': "u''", 'max_length': '15', 'unique': 'True', 'null': 'True', 'blank': 'True'}),
+ 'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+ 'router_ip': ('django.db.models.fields.IPAddressField', [], {'default': "u''", 'max_length': '15', 'null': 'True', 'blank': 'True'}),
+ 'subnet_mask': ('django.db.models.fields.IPAddressField', [], {'default': "u''", 'max_length': '15', 'null': 'True', 'blank': 'True'}),
+ 'updated': ('django.db.models.fields.DateTimeField', [], {}),
+ 'worker_ip': ('django.db.models.fields.IPAddressField', [], {'unique': 'True', 'max_length': '15'})
+ },
+ u'maasserver.sshkey': {
+ 'Meta': {'unique_together': "((u'user', u'key'),)", 'object_name': 'SSHKey'},
+ 'created': ('django.db.models.fields.DateTimeField', [], {}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'key': ('django.db.models.fields.TextField', [], {}),
+ 'updated': ('django.db.models.fields.DateTimeField', [], {}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+ },
+ u'maasserver.userprofile': {
+ 'Meta': {'object_name': 'UserProfile'},
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'})
+ },
+ 'piston.consumer': {
+ 'Meta': {'object_name': 'Consumer'},
+ 'description': ('django.db.models.fields.TextField', [], {}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'key': ('django.db.models.fields.CharField', [], {'max_length': '18'}),
+ 'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+ 'secret': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
+ 'status': ('django.db.models.fields.CharField', [], {'default': "'pending'", 'max_length': '16'}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "'consumers'", 'null': 'True', 'to': "orm['auth.User']"})
+ },
+ 'piston.token': {
+ 'Meta': {'object_name': 'Token'},
+ 'callback': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+ 'callback_confirmed': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'consumer': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Consumer']"}),
+ 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+ 'is_approved': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+ 'key': ('django.db.models.fields.CharField', [], {'max_length': '18'}),
+ 'secret': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
+ 'timestamp': ('django.db.models.fields.IntegerField', [], {'default': '1343656006L'}),
+ 'token_type': ('django.db.models.fields.IntegerField', [], {}),
+ 'user': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "'tokens'", 'null': 'True', 'to': "orm['auth.User']"}),
+ 'verifier': ('django.db.models.fields.CharField', [], {'max_length': '10'})
+ }
+ }
+
+ complete_apps = ['maasserver']
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py 2012-07-30 13:51:10 +0000
+++ src/maasserver/models/nodegroup.py 2012-07-30 14:04:35 +0000
@@ -22,6 +22,7 @@
Manager,
)
from maasserver import DefaultMeta
+from provisioningserver.omshell import generate_omapi_key
from maasserver.models.timestampedmodel import TimestampedModel
from piston.models import (
KEY_SIZE,
@@ -116,6 +117,9 @@
worker_ip = IPAddressField(null=False, editable=True, unique=True)
# DHCP server settings.
+ dhcp_key = CharField(
+ null=False, blank=False, editable=False, max_length=255,
+ default=generate_omapi_key())
subnet_mask = IPAddressField(
editable=True, unique=False, blank=True, null=True, default='')
broadcast_ip = IPAddressField(
=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py 2012-07-30 11:31:38 +0000
+++ src/maasserver/tests/test_nodegroup.py 2012-07-30 14:04:35 +0000
@@ -163,3 +163,7 @@
ip_range_high=factory.getRandomIPAddress(),
)
self.assertTrue(nodegroup.is_dhcp_enabled())
+
+ def test_dhcp_key_is_autogenerated(self):
+ nodegroup = factory.make_node_group()
+ self.assertIsNotNone(nodegroup.dhcp_key)
=== modified file 'src/provisioningserver/omshell.py'
--- src/provisioningserver/omshell.py 2012-07-25 04:00:49 +0000
+++ src/provisioningserver/omshell.py 2012-07-30 14:04:35 +0000
@@ -13,17 +13,66 @@
__metaclass__ = type
__all__ = [
+ "generate_omapi_key",
"Omshell",
]
+import os
+import shutil
from subprocess import (
CalledProcessError,
+ check_output,
PIPE,
Popen,
)
+from tempfile import mkdtemp
from textwrap import dedent
+def call_dnssec_keygen(tmpdir):
+ return check_output(
+ ['dnssec-keygen', '-r', '/dev/urandom', '-a', 'HMAC-MD5',
+ '-b', '512', '-n', 'HOST', '-K', tmpdir, '-q', 'omapi_key'])
+
+
+def generate_omapi_key():
+ """Generate a HMAC-MD5 key by calling out to the dnssec-keygen tool.
+
+ :return: The shared key suitable for OMAPI access.
+ :type: string
+ """
+ # dnssec-keygen writes out files to a specified directory, so we
+ # need to make a temp directory for that.
+
+ # mkdtemp() says it will return a directory that is readable,
+ # writable, and searchable only by the creating user ID.
+ try:
+ tmpdir = mkdtemp()
+ key_id = call_dnssec_keygen(tmpdir)
+
+ # Locate the file that was written and strip out the Key: field in
+ # it.
+ if not key_id:
+ raise AssertionError("dnssec-keygen didn't generate anything")
+ key_id = key_id.strip() # Remove trailing newline.
+ key_file_name = os.path.join(tmpdir, key_id + '.private')
+ with open(key_file_name, 'rb') as f:
+ key_file = f.read()
+
+ for line in key_file.splitlines():
+ try:
+ field, value = line.split(":")
+ except ValueError:
+ continue
+ if field == "Key":
+ return value.strip()
+
+ raise AssertionError(
+ "Key field not found in output from dnssec-keygen")
+ finally:
+ shutil.rmtree(tmpdir)
+
+
class Omshell:
"""Wrap up the omshell utility in Python.
=== modified file 'src/provisioningserver/tests/test_omshell.py'
--- src/provisioningserver/tests/test_omshell.py 2012-07-25 04:00:49 +0000
+++ src/provisioningserver/tests/test_omshell.py 2012-07-30 14:04:35 +0000
@@ -12,14 +12,23 @@
__metaclass__ = type
__all__ = []
+import os
from subprocess import CalledProcessError
+import tempfile
from textwrap import dedent
from maastesting.factory import factory
from maastesting.fakemethod import FakeMethod
from maastesting.testcase import TestCase
-from provisioningserver.omshell import Omshell
-from testtools.matchers import MatchesStructure
+from provisioningserver import omshell
+from provisioningserver.omshell import (
+ generate_omapi_key,
+ Omshell,
+ )
+from testtools.matchers import (
+ EndsWith,
+ MatchesStructure,
+ )
class TestOmshell(TestCase):
@@ -135,3 +144,32 @@
exc = self.assertRaises(
CalledProcessError, shell.remove, ip_address)
self.assertEqual(random_output, exc.output)
+
+
+class Test_generate_omapi_key(TestCase):
+ """Tests for omshell.generate_omapi_key"""
+
+ def test_generate_omapi_key_returns_a_key(self):
+ key = generate_omapi_key()
+ # Could test for != None here, but the keys seem to end in == so
+ # that's a better check that the script was actually run and
+ # produced output.
+ self.assertThat(key, EndsWith("=="))
+
+ def test_generate_omapi_key_leaves_no_temp_files(self):
+ existing_file_count = os.listdir(tempfile.gettempdir())
+ generate_omapi_key()
+ new_file_count = os.listdir(tempfile.gettempdir())
+ self.assertEqual(existing_file_count, new_file_count)
+
+ def test_generate_omapi_key_raises_assertionerror_on_no_output(self):
+ self.patch(omshell, 'call_dnssec_keygen', FakeMethod())
+ self.assertRaises(AssertionError, generate_omapi_key)
+
+ def test_generate_omapi_key_raises_assertionerror_on_bad_output(self):
+ def returns_junk(tmpdir):
+ factory.make_file(tmpdir, "bad.private")
+ return "bad"
+
+ self.patch(omshell, 'call_dnssec_keygen', returns_junk)
+ self.assertRaises(AssertionError, generate_omapi_key)
