Raise error when supplying invalid query params

Rather than stripping invalid query params and then
continuing the API call, lets actually throw an error
since we explicitly define what valid query params are.

This stops people from accidentally querying for an
incorrect param, getting back a list of 'everything' and
then acting on it assuming it was a valid query.

Adds a util function for getting keys from format strings
as the checking for invalid keys needs to take into account
what key may be required for the base_url.

Change-Id: If30badb4d71e521100a0e8974978eb6d5fa2699f

openstack/exceptions.py

@@ -152,6 +152,11 @@ class ResourceFailure(SDKException):
     pass
 
 
+class InvalidResourceQuery(SDKException):
+    """Invalid query params for resource."""
+    pass
+
+
 def raise_from_response(response, error_message=None):
     """Raise an instance of an HTTPException based on keystoneauth response."""
     if response.status_code < 400:

openstack/network/v2/floating_ip.py

@@ -73,7 +73,7 @@ class FloatingIP(resource.Resource):
 
     @classmethod
     def find_available(cls, session):
-        info = cls.list(session, fields='id', port_id='')
+        info = cls.list(session, port_id='')
         try:
             return next(info)
         except StopIteration:

openstack/resource.py

@@ -819,10 +819,21 @@ class Resource(object):
         :return: A generator of :class:`Resource` objects.
         :raises: :exc:`~openstack.exceptions.MethodNotSupported` if
                  :data:`Resource.allow_list` is not set to ``True``.
+        :raises: :exc:`~openstack.exceptions.InvalidResourceQuery` if query
+                 contains invalid params.
         """
         if not cls.allow_list:
             raise exceptions.MethodNotSupported(cls, "list")
 
+        expected_params = utils.get_string_format_keys(cls.base_path)
+        expected_params += cls._query_mapping._mapping.keys()
+
+        invalid_keys = set(params.keys()) - set(expected_params)
+        if invalid_keys:
+            raise exceptions.InvalidResourceQuery(
+                message="Invalid query params: %s" % ",".join(invalid_keys),
+                extra_data=invalid_keys)
+
         query_params = cls._query_mapping._transpose(params)
         uri = cls.base_path % params
 

openstack/tests/unit/test_resource.py

@@ -1290,6 +1290,30 @@ class TestResourceActions(base.TestCase):
         self.assertEqual(self.session.get.call_args_list[0][0][0],
                          Test.base_path % {"something": uri_param})
 
+    def test_invalid_list_params(self):
+        id = 1
+        qp = "query param!"
+        qp_name = "query-param"
+        uri_param = "uri param!"
+
+        mock_response = mock.Mock()
+        mock_response.json.side_effect = [[{"id": id}],
+                                          []]
+
+        self.session.get.return_value = mock_response
+
+        class Test(self.test_class):
+            _query_mapping = resource.QueryParameters(query_param=qp_name)
+            base_path = "/%(something)s/blah"
+            something = resource.URI("something")
+
+        try:
+            list(Test.list(self.session, paginated=True, query_param=qp,
+                           something=uri_param, something_wrong=True))
+            self.assertFail('The above line should fail')
+        except exceptions.InvalidResourceQuery as err:
+            self.assertEqual(str(err), 'Invalid query params: something_wrong')
+
     def test_list_multi_page_response_paginated(self):
         ids = [1, 2]
         resp1 = mock.Mock()

openstack/utils.py

@@ -11,6 +11,7 @@
 # under the License.
 
 import functools
+import string
 import time
 
 import deprecation
@@ -101,3 +102,29 @@ def iterate_timeout(timeout, message, wait=2):
         log.debug('Waiting %s seconds', wait)
         time.sleep(wait)
     raise exceptions.ResourceTimeout(message)
+
+
+def get_string_format_keys(fmt_string, old_style=True):
+    """Gets a list of required keys from a format string
+
+    Required mostly for parsing base_path urls for required keys, which
+    use the old style string formatting.
+    """
+    if old_style:
+        class AccessSaver(object):
+            def __init__(self):
+                self.keys = []
+
+            def __getitem__(self, key):
+                self.keys.append(key)
+
+        a = AccessSaver()
+        fmt_string % a
+
+        return a.keys
+    else:
+        keys = []
+        for t in string.Formatter().parse(fmt_string):
+            if t[1] is not None:
+                keys.append(t[1])
+        return keys