Merge ~ltrager/maas:lp1798471 into maas:master

[Lee Trager]

LP: #1798471 - Add option to enable SSH during VMware ESXi deployment

[Alberto Donato (ack)]

LGTM

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1798471 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6131/console
COMMIT: 2f3b4cd38141f6ef7d8197dd0be3274fdef8e61b

review: Needs Fixing

[Blake Rouse (blake-rouse)]

This seems very wierd as a one off option to enable SSH when it doesn't affect other images that all have SSH. Can VMware just not have SSH enabled all the time?

review: Needs Information

Unmerged commits

2f3b4cd... by Lee Trager on 2019-05-31

LP: #1798471 - Add option to enable SSH during VMware ESXi deployment

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index ab5ab67..48a436b 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -657,6 +657,9 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
         @param (boolean) "vcenter_registration" [required=false] If false, do
         not send globally defined VMware vCenter credentials to the machine.
 
+        @param (boolean) "enable_ssh" [required=false] Send runcmd's to enable
+        SSH(VMware ESXi only).
+
         @success (http-status-code) "200" 200
         @success (json) "success-json" A JSON object containing information
         about the deployed machine.
@@ -745,11 +748,15 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
                 raise MAASAPIBadRequest(
                     "Failed to render preseed: %s" % e)
 
-        if machine.osystem == 'esxi' and request.user.has_perm(
-                NodePermission.admin, machine):
-            if get_optional_param(
-                    request.POST, 'vcenter_registration', default=True,
-                    validator=StringBool):
+        if machine.osystem == 'esxi':
+            machine.enable_ssh = get_optional_param(
+                request.POST, 'enable_ssh', default=False,
+                validator=StringBool)
+            machine.save()
+            if (request.user.has_perm(NodePermission.admin, machine) and
+                    get_optional_param(
+                        request.POST, 'vcenter_registration', default=True,
+                        validator=StringBool)):
                 NodeMetadata.objects.update_or_create(
                     node=machine, key='vcenter_registration',
                     defaults={'value': 'True'})
diff --git a/src/maasserver/api/tests/test_machine.py b/src/maasserver/api/tests/test_machine.py
index 6eb2cd6..62c1b5d 100644
--- a/src/maasserver/api/tests/test_machine.py
+++ b/src/maasserver/api/tests/test_machine.py
@@ -826,6 +826,71 @@ class TestMachineAPI(APITestCase.ForUser):
                 bridge_all=True, bridge_fd=7,
                 bridge_stp=True, comment=None))
 
+    def test_POST_deploy_disables_ssh_on_esxi_by_default(self):
+        self.patch(node_module.Node, "_start")
+        self.patch(machines_module, "get_curtin_merged_config")
+        machine = factory.make_Node(
+            owner=self.user, interface=True,
+            power_type='manual',
+            status=NODE_STATUS.READY,
+            architecture=make_usable_architecture(self))
+        osystem = make_usable_osystem(self, 'esxi', ['6.7'])
+        distro_series = osystem['default_release']
+        response = self.client.post(
+            self.get_machine_uri(machine), {
+                'op': 'deploy',
+                'distro_series': distro_series,
+            })
+        self.assertEqual(
+            (http.client.OK, machine.system_id),
+            (response.status_code,
+             json_load_bytes(response.content)['system_id']))
+        self.assertFalse(reload_object(machine).enable_ssh)
+
+    def test_POST_deploy_enables_ssh_on_esxi(self):
+        self.patch(node_module.Node, "_start")
+        self.patch(machines_module, "get_curtin_merged_config")
+        machine = factory.make_Node(
+            owner=self.user, interface=True,
+            power_type='manual',
+            status=NODE_STATUS.READY,
+            architecture=make_usable_architecture(self))
+        osystem = make_usable_osystem(self, 'esxi', ['6.7'])
+        distro_series = osystem['default_release']
+        response = self.client.post(
+            self.get_machine_uri(machine), {
+                'op': 'deploy',
+                'distro_series': distro_series,
+                'enable_ssh': True,
+            })
+        self.assertEqual(
+            (http.client.OK, machine.system_id),
+            (response.status_code,
+             json_load_bytes(response.content)['system_id']))
+        self.assertTrue(reload_object(machine).enable_ssh)
+
+    def test_POST_deploy_enables_ssh_does_nothing_on_other_os(self):
+        self.patch(node_module.Node, "_start")
+        self.patch(machines_module, "get_curtin_merged_config")
+        machine = factory.make_Node(
+            owner=self.user, interface=True,
+            power_type='manual',
+            status=NODE_STATUS.READY,
+            architecture=make_usable_architecture(self))
+        osystem = make_usable_osystem(self)
+        distro_series = osystem['default_release']
+        response = self.client.post(
+            self.get_machine_uri(machine), {
+                'op': 'deploy',
+                'distro_series': distro_series,
+                'enable_ssh': True,
+            })
+        self.assertEqual(
+            (http.client.OK, machine.system_id),
+            (response.status_code,
+             json_load_bytes(response.content)['system_id']))
+        self.assertFalse(reload_object(machine).enable_ssh)
+
     def test_POST_deploy_stores_vcenter_registration_by_default(self):
         self.become_admin()
         self.patch(node_module.Node, "_start")
diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
index 32239ec..c95a564 100644
--- a/src/maasserver/node_action.py
+++ b/src/maasserver/node_action.py
@@ -481,6 +481,10 @@ class Deploy(NodeAction):
             self.node.osystem = configs['default_osystem']
             self.node.distro_series = configs['default_distro_series']
             self.node.save()
+
+        # SSH can only be enabled for VMware ESXi over the API.
+        self.node.enable_ssh = False
+
         try:
             self.node.hwe_kernel = validate_hwe_kernel(
                 hwe_kernel, self.node.min_hwe_kernel,
diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py
index 7b3e256..56375d0 100644
--- a/src/maasserver/tests/test_node_action.py
+++ b/src/maasserver/tests/test_node_action.py
@@ -707,7 +707,7 @@ class TestDeployAction(MAASServerTestCase):
         request.user = user
         node = factory.make_Node(
             interface=True, status=NODE_STATUS.READY,
-            power_type='manual', owner=None)
+            power_type='manual', owner=None, enable_ssh=True)
         mock_get_curtin_config = self.patch(
             node_action_module, 'get_curtin_config')
         mock_node_start = self.patch(node, 'start')
@@ -732,6 +732,7 @@ class TestDeployAction(MAASServerTestCase):
         self.expectThat(node.osystem, Equals("ubuntu"))
         self.expectThat(node.distro_series, Equals("bionic"))
         self.expectThat(node.install_kvm, Equals(True))
+        self.assertFalse(node.enable_ssh)
 
     def test_Deploy_raises_NodeActionError_on_install_kvm_if_os_missing(self):
         user = factory.make_admin()
diff --git a/src/metadataserver/tests/test_vendor_data.py b/src/metadataserver/tests/test_vendor_data.py
index 7d7a350..1bff5af 100644
--- a/src/metadataserver/tests/test_vendor_data.py
+++ b/src/metadataserver/tests/test_vendor_data.py
@@ -306,19 +306,42 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
 
     def test_does_nothing_if_not_vmware(self):
         mock_get_configs = self.patch(Config.objects, 'get_configs')
-        node = factory.make_Node(owner=factory.make_admin())
+        node = factory.make_Node(owner=factory.make_admin(), netboot=False)
         config = get_vendor_data(node)
         self.assertThat(mock_get_configs, MockNotCalled())
         self.assertDictEqual({}, config)
 
+    def test_does_nothing_if_netbooted(self):
+        mock_get_configs = self.patch(Config.objects, 'get_configs')
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_admin(), netboot=True)
+        config = get_vendor_data(node)
+        self.assertThat(mock_get_configs, MockNotCalled())
+        self.assertDictEqual({}, config)
+
+    def test_enables_ssh(self):
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_admin(), enable_ssh=True,
+            netboot=False)
+        config = get_vendor_data(node)
+        self.assertDictEqual(
+            {'runcmd': [
+                ['vim-cmd', 'hostsvc/enable_ssh'],
+                ['vim-cmd', 'hostsvc/start_ssh'],
+                ['vim-cmd', 'hostsvc/enable_esx_shell'],
+                ['vim-cmd', 'hostsvc/start_esx_shell'],
+            ]}, config)
+
     def test_returns_nothing_if_no_values_set(self):
