Merge pull request #1328 from samugi/THREESCALE-8000

THREESCALE-8000 + THREESCALE-8007 + THREESCALE-8252 (clear context policy)
3scale · Mar 15, 2022 · 263b6a6 · 263b6a6
2 parents f018994 + 1072ef9
commit 263b6a6
Show file tree

Hide file tree

Showing 10 changed files with 382 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed issues with OIDC filters [PR #1304](https://github.com/3scale/APIcast/pull/1304) [THREESCALE-6042](https://issues.redhat.com/browse/THREESCALE-6042)
 - Fixed issues with Upstream MTLS certs [PR #1307](https://github.com/3scale/APIcast/pull/1307) [THREESCALE-7508](https://issues.redhat.com/browse/THREESCALE-7508)
 - Fixed warning messages [PR #1318](https://github.com/3scale/APIcast/pull/1318) [THREESCALE-7906](https://issues.redhat.com/browse/THREESCALE-7906)
+- Fixed dirty context [PR #1328](https://github.com/3scale/APIcast/pull/1328) [THREESCALE-8000](https://issues.redhat.com/browse/THREESCALE-8000) [THREESCALE-8007](https://issues.redhat.com/browse/THREESCALE-8007)
 
 ### Added
 

diff --git a/gateway/src/apicast/policy/clear_context/clear_context.lua b/gateway/src/apicast/policy/clear_context/clear_context.lua
@@ -0,0 +1,20 @@
+local _M = require('apicast.policy').new('Clear Context')
+local new = _M.new
+
+function _M.new(...)
+  return new(...)
+end
+
+function _M:ssl_certificate(context)
+  --resetting the context after every other policy in the chain
+  --has executed their ssl_certificate phase. 
+  clear_table(ngx.ctx)
+end
+
+function clear_table(t)
+  for k, _ in pairs(t) do
+    t[k] = nil
+  end
+end
+
+return _M
diff --git a/gateway/src/apicast/policy/clear_context/init.lua b/gateway/src/apicast/policy/clear_context/init.lua
@@ -0,0 +1 @@
+return require('clear_context')
diff --git a/gateway/src/apicast/policy_chain.lua b/gateway/src/apicast/policy_chain.lua
@@ -70,7 +70,8 @@ local DEFAULT_POLICIES = {
     '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.

diff --git a/spec/policy/clear_context/clear_context_spec.lua b/spec/policy/clear_context/clear_context_spec.lua
@@ -0,0 +1,26 @@
+local ClearContextPolicy = require('apicast.policy.clear_context')
+local ngx_variable = require('apicast.policy.ngx_variable')
+
+
+describe('Clear Context policy', function()
+  local current_ctx = {some_key = 'some_value'}
+
+  describe('log phase context reset', function()
+    before_each(function()
+      ctx = ngx.ctx
+      stub(ngx_variable, 'available_context', function(context) return context end)
+    end)
+
+    context('.ssl_certificate', function()
+      local clear_context_policy = ClearContextPolicy.new()
+
+      it('clears the context', function()
+        ctx.current = current_ctx
+        clear_context_policy:ssl_certificate(ctx)
+        assert.is_nil(ctx.current)
+      end)
+    end)
+  end)
+end)
+
+
diff --git a/t/apicast-path-routing.t b/t/apicast-path-routing.t
@@ -1,6 +1,8 @@
 use lib 't';
 use Test::APIcast::Blackbox 'no_plan';
 
+$ENV{APICAST_HTTPS_RANDOM_PORT} = Test::APIcast::get_random_port();
+
 run_tests();
 
 __DATA__
@@ -369,4 +371,112 @@ Host: one
 --- error_code eval
 [200, 404]
 --- no_error_log
-[error]
+[error]
+
+=== TEST 6: APIcast must choose correct Product
+based on current configuration & request parameters
+--- env eval
+(
+  'APICAST_HTTPS_PORT' => $ENV{APICAST_HTTPS_RANDOM_PORT},
+  'APICAST_PATH_ROUTING' => '1',
+  '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/foo/",
+        "hosts": [
+          "127.0.0.1"
+        ],
+        "policy_chain": [
+          {
+            "name": "apicast.policy.apicast"
+          }
+        ],
+        "proxy_rules": [
+          {
+            "pattern": "/one",
+            "http_method": "GET",
+            "metric_system_name": "hits",
+            "delta": 1
+          }
+        ]
+      }
+    },
+    {
+      "id": 21,
+      "backend_version": 1,
+      "proxy": {
+        "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api-backend/bar/",
+        "hosts": [
+          "127.0.0.1"
+        ],
+        "proxy_rules": [
+          {
+            "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(res1 and res1.status, "Request failed: "..(err or ""))
+  assert(string.find(res1.body, "/foo/one"), "Expected .*/foo/one, got: "..(res1.body or ""))
+  
+  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]
diff --git a/t/apicast-policy-logging.t b/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/]
diff --git a/t/apicast-policy-oauth-mtls.t b/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