Browse Source

Allow spaces in host names in the storwize driver

Storwize/SVC naming rules allow host objects to have spaces in their
names. The SSH injection filter will currently reject such names, and so
we surround the names with quotes. Please note that this doesn't allow
to have an arbitrary character in there except a space (anything else
will be cleaned up by the driver or rejected by the SSH injection filter
up the call chain).

Change-Id: If9aec40fe34293031f08d759dd930d73ead456f5
Closes-Bug: #1270204
tags/2014.1.b3
Avishay Traeger 5 years ago
parent
commit
e62b70562d
2 changed files with 38 additions and 36 deletions
  1. 28
    27
      cinder/tests/test_storwize_svc.py
  2. 10
    9
      cinder/volume/drivers/ibm/storwize_svc/ssh.py

+ 28
- 27
cinder/tests/test_storwize_svc.py View File

@@ -533,7 +533,7 @@ port_speed!N/A
533 533
         volume_info['uid'] = ('ABCDEF' * 3) + ('0' * 14) + volume_info['id']
534 534
 
535 535
         if 'name' in kwargs:
536
-            volume_info['name'] = kwargs['name'].strip('\'\'')
536
+            volume_info['name'] = kwargs['name'].strip('\'\"')
537 537
         else:
538 538
             volume_info['name'] = 'vdisk' + volume_info['id']
539 539
 
@@ -603,7 +603,7 @@ port_speed!N/A
603 603
 
604 604
         if 'obj' not in kwargs:
605 605
             return self._errors['CMMVC5701E']
606
-        vol_name = kwargs['obj'].strip('\'\'')
606
+        vol_name = kwargs['obj'].strip('\'\"')
607 607
 
608 608
         if vol_name not in self._volumes_list:
609 609
             return self._errors['CMMVC5753E']
@@ -623,7 +623,7 @@ port_speed!N/A
623 623
     def _cmd_expandvdisksize(self, **kwargs):
624 624
         if 'obj' not in kwargs:
625 625
             return self._errors['CMMVC5701E']
626
-        vol_name = kwargs['obj'].strip('\'\'')
626
+        vol_name = kwargs['obj'].strip('\'\"')
627 627
 
628 628
         # Assume unit is gb
629 629
         if 'size' not in kwargs:
@@ -816,7 +816,7 @@ port_speed!N/A
816 816
     def _cmd_addhostport(self, **kwargs):
817 817
         if 'obj' not in kwargs:
818 818
             return self._errors['CMMVC5701E']
819
-        host_name = kwargs['obj'].strip('\'\'')
819
+        host_name = kwargs['obj'].strip('\'\"')
820 820
 
821 821
         if host_name not in self._hosts_list:
822 822
             return self._errors['CMMVC5753E']
@@ -828,11 +828,11 @@ port_speed!N/A
828 828
     def _cmd_chhost(self, **kwargs):
829 829
         if 'chapsecret' not in kwargs:
830 830
             return self._errors['CMMVC5707E']
831
-        secret = kwargs['obj'].strip('\'\'')
831
+        secret = kwargs['obj'].strip('\'\"')
832 832
 
833 833
         if 'obj' not in kwargs:
834 834
             return self._errors['CMMVC5701E']
835
-        host_name = kwargs['obj'].strip('\'\'')
835
+        host_name = kwargs['obj'].strip('\'\"')
836 836
 
837 837
         if host_name not in self._hosts_list:
838 838
             return self._errors['CMMVC5753E']
@@ -845,7 +845,7 @@ port_speed!N/A
845 845
         if 'obj' not in kwargs:
846 846
             return self._errors['CMMVC5701E']
847 847
 
848
-        host_name = kwargs['obj'].strip('\'\'')
848
+        host_name = kwargs['obj'].strip('\'\"')
849 849
         if host_name not in self._hosts_list:
850 850
             return self._errors['CMMVC5753E']
851 851
 
@@ -875,9 +875,10 @@ port_speed!N/A
875 875
             else:
876 876
                 return ('', '')
877 877
         else:
878
-            if kwargs['obj'] not in self._hosts_list:
878
+            host_name = kwargs['obj'].strip('\'\"')
879
+            if host_name not in self._hosts_list:
879 880
                 return self._errors['CMMVC5754E']
880
-            host = self._hosts_list[kwargs['obj']]
881
+            host = self._hosts_list[host_name]
881 882
             rows = []
