Repository: softlayer/softlayer-python

Issue:
`slcli globalip assign` should only take in an IP as an argument
`slcli globalip assign 12345678 9.111.123.456 ` this is the expected syntax since you can not route global ips to subnets/devices/etc.

Should still be able to support the `--target` and `--target-id` options of course. 
Update the help text to explain this a bit as well.

This command changed in #1614 

Discussion:

Patch:
diff --git a/SoftLayer/CLI/dns/zone_delete.py b/SoftLayer/CLI/dns/zone_delete.py
index 83eb1127..cca4c9c9 100644
--- a/SoftLayer/CLI/dns/zone_delete.py
+++ b/SoftLayer/CLI/dns/zone_delete.py
@@ -17,6 +17,7 @@ def cli(env, zone):
     """Delete zone.
 
     Example::
+
         slcli dns zone-delete ibm.com
         This command deletes a zone that is named ibm.com
 """
diff --git a/SoftLayer/CLI/formatting.py b/SoftLayer/CLI/formatting.py
index c4c28463..0e51eb30 100644
--- a/SoftLayer/CLI/formatting.py
+++ b/SoftLayer/CLI/formatting.py
@@ -254,8 +254,7 @@ def confirm(prompt_str, default=False):
 def no_going_back(confirmation):
     """Show a confirmation to a user.
 
-    :param confirmation str: the string the user has to enter in order to
-                             confirm their action.
+    :param confirmation str: the string the user has to enter in order to confirm their action.
     """
     if not confirmation:
         confirmation = 'yes'
diff --git a/SoftLayer/CLI/globalip/assign.py b/SoftLayer/CLI/globalip/assign.py
index 1e793761..a03d8374 100644
--- a/SoftLayer/CLI/globalip/assign.py
+++ b/SoftLayer/CLI/globalip/assign.py
@@ -5,27 +5,51 @@
 
 import SoftLayer
 from SoftLayer.CLI import environment
+from SoftLayer.CLI import helpers
 
-target_types = {'vlan': 'SoftLayer_Network_Vlan',
-                'ip': 'SoftLayer_Network_Subnet_IpAddress',
-                'hardware': 'SoftLayer_Hardware_Server',
-                'vsi': 'SoftLayer_Virtual_Guest'}
+
+# pylint: disable=unused-argument
+def targetipcallback(ctx, param, value):
+    """This is here to allow for using --target-id in some cases. Takes the first value and returns it"""
+    if value:
+        return value[0]
+    return value
 
 
 @click.command(cls=SoftLayer.CLI.command.SLCommand, epilog="More information about types and identifiers "
                "on https://sldn.softlayer.com/reference/services/SoftLayer_Network_Subnet/route/")
-@click.argument('identifier')
+@click.argument('globalip')
+@click.argument('targetip', nargs=-1, callback=targetipcallback)
 @click.option('--target', type=click.Choice(['vlan', 'ip', 'hardware', 'vsi']),
               help='choose the type. vlan, ip, hardware, vsi')
-@click.option('--target-id', help='The identifier for the destination resource to route this subnet to. ')
+@click.option('--target-id', help='The identifier for the destination resource to route this subnet to.')
 @environment.pass_env
-def cli(env, identifier, target, target_id):
-    """Assigns the subnet to a target.
+def cli(env, globalip, targetip, target, target_id):
+    """Assigns the GLOBALIP to TARGETIP.
 
+    GLOBALIP should be either the Global IP address, or the SoftLayer_Network_Subnet_IpAddress_Global id
+    See `slcli globalip list`
+    TARGETIP should be either the target IP address, or the SoftLayer_Network_Subnet_IpAddress id
+    See `slcli subnet list`
     Example::
+
         slcli globalip assign 12345678 9.111.123.456
-        This command assigns IP address with ID 12345678 to a target device whose IP address is 9.111.123.456
-    """
+        This command assigns Global IP address with ID 12345678 to a target device whose IP address is 9.111.123.456
 
+        slcli globalip assign 123.4.5.6 6.5.4.123
+        Global IPs can be specified by their IP address
+    """
     mgr = SoftLayer.NetworkManager(env.client)
