Merge lp:~blake-rouse/maas/fix-1445942-1.8 into lp:maas/1.8

[Blake Rouse]

Validate the osystem and distro_series when using the deploy action. Strip the ending '*' on releases that accept a license key.

Backport: https://code.launchpad.net/~blake-rouse/maas/fix-1445942/+merge/268486

[Blake Rouse (blake-rouse)]

Self-approving backport.

review: Approve

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2015-05-28 15:32:21 +0000
+++ src/maasserver/node_action.py	2015-08-19 20:52:03 +0000
@@ -33,6 +33,7 @@
 from collections import OrderedDict
 
 from crochet import TimeoutError
+from django.core.exceptions import ValidationError
 from maasserver import locks
 from maasserver.enum import (
     NODE_PERMISSION,
@@ -49,6 +50,7 @@
     is_failed_status,
     NON_MONITORED_STATUSES,
 )
+from maasserver.utils.osystems import validate_osystem_and_distro_series
 from metadataserver.enum import RESULT_TYPE
 from metadataserver.models.noderesult import NodeResult
 from provisioningserver.rpc.exceptions import (
@@ -296,11 +298,13 @@
             with locks.node_acquire:
                 self.node.acquire(self.user, token=None)
 
-        # Set the osystem in distro_series if provided and not empty.
         if osystem and distro_series:
-            self.node.osystem = osystem
-            self.node.distro_series = distro_series
-            self.node.save()
+            try:
+                self.node.osystem, self.node.distro_series = (
+                    validate_osystem_and_distro_series(osystem, distro_series))
+                self.node.save()
+            except ValidationError as e:
+                raise NodeActionError(e.message)
 
         try:
             self.node.start(self.user)

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2015-05-27 20:44:21 +0000
+++ src/maasserver/tests/test_node_action.py	2015-08-19 20:52:03 +0000
@@ -48,7 +48,10 @@
 )
 from maasserver.testing.factory import factory
 from maasserver.testing.orm import reload_object
-from maasserver.testing.osystems import make_osystem_with_releases
+from maasserver.testing.osystems import (
+    make_osystem_with_releases,
+    make_usable_osystem,
+)
 from maasserver.testing.testcase import (
     MAASServerTestCase,
     MAASTransactionServerTestCase,
@@ -422,21 +425,59 @@
         self.assertThat(
             node_start, MockCalledOnceWith(user))
 
+    def test_Deploy_raises_NodeActionError_for_invalid_os(self):
+        user = factory.make_User()
+        node = factory.make_Node(
+            mac=True, status=NODE_STATUS.ALLOCATED,
+            power_type='ether_wake', owner=user)
+        self.patch(node, 'start')
+        os_name = factory.make_name("os")
+        release_name = factory.make_name("release")
+        extra = {
+            "osystem": os_name,
+            "distro_series": release_name,
+        }
+        error = self.assertRaises(
+            NodeActionError, Deploy(node, user).execute, **extra)
+        self.assertEquals(
+            "%s is not a support operating system." % os_name,
+            error.message)
+
     def test_Deploy_sets_osystem_and_series(self):
         user = factory.make_User()
         node = factory.make_Node(
             mac=True, status=NODE_STATUS.ALLOCATED,
             power_type='ether_wake', owner=user)
         self.patch(node, 'start')
-        osystem = make_osystem_with_releases(self)
-        extra = {
-            "osystem": osystem["name"],
-            "distro_series": osystem["releases"][0]["name"],
-        }
-        Deploy(node, user).execute(**extra)
-        self.expectThat(node.osystem, Equals(osystem["name"]))
-        self.expectThat(
-            node.distro_series, Equals(osystem["releases"][0]["name"]))
+        osystem = make_usable_osystem(self)
+        os_name = osystem["name"]
+        release_name = osystem["releases"][0]["name"]
+        extra = {
+            "osystem": os_name,
+            "distro_series": release_name
+        }
+        Deploy(node, user).execute(**extra)
+        self.expectThat(node.osystem, Equals(os_name))
+        self.expectThat(
+            node.distro_series, Equals(release_name))
+
+    def test_Deploy_sets_osystem_and_series_strips_license_key_token(self):
+        user = factory.make_User()
+        node = factory.make_Node(
+            mac=True, status=NODE_STATUS.ALLOCATED,
+            power_type='ether_wake', owner=user)
+        self.patch(node, 'start')
+        osystem = make_usable_osystem(self)
+        os_name = osystem["name"]
+        release_name = osystem["releases"][0]["name"]
+        extra = {
+            "osystem": os_name,
+            "distro_series": release_name + '*'
+        }
+        Deploy(node, user).execute(**extra)
+        self.expectThat(node.osystem, Equals(os_name))
+        self.expectThat(
+            node.distro_series, Equals(release_name))
 
     def test_Deploy_doesnt_set_osystem_and_series_if_os_missing(self):
         user = factory.make_User()

=== modified file 'src/maasserver/utils/osystems.py'
--- src/maasserver/utils/osystems.py	2015-06-10 11:35:04 +0000
+++ src/maasserver/utils/osystems.py	2015-08-19 20:52:03 +0000
@@ -25,6 +25,7 @@
 
 from operator import itemgetter
 
+from django.core.exceptions import ValidationError
 from maasserver.clusterrpc.osystems import gen_all_known_operating_systems
 
 
@@ -176,3 +177,27 @@
             for release in releases
             if release['can_commission']
             ]
