x/ospurge
Code Issues Proposed changes

Url quoting of swift object names when deleting

Browse Source 
shade sends the swift object or container name unquoted as an url and in
shade.openstackcloud.py in get_object_metadata where OpenStackCloudException
errors with 404 status_code are silently ignored.
If the name contains special characters such as a '#' we therefore get no
metadata without errors.
The same thing happens after in shade.openstackcloud.py in delete_object when
calling self._object_store_client.delete : an OpenStackCloudHTTPError is
caught and the delete method returns False.
This resulted in ospurge not deleting such objects without even getting
an exception.
After investigation, the issue would be the same in openstacksdk which
copies the same code as shade.
Since nor shade nor openstacksdks perform url encoding of the name, we url
encode it ourselves before calling shade, since shade makes an http
request by calling the swift api with the object name as an url.

Closes-Bug: #1831753

Change-Id: Ie7aea2a14d920ca11012ff26d7f74017704765f5
This commit is contained in:
Yves-Gwenael Bourhis 2019-06-05 16:48:18 +02:00
parent 79ecb6f53c
commit fc73a2e881
2 changed files with 31 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
ospurge/resources/swift.py
View File

@ -9,6 +9,9 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
from six.moves import urllib_parse
from ospurge.resources import base from ospurge.resources import base
from ospurge.resources.base import BaseServiceResource from ospurge.resources.base import BaseServiceResource
from ospurge.resources import glance from ospurge.resources import glance
@ -17,7 +20,9 @@ from ospurge.resources import glance
class ListObjectsMixin(BaseServiceResource): class ListObjectsMixin(BaseServiceResource):
def list_objects(self): def list_objects(self):
for container in self.cloud.list_containers(): for container in self.cloud.list_containers():
for obj in self.cloud.list_objects(container['name']): for obj in self.cloud.list_objects(
urllib_parse.quote(container['name'])
):
obj['container_name'] = container['name'] obj['container_name'] = container['name']
yield obj yield obj
@ -34,7 +39,10 @@ class Objects(base.ServiceResource, glance.ListImagesMixin, ListObjectsMixin):
yield item yield item
def delete(self, resource): def delete(self, resource):
self.cloud.delete_object(resource['container_name'], resource['name']) self.cloud.delete_object(
urllib_parse.quote(resource['container_name']),
urllib_parse.quote(resource['name'])
)
@staticmethod @staticmethod
def to_str(resource): def to_str(resource):
@ -52,7 +60,7 @@ class Containers(base.ServiceResource, ListObjectsMixin):
return self.cloud.list_containers() return self.cloud.list_containers()
def delete(self, resource): def delete(self, resource):
self.cloud.delete_container(resource['name']) self.cloud.delete_container(urllib_parse.quote(resource['name']))
@staticmethod @staticmethod
def to_str(resource): def to_str(resource):

26
ospurge/tests/resources/test_swift.py
View File

@ -11,6 +11,8 @@
# under the License. # under the License.
import unittest import unittest
from six.moves import urllib_parse
import shade import shade
from ospurge.resources import swift from ospurge.resources import swift
@ -76,10 +78,17 @@ class TestObjects(unittest.TestCase):
self.assertRaises(StopIteration, next, objects) self.assertRaises(StopIteration, next, objects)
def test_delete(self): def test_delete(self):
obj = mock.MagicMock() objects = [
self.assertIsNone(swift.Objects(self.creds_manager).delete(obj)) {'name': 'toto', 'container_name': 'foo'},
self.cloud.delete_object.assert_called_once_with( {'name': 'tata%20foo', 'container_name': 'baz%20bar'},
obj['container_name'], obj['name']) {'name': 'titi#1', 'container_name': 'bar#2'},
]
for obj in objects:
self.assertIsNone(swift.Objects(self.creds_manager).delete(obj))
self.cloud.delete_object.assert_called_with(
urllib_parse.quote(obj['container_name']),
urllib_parse.quote(obj['name'])
)
def test_to_string(self): def test_to_string(self):
obj = mock.MagicMock() obj = mock.MagicMock()
@ -111,9 +120,14 @@ class TestContainers(unittest.TestCase):
self.cloud.list_containers.assert_called_once_with() self.cloud.list_containers.assert_called_once_with()
def test_delete(self): def test_delete(self):
cont = mock.MagicMock() cont = {'bytes': 8,
'count': 2,
'last_modified': '2019-06-05T15:20:59.450120',
'name': 'Pouet éêù #'}
self.assertIsNone(swift.Containers(self.creds_manager).delete(cont)) self.assertIsNone(swift.Containers(self.creds_manager).delete(cont))
self.cloud.delete_container.assert_called_once_with(cont['name']) self.cloud.delete_container.assert_called_once_with(
urllib_parse.quote(cont['name'])
)
def test_to_string(self): def test_to_string(self):
container = mock.MagicMock() container = mock.MagicMock()