-    mgr.route(identifier, target_types.get(target), target_id)
+    # Find SoftLayer_Network_Subnet_IpAddress_Global::id
+    global_ip_id = helpers.resolve_id(mgr.resolve_global_ip_ids, globalip,  name='Global IP')
+
+    # Find Global IPs SoftLayer_Network_Subnet::id
+    mask = "mask[id,ipAddress[subnetId]]"
+    subnet = env.client.call('SoftLayer_Network_Subnet_IpAddress_Global', 'getObject', id=global_ip_id, mask=mask)
+    subnet_id = subnet.get('ipAddress', {}).get('subnetId')
+
+    # For backwards compatibility
+    if target_id:
+        targetip = target_id
+
+    mgr.route(subnet_id, 'SoftLayer_Network_Subnet_IpAddress', targetip)
diff --git a/SoftLayer/CLI/globalip/cancel.py b/SoftLayer/CLI/globalip/cancel.py
index 0d9394b2..920d07c7 100644
--- a/SoftLayer/CLI/globalip/cancel.py
+++ b/SoftLayer/CLI/globalip/cancel.py
@@ -18,12 +18,12 @@ def cli(env, identifier, force):
     """Cancel global IP.
 
     Example::
+
         slcli globalip cancel 12345
     """
 
     mgr = SoftLayer.NetworkManager(env.client)