+
+
+def validate_osystem_and_distro_series(osystem, distro_series):
+    """Validate `osystem` and `distro_series` are valid choices."""
+    if '/' in distro_series:
+        series_os, release = distro_series.split('/', 1)
+        if series_os != osystem:
+            raise ValidationError(
+                "%s in distro_series does not match with "
+                "operating system %s." % (distro_series, osystem))
+    else:
+        release = distro_series
+    release = release.replace('*', '')
+    usable_osystems = list_all_usable_osystems()
+    found_osystem = get_osystem_from_osystems(usable_osystems, osystem)
+    if found_osystem is None:
+        raise ValidationError(
+            "%s is not a support operating system." % osystem)
+    found_release = get_release_from_osystem(found_osystem, release)
+    if found_release is None:
+        raise ValidationError(
+            "%s/%s is not a support operating system and release "
+            "combination." % (osystem, release))
+    return osystem, release

=== modified file 'src/maasserver/utils/tests/test_osystems.py'
--- src/maasserver/utils/tests/test_osystems.py	2015-05-21 09:41:57 +0000
+++ src/maasserver/utils/tests/test_osystems.py	2015-08-19 20:52:03 +0000
@@ -17,11 +17,13 @@
 from operator import itemgetter
 import random
 
+from django.core.exceptions import ValidationError
 from maasserver.clusterrpc.testing.osystems import (
     make_rpc_osystem,
     make_rpc_release,
 )
 from maasserver.testing.factory import factory
+from maasserver.testing.osystems import make_usable_osystem
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import osystems as osystems_module
 from maasserver.utils.osystems import (
@@ -33,6 +35,7 @@
     list_commissioning_choices,
     list_osystem_choices,
     list_release_choices,
+    validate_osystem_and_distro_series,
 )
 
 
@@ -253,3 +256,43 @@
             for release in comm_releases
             ]
         self.assertEqual(choices, list_commissioning_choices([osystem]))
+
+
+class TestValidateOsystemAndDistroSeries(MAASServerTestCase):
+
+    def test__raises_error_of_osystem_and_distro_series_dont_match(self):
+        os = factory.make_name("os")
+        release = "%s/%s" % (
+            factory.make_name("os"), factory.make_name("release"))
+        error = self.assertRaises(
+            ValidationError, validate_osystem_and_distro_series, os, release)
+        self.assertEquals(
+            "%s in distro_series does not match with "
+            "operating system %s." % (release, os), error.message)
+
+    def test__raises_error_if_not_supported_osystem(self):
+        os = factory.make_name("os")
+        release = factory.make_name("release")
+        error = self.assertRaises(
+            ValidationError, validate_osystem_and_distro_series, os, release)
+        self.assertEquals(
+            "%s is not a support operating system." % os,
+            error.message)
+
+    def test__raises_error_if_not_supported_release(self):
+        osystem = make_usable_osystem(self)
+        release = factory.make_name("release")
+        error = self.assertRaises(
+            ValidationError, validate_osystem_and_distro_series,
+            osystem['name'], release)
+        self.assertEquals(
+            "%s/%s is not a support operating system and release "
+            "combination." % (osystem['name'], release),
+            error.message)
+
+    def test__returns_osystem_and_release_with_license_key_stripped(self):
+        osystem = make_usable_osystem(self)
+        release = osystem['default_release']
+        self.assertEquals(
+            (osystem['name'], release),
+            validate_osystem_and_distro_series(osystem['name'], release + '*'))

=== modified file 'src/maasserver/websockets/handlers/tests/test_node.py'
--- src/maasserver/websockets/handlers/tests/test_node.py	2015-05-28 15:07:01 +0000
+++ src/maasserver/websockets/handlers/tests/test_node.py	2015-08-19 20:52:03 +0000
@@ -37,7 +37,7 @@
 )
 from maasserver.testing.factory import factory
 from maasserver.testing.orm import reload_object
-from maasserver.testing.osystems import make_osystem_with_releases
+from maasserver.testing.osystems import make_usable_osystem
 from maasserver.testing.testcase import (
     MAASServerTestCase,
     MAASTransactionServerTestCase,
@@ -719,7 +719,7 @@
         user = factory.make_User()
         factory.make_SSHKey(user)
         node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=user)
-        osystem = make_osystem_with_releases(self)
+        osystem = make_usable_osystem(self)
         handler = NodeHandler(user, {})
         handler.action({
             "system_id": node.system_id,