Merge "Remove deprecated [api] use_forwarded_for"

Author: Zuul

doc/source/admin/metadata-service.rst

@@ -76,7 +76,6 @@ service-related options:
 - :oslo.config:option:`neutron.service_metadata_proxy`
 - :oslo.config:option:`neutron.metadata_proxy_shared_secret`
 - :oslo.config:option:`api.metadata_cache_expiration`
-- :oslo.config:option:`api.use_forwarded_for`
 - :oslo.config:option:`api.local_metadata_per_cell`
 - :oslo.config:option:`api.dhcp_domain`
 
@@ -105,7 +104,6 @@ following to a :file:`nova-api.conf` file:
     [api]
     dhcp_domain =
     metadata_cache_expiration = 15
-    use_forwarded_for = False
     local_metadata_per_cell = False
     vendordata_providers = StaticJSON
     vendordata_jsonfile_path = /etc/nova/vendor_data.json
@@ -124,7 +122,6 @@ The :program:`nova-api-metadata` application accepts almost the same options:
 - :oslo.config:option:`neutron.service_metadata_proxy`
 - :oslo.config:option:`neutron.metadata_proxy_shared_secret`
 - :oslo.config:option:`api.metadata_cache_expiration`
-- :oslo.config:option:`api.use_forwarded_for`
 - :oslo.config:option:`api.local_metadata_per_cell`
 - :oslo.config:option:`api.dhcp_domain`
 
@@ -151,7 +148,6 @@ file:
     [api]
     dhcp_domain =
     metadata_cache_expiration = 15
-    use_forwarded_for = False
     local_metadata_per_cell = False
 
 .. note::

nova/api/auth.py

@@ -93,8 +93,6 @@ def _create_context(env, **kwargs):
     def __call__(self, req):
         # Build a context, including the auth_token...
         remote_address = req.remote_addr
-        if CONF.api.use_forwarded_for:
-            remote_address = req.headers.get('X-Forwarded-For', remote_address)
 
         service_catalog = None
         if req.headers.get('X_SERVICE_CATALOG') is not None:

nova/api/metadata/handler.py

@@ -141,8 +141,6 @@ def __call__(self, req):
 
     def _handle_remote_ip_request(self, req):
         remote_address = req.remote_addr
-        if CONF.api.use_forwarded_for:
-            remote_address = req.headers.get('X-Forwarded-For', remote_address)
 
         try:
             meta_data = self.get_metadata_by_remote_address(remote_address)

nova/api/openstack/auth.py

@@ -51,8 +51,6 @@ def base_call(self, req, project_id_in_path, always_admin=True):
         user_id, _sep, project_id = token.partition(':')
         project_id = project_id or user_id
         remote_address = getattr(req, 'remote_addr', '127.0.0.1')