882 883
             rows.append(['id', host['id']])
883 884
             rows.append(['name', host['host_name']])
@@ -931,15 +932,15 @@ port_speed!N/A
931 932
 
932 933
         if 'host' not in kwargs:
933 934
             return self._errors['CMMVC5707E']
934
-        mapping_info['host'] = kwargs['host'].strip('\'\'')
935
+        mapping_info['host'] = kwargs['host'].strip('\'\"')
935 936
 
936 937
         if 'scsi' not in kwargs:
937 938
             return self._errors['CMMVC5707E']
938
-        mapping_info['lun'] = kwargs['scsi'].strip('\'\'')
939
+        mapping_info['lun'] = kwargs['scsi'].strip('\'\"')
939 940
 
940 941
         if 'obj' not in kwargs:
941 942
             return self._errors['CMMVC5707E']
942
-        mapping_info['vol'] = kwargs['obj'].strip('\'\'')
943
+        mapping_info['vol'] = kwargs['obj'].strip('\'\"')
943 944
 
944 945
         if mapping_info['vol'] not in self._volumes_list:
945 946
             return self._errors['CMMVC5753E']
@@ -967,11 +968,11 @@ port_speed!N/A
967 968
     def _cmd_rmvdiskhostmap(self, **kwargs):
968 969
         if 'host' not in kwargs:
969 970
             return self._errors['CMMVC5707E']
970
-        host = kwargs['host'].strip('\'\'')
971
+        host = kwargs['host'].strip('\'\"')
971 972
 
972 973
         if 'obj' not in kwargs:
973 974
             return self._errors['CMMVC5701E']
974
-        vol = kwargs['obj'].strip('\'\'')
975
+        vol = kwargs['obj'].strip('\'\"')
975 976
 
976 977
         mapping_ids = []
977 978
         for v in self._mappings_list.itervalues():
@@ -992,7 +993,7 @@ port_speed!N/A
992 993
 
993 994
     # List information about host->vdisk mappings
994 995
     def _cmd_lshostvdiskmap(self, **kwargs):
995
-        host_name = kwargs['obj']
996
+        host_name = kwargs['obj'].strip('\'\"')
996 997
 
997 998
         if host_name not in self._hosts_list:
998 999
             return self._errors['CMMVC5754E']
@@ -1044,13 +1045,13 @@ port_speed!N/A
1044 1045
 
1045 1046
         if 'source' not in kwargs:
1046 1047
             return self._errors['CMMVC5707E']
1047
-        source = kwargs['source'].strip('\'\'')
1048
+        source = kwargs['source'].strip('\'\"')
1048 1049
         if source not in self._volumes_list:
1049 1050
             return self._errors['CMMVC5754E']
1050 1051
 
1051 1052
         if 'target' not in kwargs:
1052 1053
             return self._errors['CMMVC5707E']
1053
-        target = kwargs['target'].strip('\'\'')
1054
+        target = kwargs['target'].strip('\'\"')
1054 1055
         if target not in self._volumes_list:
1055 1056
             return self._errors['CMMVC5754E']
1056 1057
 
@@ -1211,8 +1212,8 @@ port_speed!N/A
1211 1212
     def _cmd_migratevdisk(self, **kwargs):
1212 1213
         if 'mdiskgrp' not in kwargs or 'vdisk' not in kwargs:
1213 1214
             return self._errors['CMMVC5707E']
1214
-        mdiskgrp = kwargs['mdiskgrp'].strip('\'\'')
1215
-        vdisk = kwargs['vdisk'].strip('\'\'')
1215
+        mdiskgrp = kwargs['mdiskgrp'].strip('\'\"')
1216
+        vdisk = kwargs['vdisk'].strip('\'\"')
1216 1217
 
1217 1218
         if vdisk in self._volumes_list:
1218 1219
             curr_mdiskgrp = self._volumes_list
@@ -1244,13 +1245,13 @@ port_speed!N/A
1244 1245
     def _cmd_addvdiskcopy(self, **kwargs):
1245 1246
         if 'obj' not in kwargs:
1246 1247
             return self._errors['CMMVC5701E']
1247
-        vol_name = kwargs['obj'].strip('\'\'')
1248
+        vol_name = kwargs['obj'].strip('\'\"')
1248 1249
         if vol_name not in self._volumes_list:
1249 1250
             return self._errors['CMMVC5753E']
