Merge "api: Log correct client IP if load balancer in use"

Author: Zuul

nova/api/openstack/requestlog.py

@@ -21,6 +21,9 @@
 
 from nova.api.openstack import wsgi
 from nova.api import wsgi as base_wsgi
+import nova.conf
+
+CONF = nova.conf.CONF
 
 # TODO(sdague) maybe we can use a better name here for the logger
 LOG = logging.getLogger(__name__)
@@ -65,8 +68,17 @@ def _log_req(self, req, res, start):
         # wsgi stack, res is going to be an empty dict for the
         # fallback logging. So never count on it having attributes.
         status = getattr(res, "status", "500 Error").split(None, 1)[0]
+
+        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': req.environ.get('REMOTE_ADDR', '-'),
+            'REMOTE_ADDR': remote_address,
             'REQUEST_METHOD': req.environ['REQUEST_METHOD'],
             'REQUEST_URI': self._get_uri(req.environ),
             'status': status,

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

@@ -58,7 +58,7 @@ def test_logs_requests(self, emit):
         """
 
         emit.return_value = True
-        self.useFixture(fixtures.ConfFixture())
+        conf = self.useFixture(fixtures.ConfFixture()).conf
         self.useFixture(fixtures.RPCFixture('nova.test'))
         api = self.useFixture(fixtures.OSAPIFixture()).api
 
@@ -73,6 +73,25 @@ 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.