openstack/cinder
Code Issues Proposed changes

LIO: Handle initiator IQNs as case insensitive

Browse Source 
If there is a mismatch in case between the stored
initiator IQN in the LIO ACL vs. the IQN provided
in a later call, rtstool would fail to find the
existing ACL.

Compare IQNs in a case insensitive manner to ensure
they will always match as intended.  (Refer to RFC
3720 section 3.2.6.1 for details on this.)

Closes-Bug: #1522053

Change-Id: I6f535f3f4fbfcbbb49da30cffb08d17b3cac778a
This commit is contained in:
Eric Harney 2015-12-01 18:14:17 -05:00
parent 65bed98079
commit c3d8c0d058
2 changed files with 96 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cinder/cmd/rtstool.py
View File

@ -128,7 +128,7 @@ def add_initiator(target_iqn, initiator_iqn, userid, password):
tpg = next(target.tpgs) # get the first one
for acl in tpg.node_acls:
# See if this ACL configuration already exists
if acl.node_wwn == initiator_iqn:
if acl.node_wwn.lower() == initiator_iqn.lower():
# No further action required
return
@ -143,7 +143,7 @@ def delete_initiator(target_iqn, initiator_iqn):
target = _lookup_target(target_iqn, initiator_iqn)
tpg = next(target.tpgs) # get the first one
for acl in tpg.node_acls:
if acl.node_wwn == initiator_iqn:
if acl.node_wwn.lower() == initiator_iqn.lower():
acl.delete()
return

102
cinder/tests/unit/test_cmd.py
View File

@ -803,6 +803,9 @@ class TestCinderRtstoolCmd(test.TestCase):
sys.argv = ['cinder-rtstool']
CONF(sys.argv[1:], project='cinder', version=version.version_string())
self.INITIATOR_IQN = 'iqn.2015.12.com.example.openstack.i:UNIT1'
self.TARGET_IQN = 'iqn.2015.12.com.example.openstack.i:TARGET1'
def tearDown(self):
super(TestCinderRtstoolCmd, self).tearDown()
@ -973,7 +976,7 @@ class TestCinderRtstoolCmd(test.TestCase):
self.assertRaises(rtslib_fb.utils.RTSLibError,
cinder_rtstool.add_initiator,
mock.sentinel.target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
@ -984,7 +987,7 @@ class TestCinderRtstoolCmd(test.TestCase):
self.assertRaises(cinder_rtstool.RtstoolError,
cinder_rtstool.add_initiator,
mock.sentinel.target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
@ -994,15 +997,64 @@ class TestCinderRtstoolCmd(test.TestCase):
def test_add_initiator_acl_exists(self, rtsroot, node_acl, mapped_lun):
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': mock.sentinel.initiator_iqn}]
acl = mock.MagicMock(node_wwn=mock.sentinel.initiator_iqn)
[{'node_acls': self.INITIATOR_IQN}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=self.TARGET_IQN)
rtsroot.return_value = mock.MagicMock(targets=[target])
cinder_rtstool.add_initiator(self.TARGET_IQN,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
self.assertFalse(node_acl.called)
self.assertFalse(mapped_lun.called)
@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_add_initiator_acl_exists_case_1(self,
rtsroot,
node_acl,
mapped_lun):
"""Ensure initiator iqns are handled in a case-insensitive manner."""
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': self.INITIATOR_IQN.lower()}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])
cinder_rtstool.add_initiator(target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
self.assertFalse(node_acl.called)
self.assertFalse(mapped_lun.called)
@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_add_initiator_acl_exists_case_2(self,
rtsroot,
node_acl,
mapped_lun):
"""Ensure initiator iqns are handled in a case-insensitive manner."""
iqn_lower = self.INITIATOR_IQN.lower()
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': self.INITIATOR_IQN}]
acl = mock.MagicMock(node_wwn=iqn_lower)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])
cinder_rtstool.add_initiator(target_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
self.assertFalse(node_acl.called)
@ -1014,7 +1066,7 @@ class TestCinderRtstoolCmd(test.TestCase):
def test_add_initiator(self, rtsroot, node_acl, mapped_lun):
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': mock.sentinel.initiator_iqn}]
[{'node_acls': self.INITIATOR_IQN}]
tpg = mock.MagicMock()
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
@ -1025,11 +1077,11 @@ class TestCinderRtstoolCmd(test.TestCase):
node_acl.return_value = acl_new
cinder_rtstool.add_initiator(target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
node_acl.assert_called_once_with(tpg,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mode='create')
mapped_lun.assert_called_once_with(acl_new, 0, tpg_lun=0)
@ -1058,6 +1110,40 @@ class TestCinderRtstoolCmd(test.TestCase):
target.delete.assert_called_once_with()
storage_object.delete.assert_called_once_with()
@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_delete_initiator(self, rtsroot, node_acl, mapped_lun):
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': self.INITIATOR_IQN}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])
cinder_rtstool.delete_initiator(target_iqn,
self.INITIATOR_IQN)
@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_delete_initiator_case(self, rtsroot, node_acl, mapped_lun):
"""Ensure iqns are handled in a case-insensitive manner."""
initiator_iqn_lower = self.INITIATOR_IQN.lower()
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': initiator_iqn_lower}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])
cinder_rtstool.delete_initiator(target_iqn,
self.INITIATOR_IQN)
@mock.patch.object(cinder_rtstool, 'os', autospec=True)
@mock.patch.object(cinder_rtstool, 'rtslib_fb', autospec=True)
def test_save_with_filename(self, mock_rtslib, mock_os):