insulate TroveContext from possible changes in oslo.context

Earlier code introduced in[1] aimed to handle the situation where a
context object serialized and sent over a message queue as a
dictionary is received by a version of code that doesn't necessarily
understand that context object.

That code relied on the fact that vars(TroveContext).keys() was a
dictionary that matched the full set of parameters that you can send
to the initializers for TroveContext() and RequestContext().

Recent changes in oslo.context [2] and [3] broke that
assumption. vars() on TroveContext provided a dictionary with the
internal representation(s) that are not the same as the kwargs in
RequestContext() and the params that TroveContext used to pop.

To get around this, new code introduced here uses
oslo_utils.reflection to determine all the possible kwargs that can be
sent to TroveContext, some of which are consumed by TroveContext and
the rest of which are sent along to RequestContext().

The construct in the earlier _remove_incompatible_context_args() also
modified kwargs.keys() by pop'ing values from it while iterating over
this. Python 3 takes a dim view of this and therefore some changes
have been made to accomodate this.

A unittest has been added to ensure that the from_dict() method
properly dumps stuff it doesn't know about. The test explicitly
verifies the warning generated when a bogus argument is eliminated.

[1] 24c5e8e244380e3e7e309f0b4aaffde32dbf0789
[2] 2394cff0631944a9259bfe04925e444d9f817758
[3] f25543fcc792ebf155728a91fde06e8dc4e96cea

Change-Id: I477dd29e034295e770925091c4ac6268c22ae59b
Related-Bug:#1661790

3 files changed, 58 insertions, 17 deletions

diff --git a/tools/trove-pylint.config b/tools/trove-pylint.config
index fad15049..6a38a8be 100644
--- a/tools/trove-pylint.config
+++ b/tools/trove-pylint.config
@@ -250,6 +250,18 @@
       "Commands.params_of"
     ],
     [
+      "trove/common/context.py",
+      "E1101",
+      "Module 'inspect' has no 'getfullargspec' member",
+      "TroveContext._remove_incompatible_context_args"
+    ],
+    [
+      "trove/common/context.py",
+      "no-member",
+      "Module 'inspect' has no 'getfullargspec' member",
+      "TroveContext._remove_incompatible_context_args"
+    ],
+    [
       "trove/common/extensions.py",
       "E1003",
       "Bad first argument 'ExtensionMiddleware' given to super()",
diff --git a/trove/common/context.py b/trove/common/context.py
index 5098626a..ed00ed48 100644
--- a/trove/common/context.py
+++ b/trove/common/context.py
@@ -20,9 +20,12 @@ Projects should subclass this class if they wish to enhance the request
 context or provide additional information in their specific WSGI pipeline.
 """
 
+
 from oslo_context import context
 from oslo_log import log as logging
+from oslo_utils import reflection
 
+from trove.common.i18n import _
 from trove.common import local
 from trove.common.serializable_notification import SerializableNotification
 
@@ -34,15 +37,15 @@ class TroveContext(context.RequestContext):
     Stores information about the security context under which the user
     accesses the system, as well as additional request information.
     """
-    def __init__(self, **kwargs):
-        self.limit = kwargs.pop('limit', None)
-        self.marker = kwargs.pop('marker', None)
-        self.service_catalog = kwargs.pop('service_catalog', None)
-        self.user_identity = kwargs.pop('user_identity', None)
-        self.instance_id = kwargs.pop('instance_id', None)
-
-        # TODO(esp): not sure we need this
-        self.timeout = kwargs.pop('timeout', None)
+    def __init__(self, limit=None, marker=None, service_catalog=None,
+                 user_identity=None, instance_id=None, timeout=None,
+                 **kwargs):
+        self.limit = limit
+        self.marker = marker
+        self.service_catalog = service_catalog
+        self.user_identity = user_identity
+        self.instance_id = instance_id
+        self.timeout = timeout
         super(TroveContext, self).__init__(**kwargs)
 
         if not hasattr(local.store, 'context'):
@@ -65,16 +68,20 @@ class TroveContext(context.RequestContext):
 
     @classmethod
     def _remove_incompatible_context_args(cls, values):
-        LOG.debug("Running in unsafe mode and ignoring incompatible context.")
-        return values
+        realvalues = {}
+
+        args = (reflection.get_callable_args(context.RequestContext.__init__) +
+                reflection.get_callable_args(TroveContext.__init__))
 
-        context_keys = vars(cls()).keys()
         for dict_key in values.keys():
-            if dict_key not in context_keys:
-                LOG.debug("Argument being removed before instantiating "
-                          "TroveContext object - %s" % dict_key)
-                values.pop(dict_key, None)
-        return values
+            if dict_key in args:
+                realvalues[dict_key] = values.get(dict_key)
+            else:
+                LOG.warning(_("Argument being removed before instantiating "
+                              "TroveContext object - %(key)s = %(value)s"),
+                            {'key': dict_key, 'value': values.get(dict_key)})
+
+        return realvalues
 
     @classmethod
     def from_dict(cls, values):
diff --git a/trove/tests/unittests/common/test_context.py b/trove/tests/unittests/common/test_context.py
index f49a7a18..2c98d5b8 100644
--- a/trove/tests/unittests/common/test_context.py
+++ b/trove/tests/unittests/common/test_context.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 #
+import mock
 from mock import Mock
 
 from testtools.matchers import Equals, Is
@@ -63,3 +64,24 @@ class TestTroveContext(trove_testtools.TestCase):
         self.assertThat(n_dict.get('notification_classname'),
                         Equals('trove.common.notification.'
                                'DBaaSInstanceCreate'))
+
+    def test_create_with_bogus(self):
+        with mock.patch('trove.common.context.LOG') as mock_log:
+            ctx = context.TroveContext.from_dict(
+                {'user': 'test_user_id',
+                 'request_id': 'test_req_id',
+                 'tenant': 'abc',
+                 'blah_blah': 'blah blah'})
+            mock_log.warning.assert_called()
+            mock_log.warning.assert_called_with('Argument being removed '
+                                                'before instantiating '
+                                                'TroveContext object - '
+                                                '%(key)s = %(value)s',
+                                                {'value': 'blah blah',
+                                                 'key': 'blah_blah'})
+        self.assertThat(ctx.user, Equals('test_user_id'))
+        self.assertThat(ctx.request_id, Equals('test_req_id'))
+        self.assertThat(ctx.tenant, Equals('abc'))
+        self.assertThat(ctx.limit, Is(None))
+        self.assertThat(ctx.marker, Is(None))
+        self.assertThat(ctx.service_catalog, Is(None))