-        node = factory.make_Node(osystem='esxi', owner=factory.make_admin())
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_admin(), netboot=False)
         node.nodemetadata_set.create(key='vcenter_registration', value='True')
         config = get_vendor_data(node)
         self.assertDictEqual({}, config)
 
     def test_returns_vcenter_yaml(self):
-        node = factory.make_Node(osystem='esxi', owner=factory.make_admin())
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_admin(), netboot=False)
         node.nodemetadata_set.create(key='vcenter_registration', value='True')
         vcenter = {
             'vcenter_server': factory.make_name('vcenter_server'),
@@ -337,7 +360,8 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
 
     def test_returns_vcenter_yaml_if_rbac_admin(self):
         rbac = self.useFixture(RBACEnabled())
-        node = factory.make_Node(osystem='esxi', owner=factory.make_User())
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_User(), netboot=False)
         node.nodemetadata_set.create(key='vcenter_registration', value='True')
         rbac.store.add_pool(node.pool)
         rbac.store.allow(node.owner.username, node.pool, 'admin-machines')
@@ -358,7 +382,8 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
 
     def test_returns_nothing_if_rbac_user(self):
         rbac = self.useFixture(RBACEnabled())
-        node = factory.make_Node(osystem='esxi', owner=factory.make_User())
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_User(), netboot=False)
         node.nodemetadata_set.create(key='vcenter_registration', value='True')
         rbac.store.add_pool(node.pool)
         rbac.store.allow(node.owner.username, node.pool, 'deploy-machines')
@@ -374,7 +399,7 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         self.assertDictEqual({}, config)
 
     def test_returns_nothing_if_no_user(self):
-        node = factory.make_Node(osystem='esxi')
+        node = factory.make_Node(osystem='esxi', netboot=False)
         for i in ['server', 'username', 'password', 'datacenter']:
             key = 'vcenter_%s' % i
             Config.objects.set_config(key, factory.make_name(key))
@@ -382,7 +407,8 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         self.assertDictEqual({}, config)
 
     def test_returns_nothing_if_user(self):
-        node = factory.make_Node(osystem='esxi', owner=factory.make_User())
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_User(), netboot=False)
         for i in ['server', 'username', 'password', 'datacenter']:
             key = 'vcenter_%s' % i
             Config.objects.set_config(key, factory.make_name(key))
@@ -390,7 +416,8 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         self.assertDictEqual({}, config)
 
     def test_returns_nothing_if_vcenter_registration_not_set(self):
-        node = factory.make_Node(osystem='esxi', owner=factory.make_admin())
+        node = factory.make_Node(
+            osystem='esxi', owner=factory.make_admin(), netboot=False)
         for i in ['server', 'username', 'password', 'datacenter']:
             key = 'vcenter_%s' % i
             Config.objects.set_config(key, factory.make_name(key))
diff --git a/src/metadataserver/vendor_data.py b/src/metadataserver/vendor_data.py
index 5010302..93ec9a6 100644
--- a/src/metadataserver/vendor_data.py
+++ b/src/metadataserver/vendor_data.py
@@ -35,7 +35,7 @@ def get_vendor_data(node):
         generate_rack_controller_configuration(node),
         generate_kvm_pod_configuration(node),
         generate_ephemeral_deployment_network_configuration(node),
-        generate_vcenter_configuration(node),
+        generate_esxi_configuration(node),
     ))
 
 
@@ -202,11 +202,21 @@ def generate_kvm_pod_configuration(node):
         yield "packages", packages
 
 
-def generate_vcenter_configuration(node):
+def generate_esxi_configuration(node):
     """Generate vendor config when deploying ESXi."""
-    if node.osystem != 'esxi':
+    if node.osystem != 'esxi' or node.netboot:
         # Only return vcenter credentials if vcenter is being deployed.
         return
+    # By default VMware ESXi disables SSH and strongly discourages users from
+    # enabling. Only enable if told to do so, otherwise keep image default.
+    if node.enable_ssh:
+        yield 'runcmd', [
+            ['vim-cmd', 'hostsvc/enable_ssh'],
+            ['vim-cmd', 'hostsvc/start_ssh'],
+            # Enable the local shell as well.
+            ['vim-cmd', 'hostsvc/enable_esx_shell'],
+            ['vim-cmd', 'hostsvc/start_esx_shell'],
+        ]
     if not node.owner or not node.owner.has_perm(NodePermission.admin, node):
         # VMware vCenter credentials are only given to machines deployed by
         # administrators.