1250 1251
         vol = self._volumes_list[vol_name]
1251 1252
         if 'mdiskgrp' not in kwargs:
1252 1253
             return self._errors['CMMVC5707E']
1253
-        mdiskgrp = kwargs['mdiskgrp'].strip('\'\'')
1254
+        mdiskgrp = kwargs['mdiskgrp'].strip('\'\"')
1254 1255
 
1255 1256
         copy_info = {}
1256 1257
         copy_info['id'] = self._find_unused_id(vol['copies'])
@@ -1297,7 +1298,7 @@ port_speed!N/A
1297 1298
         if 'copy' not in kwargs:
1298 1299
             return self._print_info_cmd(rows=rows, **kwargs)
1299 1300
         else:
1300
-            copy_id = kwargs['copy'].strip('\'\'')
1301
+            copy_id = kwargs['copy'].strip('\'\"')
1301 1302
             if copy_id not in vol['copies']:
1302 1303
                 return self._errors['CMMVC6353E']
1303 1304
             copy = vol['copies'][copy_id]
@@ -1325,10 +1326,10 @@ port_speed!N/A
1325 1326
     def _cmd_rmvdiskcopy(self, **kwargs):
1326 1327
         if 'obj' not in kwargs:
1327 1328
             return self._errors['CMMVC5701E']
1328
-        vol_name = kwargs['obj'].strip('\'\'')
1329
+        vol_name = kwargs['obj'].strip('\'\"')
1329 1330
         if 'copy' not in kwargs:
1330 1331
             return self._errors['CMMVC5707E']
1331
-        copy_id = kwargs['copy'].strip('\'\'')
1332
+        copy_id = kwargs['copy'].strip('\'\"')
1332 1333
         if vol_name not in self._volumes_list:
1333 1334
             return self._errors['CMMVC5753E']
1334 1335
         vol = self._volumes_list[vol_name]
@@ -1341,7 +1342,7 @@ port_speed!N/A
1341 1342
     def _cmd_chvdisk(self, **kwargs):
1342 1343
         if 'obj' not in kwargs:
1343 1344
             return self._errors['CMMVC5701E']
1344
-        vol_name = kwargs['obj'].strip('\'\'')
1345
+        vol_name = kwargs['obj'].strip('\'\"')
1345 1346
         vol = self._volumes_list[vol_name]
1346 1347
         kwargs.pop('obj')
1347 1348
 
@@ -1363,7 +1364,7 @@ port_speed!N/A
1363 1364
     def _cmd_movevdisk(self, **kwargs):
1364 1365
         if 'obj' not in kwargs:
1365 1366
             return self._errors['CMMVC5701E']
1366
-        vol_name = kwargs['obj'].strip('\'\'')
1367
+        vol_name = kwargs['obj'].strip('\'\"')
1367 1368
         vol = self._volumes_list[vol_name]
1368 1369
 
1369 1370
         if 'iogrp' not in kwargs:
@@ -1505,6 +1506,7 @@ class StorwizeSVCFakeDriver(storwize_svc.StorwizeSVCDriver):
1505 1506
     def _run_ssh(self, cmd, check_exit_code=True, attempts=1):
1506 1507
         try:
1507 1508
             LOG.debug(_('Run CLI command: %s') % cmd)
1509
+            utils.check_ssh_injection(cmd)
1508 1510
             ret = self.fake_storage.execute_command(cmd, check_exit_code)
1509 1511
             (stdout, stderr) = ret
