moved context reset at the end of the ssl_certificate phase. This tak…

…es care of concurrent requests scenarios. Adapted some tests to match hosts correctly since now those policy chains (that only include policies that don't implement ssl_certificate) no longer use the context.service defined in find_service.lua/ssl_certificate (that was obtained using the server_name of the SNI during the handshake), so they need to specify correct Host headers like in a real request scenario.

gateway/src/apicast/policy/clear_context/clear_context.lua

@@ -2,27 +2,19 @@ local _M = require('apicast.policy').new('Clear Context')
 local new = _M.new
 
 function _M.new(...)
-  ssl_ctx_reset = false
   return new(...)
 end
 
 function _M:ssl_certificate(context)
-  reset_context(context)
-  ssl_ctx_reset = true
+  --resetting the context after every other policy in the chain
+  --has executed their ssl_certificate phase. 
+  clear_table(ngx.ctx)
 end
 
-function _M:rewrite(context)
-  --Do not reset the context again if the ssl_certificate phase
-  --was executed for this request. This way other policies that edited 
-  --the context in their ssl_* phases won't have their changes cleared
-  if not ssl_ctx_reset then
-    reset_context(context)
+function clear_table(t)
+  for k, _ in pairs(t) do
+    t[k] = nil
   end
-  ssl_ctx_reset = false
-end
-
-function reset_context(context)
-  context.current = {}
 end
 
 return _M

gateway/src/apicast/policy_chain.lua

@@ -67,11 +67,11 @@ end
 
 
 local DEFAULT_POLICIES = {
-    'apicast.policy.clear_context',
     'apicast.policy.load_configuration',
     'apicast.policy.find_service',
     'apicast.policy.local_chain',
-    'apicast.policy.nginx_metrics'
+    'apicast.policy.nginx_metrics',
+    'apicast.policy.clear_context'
 }
 
 --- Return new policy chain with default policies.

spec/policy/clear_context/clear_context_spec.lua

@@ -5,9 +5,9 @@ local ngx_variable = require('apicast.policy.ngx_variable')
 describe('Clear Context policy', function()
   local current_ctx = {some_key = 'some_value'}
 
-  describe('.ssl_certificate + .rewrite', function()
+  describe('log phase context reset', function()
     before_each(function()
-      ctx = {}
+      ctx = ngx.ctx
       stub(ngx_variable, 'available_context', function(context) return context end)
     end)
 
@@ -17,56 +17,7 @@ describe('Clear Context policy', function()
       it('clears the context', function()
         ctx.current = current_ctx
         clear_context_policy:ssl_certificate(ctx)
-        assert.are.same({}, ctx.current)
-      end)
-    end)
-
-    context('.rewrite', function()
-      local clear_context_policy = ClearContextPolicy.new()
-
-      it('clears the context', function()
-        ctx.current = current_ctx
-        clear_context_policy:rewrite(ctx)
-        assert.are.same({}, ctx.current)
-      end)
-    end)
-
-    context('.ssl_certificate and .rewrite', function()
-      local clear_context_policy = ClearContextPolicy.new()
-
-      it('clears the context once', function()
-        ctx.current = current_ctx
-        clear_context_policy:ssl_certificate(ctx)
-        assert.are.same({}, ctx.current)
-        ctx.current = current_ctx
-        clear_context_policy:rewrite(ctx)
-        assert.are.same(current_ctx, ctx.current)
-      end)
-    end)
-
-    context('.ssl_certificate and .ssl_certificate', function()
-      local clear_context_policy = ClearContextPolicy.new()
-
-      it('clears the context twice', function()
-        ctx.current = current_ctx
-        clear_context_policy:ssl_certificate(ctx)
-        assert.are.same({}, ctx.current)
-        ctx.current = current_ctx
-        clear_context_policy:ssl_certificate(ctx)
-        assert.are.same({}, ctx.current)
-      end)
-    end)
-
-    context('.rewrite and .rewrite', function()
-      local clear_context_policy = ClearContextPolicy.new()
-
-      it('clears the context twice', function()
-        ctx.current = current_ctx
-        clear_context_policy:rewrite(ctx)
-        assert.are.same({}, ctx.current)
-        ctx.current = current_ctx
-        clear_context_policy:rewrite(ctx)
-        assert.are.same({}, ctx.current)
+        assert.is_nil(ctx.current)
       end)
     end)
   end)

t/apicast-path-routing.t

@@ -373,8 +373,8 @@ Host: one
 --- no_error_log
 [error]
 
-=== TEST 6: Multiple requests that reuse the same TCP connection with path routing. 
-Routing must work as expected to the right service
+=== TEST 6: APIcast must choose correct Product
+based on current configuration & request parameters
 --- env eval
 (
   'APICAST_HTTPS_PORT' => $ENV{APICAST_HTTPS_RANDOM_PORT},
@@ -443,28 +443,40 @@ content_by_lua_block {
   local resty_env = require 'resty.env'
   
   local https_port = resty_env.get('APICAST_HTTPS_PORT')
-  local uri_one = "https://127.0.0.1:".. https_port .."/one?user_key=foo"
-  local uri_two = "https://127.0.0.1:".. https_port .."/two?user_key=foo"
-  
   local httpc = http.new()
-  local res1, err1 = httpc:request_uri(uri_one, {
-    method = "GET",
-    ssl_verify = false,
-    version = 1.1
+  httpc:connect("127.0.0.1", https_port)
+  httpc:ssl_handshake(nil, "127.0.0.1", false)
+
+  local responses, err = httpc:request_pipeline({
+    {
+      method = "GET",
+      path = "/one",
+      version = 1.1,
+      ssl_verify = false,
+      query = "?user_key=foo"
+    },
+    {
+      method = "GET",
+      path = "/two",
+      version = 1.1,
+      ssl_verify = false,
+      query = "?user_key=foo"
+    }
   })
-  assert(res1, "Request failed: "..(err1 or ""))
+  
+  local res1 = responses[1]
+  res1.body = responses[1]:read_body()
+  local res2 = responses[2]
+  res2.body = responses[2]:read_body()
+
+  assert(res1 and res1.status, "Request failed: "..(err or ""))
   assert(string.find(res1.body, "/foo/one"), "Expected .*/foo/one, got: "..(res1.body or ""))
   
-  local res2, err2 = httpc:request_uri(uri_two, {
-    method = "GET",
-    ssl_verify = false, 
-    version = 1.1
-  })
-  assert(res2, "Request failed: "..(err2 or ""))
-  --check for incorrect routing THREESCALE-8000:
+  assert(res2, "Request failed: "..(err or ""))
+  --Check if incorrect routing as reported in THREESCALE-8000 is happening:
   assert(not string.find(res2.body, "/foo/two"), "Expected != .*/foo/two, got: "..(res2.body or "")) 
   assert(string.find(res2.body, "/bar/two"), "Expected .*/bar/two, got: "..(res2.body or ""))
   httpc:close()
 }
 --- no_error_log
-[error]
+[error]

t/apicast-policy-logging.t

@@ -7,6 +7,7 @@ our $public_key = `cat t/fixtures/rsa.pub`;
 # Test::Nginx does not allow to grep access logs, so we redirect them to
 # stderr to be able to use "grep_error_log" by setting APICAST_ACCESS_LOG_FILE
 $ENV{APICAST_ACCESS_LOG_FILE} = "$Test::Nginx::Util::ErrLogFile";
+$ENV{APICAST_HTTPS_RANDOM_PORT} = Test::APIcast::get_random_port();
 check_accum_error_log();
 run_tests();
 
@@ -643,3 +644,100 @@ OK
 --- error_code: 200
 --- no_error_log eval
 [qr/^Status\:\:200 USER_KEY\:\:123 42/]
+
+=== TEST 15: original_request must contain the right information 
+for requests after the first when the same TCP connection is reused
+--- env eval
+(
+    'APICAST_HTTPS_PORT' => $ENV{APICAST_HTTPS_RANDOM_PORT},
+    'HTTP_KEEPALIVE_TIMEOUT' => '5'
+)
+--- backend
+location /transactions/authrep.xml {
+  content_by_lua_block {
+    ngx.exit(200)
+  }
+}
+--- configuration
+{
+  "services": [
+    {
+      "id": 42,
+      "backend_version": 1,
+      "proxy": {
+        "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api-backend/",
+        "hosts": ["127.0.0.1"],
+        "policy_chain": [
+          {
+            "name": "apicast.policy.apicast"
+          },
+          {
+            "name": "apicast.policy.logging",
+            "configuration": {
+              "custom_logging": "Status::{{ status }} {{original_request.path}}",
+              "enable_json_logs": false
+            }
+          }
+        ],
+        "proxy_rules": [
+          {
+            "pattern": "/one",
+            "http_method": "GET",
+            "metric_system_name": "hits",
+            "delta": 1
+          },
+          {
+            "pattern": "/two",
+            "http_method": "GET",
+            "metric_system_name": "hits",
+            "delta": 1
+          }
+        ]
+      }
+    }
+  ]
+}
+--- upstream
+location ~ /api-backend(/.+) {
+  echo 'yay, api backend: $1';
+}
+--- test
+content_by_lua_block {
+  local http = require "resty.http"
+  local resty_env = require 'resty.env'
+
+  local https_port = resty_env.get('APICAST_HTTPS_PORT')
+  local httpc = http.new()
+  httpc:connect("127.0.0.1", https_port)
+  httpc:ssl_handshake(nil, "127.0.0.1", false)
+
+  local responses, err = httpc:request_pipeline({
+    {
+      method = "GET",
+      path = "/one",
+      version = 1.1,
+      ssl_verify = false,
+      query = "?user_key=foo"
+    },
+    {
+      method = "GET",
+      path = "/two",
+      version = 1.1,
+      ssl_verify = false,
+      query = "?user_key=foo"
+    }
+  })
+
+  local res1 = responses[1]
+  res1.body = responses[1]:read_body()
+  local res2 = responses[2]
+  res2.body = responses[2]:read_body()
+
+  assert(responses[1], "Request failed"..(err or ""))
+  assert(responses[2], "Request failed"..(err or ""))
+  httpc:close()
+}
+--- no_error_log
+[error]
+--- error_log eval
+[qr/^Status\:\:200 \/one/, qr/^Status\:\:200 \/two/]

t/apicast-policy-oauth-mtls.t

@@ -70,7 +70,7 @@ proxy_ssl_trusted_certificate $TEST_NGINX_SERVER_ROOT/html/ca.crt;
 proxy_ssl_certificate $TEST_NGINX_SERVER_ROOT/html/client.crt;
 proxy_ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/client.key;
 proxy_pass https://$server_addr:$apicast_port/t;
-proxy_set_header Host localhost;
+proxy_set_header Host test;
 proxy_set_header Authorization "Bearer eyJraWQiOiJzb21la2lkIiwiYWxnIjoiUlMyNTYifQ.eyJleHAiOjE5MjY4NzMwNTQsInN1YiI6InNvbWVvbmUiLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsiZGlyZWN0b3IiXX0sImZvbyI6IjEiLCJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2FwaWNhc3QiLCJhdWQiOiJhdWRpZW5jZSIsImNuZiI6eyJ4NXQjUzI1NiI6Ilk0X0xWbGtwRTZxa3NjUGJ0b0ttM2lpS0JnZndiT2ZiZEtCRWRuWjZaUFkifX0.Iin-tr6EVhEXjbj9R6xZSToBxZZBDXhl6i9ROw6SJQE6RWJeLt6mKK4jdTMVdxoZfm1J_NqayGJh3N99kHdIbA";
 log_by_lua_block { collectgarbage() }
 --- response_body
@@ -140,7 +140,7 @@ proxy_ssl_trusted_certificate $TEST_NGINX_SERVER_ROOT/html/ca.crt;
 proxy_ssl_certificate $TEST_NGINX_SERVER_ROOT/html/client.crt;
 proxy_ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/client.key;
 proxy_pass https://$server_addr:$apicast_port/t;
-proxy_set_header Host localhost;
+proxy_set_header Host test;
 proxy_set_header Authorization "Bearer eyJraWQiOiJzb21la2lkIiwiYWxnIjoiUlMyNTYifQ.eyJleHAiOjE5MjQxMjQ1ODIsInN1YiI6InNvbWVvbmUiLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsiZGlyZWN0b3IiXX0sImZvbyI6IjEiLCJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2FwaWNhc3QiLCJhdWQiOiJhdWRpZW5jZSIsImNuZiI6eyJ4NXQjUzI1NiI6ImludmFsaWQifX0.h9Lay5rff08ipXd2juLS_A0fpJKn6UPD1AIxBCibdTi1wyhF5fCLmxzfwozgtqVTlcOGTu9ZtVfp88tRZ2mE-A";
 log_by_lua_block { collectgarbage() }
 --- response_body chomp
@@ -208,7 +208,7 @@ log_by_lua_block { collectgarbage() }
 proxy_ssl_verify on;
 proxy_ssl_trusted_certificate $TEST_NGINX_SERVER_ROOT/html/ca.crt;
 proxy_pass https://$server_addr:$apicast_port/t;
-proxy_set_header Host localhost;
+proxy_set_header Host test;
 proxy_set_header Authorization "Bearer eyJraWQiOiJzb21la2lkIiwiYWxnIjoiUlMyNTYifQ.eyJleHAiOjE5MjY4NzMwNTQsInN1YiI6InNvbWVvbmUiLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsiZGlyZWN0b3IiXX0sImZvbyI6IjEiLCJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2FwaWNhc3QiLCJhdWQiOiJhdWRpZW5jZSIsImNuZiI6eyJ4NXQjUzI1NiI6Ilk0X0xWbGtwRTZxa3NjUGJ0b0ttM2lpS0JnZndiT2ZiZEtCRWRuWjZaUFkifX0.Iin-tr6EVhEXjbj9R6xZSToBxZZBDXhl6i9ROw6SJQE6RWJeLt6mKK4jdTMVdxoZfm1J_NqayGJh3N99kHdIbA";
 log_by_lua_block { collectgarbage() }
 --- response_body chomp
@@ -278,7 +278,7 @@ proxy_ssl_trusted_certificate $TEST_NGINX_SERVER_ROOT/html/ca.crt;
 proxy_ssl_certificate $TEST_NGINX_SERVER_ROOT/html/client.crt;
 proxy_ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/client.key;
 proxy_pass https://$server_addr:$apicast_port/t;
-proxy_set_header Host localhost;
+proxy_set_header Host test;
 proxy_set_header Authorization "Bearer eyJraWQiOiJzb21la2lkIiwiYWxnIjoiUlMyNTYifQ.eyJleHAiOjE5MjQxMjU0MzQsInN1YiI6InNvbWVvbmUiLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsiZGlyZWN0b3IiXX0sImZvbyI6IjEiLCJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tL2F1dGgvcmVhbG1zL2FwaWNhc3QiLCJhdWQiOiJhdWRpZW5jZSJ9.GDYu4nT73_vPV4ZGa5DL8TAWZvn2TI47WxbXFH6wnUMn818slif-gUp_14pGleOR6VcLrEAC3VwEidtn08Ah8A";
 log_by_lua_block { collectgarbage() }
 --- response_body chomp

t/apicast-policy-routing.t

@@ -2476,8 +2476,8 @@ operations is an empty array instead of nil
 [error]
 
 
-=== TEST 36: Multiple requests that reuse the same TCP connection. 
-Routing must work as expected to the right backend
+=== TEST 36: Routing policy must choose correct backend 
+based on current configuration & request parameters
 --- env eval
 (
     'APICAST_HTTPS_PORT' => $ENV{APICAST_HTTPS_RANDOM_PORT},
@@ -2549,28 +2549,40 @@ content_by_lua_block {
   local resty_env = require 'resty.env'
 
   local https_port = resty_env.get('APICAST_HTTPS_PORT')
-  local uri_one = "https://127.0.0.1:".. https_port .."/one?user_key=foo"
-  local uri_two = "https://127.0.0.1:".. https_port .."/two?user_key=foo"
-
   local httpc = http.new()
-  local res1, err1 = httpc:request_uri(uri_one, {
+  httpc:connect("127.0.0.1", https_port)
+  httpc:ssl_handshake(nil, "127.0.0.1", false)
+
+  local responses, err = httpc:request_pipeline({
+    {
+      method = "GET",
+      path = "/one",
+      version = 1.1,
+      ssl_verify = false,
+      query = "?user_key=foo"
+    },
+    {
       method = "GET",
+      path = "/two",
+      version = 1.1,
       ssl_verify = false,
-      version = 1.1
+      query = "?user_key=foo"
+    }
   })
-  assert(res1, "Request failed"..(err1 or ""))
+
+  local res1 = responses[1]
+  res1.body = responses[1]:read_body()
+  local res2 = responses[2]
+  res2.body = responses[2]:read_body()
+
+  assert(res1, "Request failed"..(err or ""))
   assert(string.find(res1.body, "/foo/one"), "Expected: /foo/one, got: "..(res1.body or ""))
 
-  local res2, err2 = httpc:request_uri(uri_two, {
-    method = "GET",
-    ssl_verify = false, 
-    version = 1.1
-  })
-  assert(res2, "Request failed"..(err2 or ""))
-  --check for incorrect routing THREESCALE-8007:
+  assert(res2, "Request failed"..(err or ""))
+  --Check if incorrect routing as reported in THREESCALE-8007 is happening:
   assert(not string.find(res2.body, "/foo/two"), "Expected != .*/foo/two, got: "..(res2.body or "")) 
   assert(string.find(res2.body, "/bar/two"), "Expected: /bar/two, got: "..(res2.body or ""))
   httpc:close()
 }
 --- no_error_log
-[error]
+[error]