-        if CONF.api.use_forwarded_for:
-            remote_address = req.headers.get('X-Forwarded-For', remote_address)
         is_admin = always_admin or (user_id == 'admin')
         ctx = context.RequestContext(
             user_id, project_id, is_admin=is_admin,

nova/api/openstack/requestlog.py

@@ -71,12 +71,6 @@ def _log_req(self, req, res, start):
 
         remote_address = req.environ.get('REMOTE_ADDR', '-')
 
-        # If the API is configured to treat the X-Forwarded-For header as the
-        # canonical remote address, use its value instead.
-        if CONF.api.use_forwarded_for:
-            remote_address = req.environ.get(
-                'HTTP_X_FORWARDED_FOR', remote_address)
-
         data = {
             'REMOTE_ADDR': remote_address,
             'REQUEST_METHOD': req.environ['REQUEST_METHOD'],

nova/conf/api.py

@@ -39,19 +39,6 @@
 """,
         help="""
 Determine the strategy to use for authentication.
-"""),
-    cfg.BoolOpt("use_forwarded_for",
-        default=False,
-        deprecated_for_removal=True,
-        deprecated_reason='This feature is duplicate of the HTTPProxyToWSGI '
-                          'middleware in oslo.middleware',
-        deprecated_group="DEFAULT",
-        deprecated_since='26.0.0',
-        help="""
-When True, the 'X-Forwarded-For' header is treated as the canonical remote
-address. When False (the default), the 'remote_address' header is used.
-
-You should only enable this if you have an HTML sanitizing proxy.
 """),
 ]
 

nova/tests/unit/api/openstack/test_requestlog.py

@@ -58,7 +58,7 @@ def test_logs_requests(self, emit):
         """
 
         emit.return_value = True
-        conf = self.useFixture(fixtures.ConfFixture()).conf
+        self.useFixture(fixtures.ConfFixture())
         self.useFixture(fixtures.RPCFixture('nova.test'))
         api = self.useFixture(fixtures.OSAPIFixture()).api
 
@@ -73,25 +73,6 @@ def test_logs_requests(self, emit):
                 '"GET /" status: 200 len: %s' % content_length)
         self.assertIn(log1, self.stdlog.logger.output)
 
-        # Verify handling of X-Forwarded-For header, example: load balancer.
-        # First, try without setting CONF.api.use_forwarded_for, it should not
-        # use the header value.
-        headers = {'X-Forwarded-For': '1.2.3.4'}
-        resp = api.api_request('/', strip_version=True, headers=headers)
-        content_length = resp.headers['content-length']
-        log2 = ('INFO [nova.api.openstack.requestlog] 127.0.0.1 '
-                '"GET /" status: 200 len: %s' % content_length)
-        self.assertIn(log2, self.stdlog.logger.output)
-
-        # Now set CONF.api.use_forwarded_for, it should use the header value.
-        conf.set_override('use_forwarded_for', True, 'api')
-        headers = {'X-Forwarded-For': '1.2.3.4'}
-        resp = api.api_request('/', strip_version=True, headers=headers)
-        content_length = resp.headers['content-length']
-        log3 = ('INFO [nova.api.openstack.requestlog] 1.2.3.4 '
-                '"GET /" status: 200 len: %s' % content_length)
-        self.assertIn(log3, self.stdlog.logger.output)
-
     @mock.patch('nova.api.openstack.requestlog.RequestLog._should_emit')
     def test_logs_mv(self, emit):
         """Ensure logs register microversion if passed.

nova/tests/unit/test_metadata.py

@@ -1152,36 +1152,6 @@ def test_invalid_path_is_404(self):
                                 relpath="/2009-04-04/user-data-invalid")
         self.assertEqual(response.status_int, 404)
 
-    def test_user_data_with_use_forwarded_header(self):
-        expected_addr = "192.192.192.2"
-
-        def fake_get_metadata(self_gm, address):
-            if address == expected_addr:
-                return self.mdinst
-            else:
-                raise Exception("Expected addr of %s, got %s" %
-                                (expected_addr, address))
-
-        self.flags(use_forwarded_for=True, group='api')
-        response = fake_request(self, self.mdinst,
-                                relpath="/2009-04-04/user-data",
-                                address="168.168.168.1",
-                                fake_get_metadata=fake_get_metadata,
-                                headers={'X-Forwarded-For': expected_addr})
-
-        self.assertEqual(response.status_int, 200)
-        response_ctype = response.headers['Content-Type']
-        self.assertTrue(response_ctype.startswith("text/plain"))
-        self.assertEqual(response.body,
-                         base64.decode_as_bytes(self.instance['user_data']))
-
-        response = fake_request(self, self.mdinst,
-                                relpath="/2009-04-04/user-data",
-                                address="168.168.168.1",
-                                fake_get_metadata=fake_get_metadata,
-                                headers=None)
-        self.assertEqual(response.status_int, 500)
-
     @mock.patch('oslo_utils.secretutils.constant_time_compare')
     def test_by_instance_id_uses_constant_time_compare(self, mock_compare):
         mock_compare.side_effect = test.TestingException

releasenotes/notes/remove-use_forwarded_for-dd89edfa3addb305.yaml

@@ -0,0 +1,4 @@
+---
+upgrade:
+  - |
+    The deprecated ``[api] use_forwarded_for`` option has been removed.