Merge pull request #535 from 3scale/content-phase

introduce content phase

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Liquid template can find files in current folder too [PR #533](https://github.com/3scale/apicast/pull/533)
 - `bin/apicast` respects `APICAST_OPENRESTY_BINARY` and `TEST_NGINX_BINARY` environment [PR #540](https://github.com/3scale/apicast/pull/540)
 - Caching policy [PR #546](https://github.com/3scale/apicast/pull/546)
+- New phase: `content` for generating content or getting the upstream response [PR #535](https://github.com/3scale/apicast/pull/535)
 
 ## Fixed
 
@@ -38,6 +39,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Propagate full package.path and cpath from the CLI to Nginx [PR #538](https://github.com/3scale/apicast/pull/538)
 - `post_action` phase now shares `ngx.ctx` with the main request [PR #539](https://github.com/3scale/apicast/pull/539)
 - Decrease nginx timer resolution to improve performance and enable PCRE JIT [PR #543](https://github.com/3scale/apicast/pull/543)
+- Moved `proxy_pass` into new internal location `@upstream` [PR #535](https://github.com/3scale/apicast/pull/535)
 
 ## [3.2.0-alpha2] - 2017-11-30
 

gateway/conf.d/apicast.conf

@@ -59,6 +59,28 @@ location @out_of_band_authrep_action {
   }
 }
 
+location @upstream {
+  internal;
+
+  rewrite_by_lua_block {
+    require('resty.ctx').apply()
+  }
+
+  proxy_pass $proxy_pass;
+
+  proxy_http_version 1.1;
+  proxy_set_header X-Real-IP  $remote_addr;
+  proxy_set_header Host $http_host;
+  proxy_set_header X-3scale-proxy-secret-token $secret_token;
+  proxy_set_header X-3scale-debug "";
+  proxy_set_header Connection "";
+
+  # these are duplicated so when request is redirected here those phases are executed
+  post_action @out_of_band_authrep_action;
+  body_filter_by_lua_block { require('apicast.executor'):body_filter() }
+  header_filter_by_lua_block { require('apicast.executor'):header_filter() }
+}
+
 location / {
   set $cached_key null;
   set $credentials null;
@@ -85,20 +107,13 @@ location / {
     require('apicast.executor'):rewrite()
   }
   access_by_lua_block { require('apicast.executor'):access() }
-  body_filter_by_lua_block { require('apicast.executor'):body_filter() }
-  header_filter_by_lua_block { require('apicast.executor'):header_filter() }
 
   content_by_lua_block { require('apicast.executor'):content() }
 
-  proxy_pass $proxy_pass;
-  proxy_http_version 1.1;
-  proxy_set_header X-Real-IP  $remote_addr;
-  proxy_set_header Host $http_host;
-  proxy_set_header X-3scale-proxy-secret-token $secret_token;
-  proxy_set_header X-3scale-debug "";
-  proxy_set_header Connection "";
-
+  # these are duplicated so those phases are executed when no internal redirect happens
   post_action @out_of_band_authrep_action;
+  body_filter_by_lua_block { require('apicast.executor'):body_filter() }
+  header_filter_by_lua_block { require('apicast.executor'):header_filter() }
 
   include ../apicast.d/location.d/*.conf;
 }

gateway/src/apicast/executor.lua

@@ -19,6 +19,7 @@ local mt = { __index = _M }
 -- forward all policy methods to the policy chain
 for _,phase in policy.phases() do
     _M[phase] = function(self, ...)
+        ngx.log(ngx.DEBUG, 'executor phase: ', phase)
         return self.policy_chain[phase](self.policy_chain, self:context(phase), ...)
     end
 end

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

@@ -82,6 +82,12 @@ function _M:access(context)
   return ok, err
 end
 
+_M.content = function()
+  if not ngx.headers_sent then
+    ngx.exec("@upstream")
+  end
+end
+
 _M.body_filter = noop
 _M.header_filter = noop
 

gateway/src/apicast/policy/echo/policy.lua

@@ -19,13 +19,15 @@ function _M.new(configuration)
   return policy
 end
 
+function _M.content()
+  ngx.say(ngx.var.request)
+end
+
 function _M:rewrite()
   if self.status then
     ngx.status = self.status
   end
 
-  ngx.say(ngx.var.request)
-
   if self.exit == 'request' then
     return ngx.exit(ngx.status)
   elseif self.exit == 'phase' then

gateway/src/apicast/policy/policy.lua

@@ -10,7 +10,8 @@ local _M = { }
 
 local PHASES = {
     'init', 'init_worker',
-    'rewrite', 'access', 'balancer',
+    'rewrite', 'access',
+    'content', 'balancer',
     'header_filter', 'body_filter',
     'post_action',  'log'
 }

gateway/src/apicast/policy_chain.lua

@@ -143,6 +143,7 @@ end
 local function call_chain(phase_name)
     return function(self, ...)
         for i=1, #self do
+            ngx.log(ngx.DEBUG, 'policy chain execute phase: ', phase_name, ', policy: ', self[i]._NAME, ', i: ', i)
             self[i][phase_name](self[i], ...)
         end
     end

spec/policy_spec.lua

@@ -3,7 +3,8 @@ local policy = require 'apicast.policy'
 describe('policy', function()
   local phases = {
     'init', 'init_worker',
-    'rewrite', 'access', 'balancer',
+    'rewrite', 'access',
+    'content', 'balancer',
     'header_filter', 'body_filter',
     'post_action',  'log'
   }

t/apicast-policy-chains.t

@@ -8,11 +8,8 @@ __DATA__
 === TEST 1: custom policy chain
 This test uses the phase logger policy to verify that all of its phases are run
 when we use a policy chain that contains it. The policy chain also contains the
-normal apicast policy, so we can check that the authorize flow continues
-working. Notice that 'post_action' and 'log' are not ran. There's no
-post_action defined and log only runs when post_action runs.
-init and init_worker are not run either. They are not executed in policies
-defined at the service level.
+normal apicast policy, so we can check that the authorize flow continues working.
+Phases init and init_worker are not executed for policies defined at the service level.
 
 --- http_config
   include $TEST_NGINX_UPSTREAM_CONFIG;
@@ -62,7 +59,70 @@ yay, api backend
 --- error_log chomp
 running phase: rewrite
 running phase: access
+running phase: content
 running phase: balancer
 running phase: header_filter
 running phase: body_filter
 running phase: post_action
+running phase: log
+
+
+
+=== TEST 2: custom policy chain responds with content
+This tests uses phase logger policy to verify all needed phases are executed.
+When some policy responds with content header_filter, body_filter and post_actions should
+still be executed.
+
+--- http_config
+  include $TEST_NGINX_UPSTREAM_CONFIG;
+  lua_package_path "$TEST_NGINX_LUA_PATH";
+  init_by_lua_block {
+    require('apicast.configuration_loader').mock({
+      services = {
+        {
+          id = 42,
+          backend_version = 1,
+          backend_authentication_type = 'service_token',
+          backend_authentication_value = 'token-value',
+          proxy = {
+            policy_chain = { { name = 'apicast.policy.phase_logger' }, { name = 'apicast.policy.echo' } },
+            api_backend = "http://127.0.0.1:$TEST_NGINX_SERVER_PORT/api-backend/",
+            proxy_rules = {
+              {
+                http_method = "GET",
+                pattern = "/test",
+                metric_system_name = "hits",
+                delta = 1
+              }
+            }
+          }
+        }
+      }
+    })
+  }
+
+--- config
+  include $TEST_NGINX_APICAST_CONFIG;
+
+  location /transactions/authrep.xml {
+    content_by_lua_block { ngx.exit(200) }
+  }
+
+  location /api-backend/ {
+     echo 'yay, api backend';
+  }
+--- request
+GET /test
+--- response_body
+GET /test HTTP/1.1
+--- error_code: 200
+--- no_error_log
+[error]
+--- error_log chomp
+running phase: rewrite
+running phase: access
+running phase: content
+running phase: header_filter
+running phase: body_filter
+running phase: post_action
+running phase: log

t/apicast.t

@@ -107,8 +107,6 @@ No Mapping Rule matched
 Content-Type: text/plain; charset=utf-8
 no mapping rules!
 --- error_code: 404
---- error_log
-skipping after action, no cached key
 
 === TEST 5: no mapping rules matched configurable error
 The message is configurable and status also.
@@ -135,8 +133,6 @@ GET /?user_key=value
 --- response_body chomp
 no mapping rules!
 --- error_code: 412
---- error_log
-skipping after action, no cached key
 
 === TEST 6: authentication credentials invalid default error
 There are defaults defined for the error message, the content-type, and the

t/deprecation-warnings.t

@@ -15,17 +15,26 @@ APIcast should emit deprecation warnings when loading code using the old paths.
       "backend_version": 1,
       "proxy": {
         "policy_chain": [
-	  { "name": "policy.echo" },
-	  { "name": "apicast" }
-	]
+          { "name": "policy.echo" },
+          { "name": "apicast" }
+        ],
+        "proxy_rules": [
+          { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 }
+        ]
       }
     }
   ]
 }
+--- backend
+  location /transactions/authrep.xml {
+    content_by_lua_block {
+      ngx.exit(200)
+    }
+  }
 --- request
-GET /echo
+GET /echo?user_key=foo
 --- response_body
-GET /echo HTTP/1.1
+GET /echo?user_key=foo HTTP/1.1
 --- error_code: 200
 --- no_error_log
 [error]