-    global_ip_id = helpers.resolve_id(mgr.resolve_global_ip_ids, identifier,
-                                      name='global ip')
+    global_ip_id = helpers.resolve_id(mgr.resolve_global_ip_ids, identifier, name='global ip')
 
     if not force:
         if not (env.skip_confirmations or
diff --git a/SoftLayer/CLI/globalip/unassign.py b/SoftLayer/CLI/globalip/unassign.py
index 563ebb10..564c74a8 100644
--- a/SoftLayer/CLI/globalip/unassign.py
+++ b/SoftLayer/CLI/globalip/unassign.py
@@ -12,9 +12,21 @@
 @click.argument('identifier')
 @environment.pass_env
 def cli(env, identifier):
-    """Unassigns a global IP from a target."""
+    """Unroutes IDENTIFIER
+
+    IDENTIFIER should be either the Global IP address, or the SoftLayer_Network_Subnet_IpAddress_Global id
+    Example::
+
+        slcli globalip unassign 123456
+
+        slcli globalip unassign 123.43.22.11
+"""
 
     mgr = SoftLayer.NetworkManager(env.client)
-    global_ip_id = helpers.resolve_id(mgr.resolve_global_ip_ids, identifier,
-                                      name='global ip')
-    mgr.unassign_global_ip(global_ip_id)
+    global_ip_id = helpers.resolve_id(mgr.resolve_global_ip_ids, identifier, name='global ip')
+
+    # Find Global IPs SoftLayer_Network_Subnet::id
+    mask = "mask[id,ipAddress[subnetId]]"
+    subnet = env.client.call('SoftLayer_Network_Subnet_IpAddress_Global', 'getObject', id=global_ip_id, mask=mask)
+    subnet_id = subnet.get('ipAddress', {}).get('subnetId')
+    mgr.clear_route(subnet_id)
diff --git a/SoftLayer/fixtures/SoftLayer_Network_Subnet_IpAddress_Global.py b/SoftLayer/fixtures/SoftLayer_Network_Subnet_IpAddress_Global.py
index 89cd22f5..39244730 100644
--- a/SoftLayer/fixtures/SoftLayer_Network_Subnet_IpAddress_Global.py
+++ b/SoftLayer/fixtures/SoftLayer_Network_Subnet_IpAddress_Global.py
@@ -1,3 +1,3 @@
 route = True
 unroute = True
-getObject = {'id': 1234, 'billingItem': {'id': 1234}}
+getObject = {'id': 1234, 'billingItem': {'id': 1234}, 'ipAddress': {'subnetId': 9988}}
diff --git a/SoftLayer/managers/network.py b/SoftLayer/managers/network.py
index 20186460..49af7197 100644
--- a/SoftLayer/managers/network.py
+++ b/SoftLayer/managers/network.py
@@ -841,12 +841,12 @@ def get_closed_pods(self):
     def route(self, subnet_id, type_serv, target):
         """Assigns a subnet to a specified target.
 
-        :param int subnet_id: The ID of the global IP being assigned
+        https://sldn.softlayer.com/reference/services/SoftLayer_Network_Subnet/route/
+        :param int subnet_id: The ID of the SoftLayer_Network_Subnet_IpAddress being routed
         :param string type_serv: The type service to assign
         :param string target: The instance to assign
         """
-        return self.client.call('SoftLayer_Network_Subnet', 'route',
-                                type_serv, target, id=subnet_id, )
+        return self.client.call('SoftLayer_Network_Subnet', 'route', type_serv, target, id=subnet_id, )
 
     def get_datacenter(self, _filter=None, datacenter=None):
         """Calls SoftLayer_Location::getDatacenters()


Test patch:
diff --git a/tests/CLI/modules/globalip_tests.py b/tests/CLI/modules/globalip_tests.py
index 43c5b0f4..a309d563 100644
--- a/tests/CLI/modules/globalip_tests.py
+++ b/tests/CLI/modules/globalip_tests.py
@@ -12,15 +12,9 @@
 import json
 
 
-class DnsTests(testing.TestCase):
+class GlobalIpTests(testing.TestCase):
 
-    def test_ip_assign(self):
-        result = self.run_command(['globalip', 'assign', '1'])
-
-        self.assert_no_fail(result)
-        self.assertEqual(result.output, "")
-
-    @mock.patch('SoftLayer.CLI.formatting.no_going_back')
+    @mock.patch('SoftLayer.CLI.formatting.confirm')
     def test_ip_cancel(self, no_going_back_mock):
         # Test using --really flag
         result = self.run_command(['--really', 'globalip', 'cancel', '1'])
@@ -39,7 +33,7 @@ def test_ip_cancel(self, no_going_back_mock):
         no_going_back_mock.return_value = False
         result = self.run_command(['globalip', 'cancel', '1'])
 
-        self.assertEqual(result.exit_code, 0)
+        self.assertEqual(result.exit_code, 2)
 
     def test_ip_list(self):
         result = self.run_command(['globalip', 'list', '--ip-version=v4'])
@@ -84,6 +78,31 @@ def test_ip_unassign(self):
         result = self.run_command(['globalip', 'unassign', '1'])
         self.assert_no_fail(result)
         self.assertEqual(result.output, "")
+        self.assert_called_with('SoftLayer_Network_Subnet', 'clearRoute', identifier=9988)
+
+    def test_ip_assign(self):
+        result = self.run_command(['globalip', 'assign', '1', '999'])
+        self.assert_no_fail(result)
+        self.assertEqual(result.output, "")
+        service = 'SoftLayer_Network_Subnet_IpAddress'
+        self.assert_called_with('SoftLayer_Network_Subnet', 'route', identifier=9988, args=(service, '999'))
+
+    def test_ip_assign_target(self):
+        result = self.run_command(['globalip', 'assign', '1', '--target-id=8123'])
+        self.assert_no_fail(result)
+        self.assertEqual(result.output, "")
+        service = 'SoftLayer_Network_Subnet_IpAddress'
+        self.assert_called_with('SoftLayer_Network_Subnet', 'route', identifier=9988, args=(service, '8123'))
+
+    def test_ip_assign_ip(self):
+        mock_api = self.set_mock('SoftLayer_Account', 'getGlobalIpRecords')
+        mock_api.return_value = [{"id": 112233}]
+        result = self.run_command(['globalip', 'assign', '192.168.1.1', '1.2.3.4'])
+        self.assert_no_fail(result)
+        self.assertEqual(result.output, "")
+        service = 'SoftLayer_Network_Subnet_IpAddress'
+        self.assert_called_with(f"{service}_Global", "getObject", identifier=112233)
+        self.assert_called_with('SoftLayer_Network_Subnet', 'route', identifier=9988, args=(service, '1.2.3.4'))
 
     def test_ip_cancel_force(self):
         result = self.run_command(['globalip', 'cancel', '1', '--force'])