1510 1512
             LOG.debug(_('CLI output:\n stdout: %(stdout)s\n stderr: '
@@ -2329,7 +2331,6 @@ class StorwizeSVCDriverTestCase(test.TestCase):
2329 2331
                             self.driver._state['extent_size'])
2330 2332
         self.driver.migrate_volume(ctxt, volume, host)
2331 2333
         attrs = self.driver._helpers.get_vdisk_attributes(volume['name'])
2332
-        print('AVISHAY ' + str(attrs))
2333 2334
         self.assertIn('openstack3', attrs['mdisk_grp_name'])
2334 2335
         self.driver.delete_volume(volume)
2335 2336
 

+ 10
- 9
cinder/volume/drivers/ibm/storwize_svc/ssh.py View File

@@ -109,12 +109,13 @@ class StorwizeSSH(object):
109 109
 
110 110
     def mkhost(self, host_name, port_type, port_name):
111 111
         port = self._create_port_arg(port_type, port_name)
112
-        ssh_cmd = ['svctask', 'mkhost', '-force'] + port + ['-name', host_name]
112
+        ssh_cmd = ['svctask', 'mkhost', '-force'] + port
113
+        ssh_cmd += ['-name', '"%s"' % host_name]
113 114
         return self.run_ssh_check_created(ssh_cmd)
114 115
 
115 116
     def addhostport(self, host, port_type, port_name):
116 117
         port = self._create_port_arg(port_type, port_name)
117
-        ssh_cmd = ['svctask', 'addhostport', '-force'] + port + [host]
118
+        ssh_cmd = ['svctask', 'addhostport', '-force'] + port + ['"%s"' % host]
118 119
         self.run_ssh_assert_no_output(ssh_cmd)
119 120
 
120 121
     def lshost(self, host=None):
@@ -122,11 +123,11 @@ class StorwizeSSH(object):
122 123
         ssh_cmd = ['svcinfo', 'lshost', '-delim', '!']
123 124
         if host:
124 125
             with_header = False
125
-            ssh_cmd.append(host)
126
+            ssh_cmd.append('"%s"' % host)
126 127
         return self.run_ssh_info(ssh_cmd, with_header=with_header)
127 128
 
128 129
     def add_chap_secret(self, secret, host):
129
-        ssh_cmd = ['svctask', 'chhost', '-chapsecret', secret, host]
130
+        ssh_cmd = ['svctask', 'chhost', '-chapsecret', secret, '"%s"' % host]
130 131
         self.run_ssh_assert_no_output(ssh_cmd)
131 132
 
132 133
     def lsiscsiauth(self):
@@ -137,7 +138,7 @@ class StorwizeSSH(object):
137 138
         if wwpn:
138 139
             ssh_cmd = ['svcinfo', 'lsfabric', '-wwpn', wwpn, '-delim', '!']
139 140
         elif host:
140
-            ssh_cmd = ['svcinfo', 'lsfabric', '-host', host]
141
+            ssh_cmd = ['svcinfo', 'lsfabric', '-host', '"%s"' % host]
141 142
         else:
142 143
             msg = (_('Must pass wwpn or host to lsfabric.'))
143 144
             LOG.error(msg)
@@ -149,7 +150,7 @@ class StorwizeSSH(object):
149 150
 
150 151
         If vdisk already mapped and multihostmap is True, use the force flag.
151 152
         """
152
-        ssh_cmd = ['svctask', 'mkvdiskhostmap', '-host', host,
153
+        ssh_cmd = ['svctask', 'mkvdiskhostmap', '-host', '"%s"' % host,
153 154
                    '-scsi', lun, vdisk]
154 155
         out, err = self._ssh(ssh_cmd, check_exit_code=False)
155 156
         if 'successfully created' in out:
@@ -171,7 +172,7 @@ class StorwizeSSH(object):
171 172
         return self.run_ssh_check_created(ssh_cmd)
172 173
 
173 174
     def rmvdiskhostmap(self, host, vdisk):
174
-        ssh_cmd = ['svctask', 'rmvdiskhostmap', '-host', host, vdisk]
175
+        ssh_cmd = ['svctask', 'rmvdiskhostmap', '-host', '"%s"' % host, vdisk]
175 176
         self.run_ssh_assert_no_output(ssh_cmd)
176 177
 
177 178
     def lsvdiskhostmap(self, vdisk):
@@ -179,11 +180,11 @@ class StorwizeSSH(object):
179 180
         return self.run_ssh_info(ssh_cmd, with_header=True)
180 181
 
181 182
     def lshostvdiskmap(self, host):
182
-        ssh_cmd = ['svcinfo', 'lshostvdiskmap', '-delim', '!', host]
183
+        ssh_cmd = ['svcinfo', 'lshostvdiskmap', '-delim', '!', '"%s"' % host]
183 184
         return self.run_ssh_info(ssh_cmd, with_header=True)
184 185
 
185 186
     def rmhost(self, host):
186
-        ssh_cmd = ['svctask', 'rmhost', host]
187
+        ssh_cmd = ['svctask', 'rmhost', '"%s"' % host]
187 188
         self.run_ssh_assert_no_output(ssh_cmd)
188 189
 
189 190
     def mkvdisk(self, name, size, units, pool, opts, params):

Loading…
Cancel
Save