fix(request-transformer) ensure feature flags are either both set or …

…both unset (#3) * fix(request-transformer) ensure REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE is set if REQUEST_TRANSFORMER_ADVANCED_ENABLE_LIMIT_BODY is turned on * fix(request-transformer) add error log for invalid number, fix tests * fix(request-transformer) fix typo, add tests for error logs
Kong · Jun 20, 2018 · 4639306 · 4639306
1 parent f8e5a50
commit 4639306
Show file tree

Hide file tree

Showing 5 changed files with 190 additions and 6 deletions.
diff --git a/kong/plugins/request-transformer-advanced/feature_flags/limit_body.lua b/kong/plugins/request-transformer-advanced/feature_flags/limit_body.lua
@@ -1,5 +1,5 @@
 local utils  = require "kong.tools.utils"
-local feature_flags, FLAGS, VALUES
+local FLAGS, VALUES
 
 
 local ok, feature_flags = utils.load_module_if_exists("kong.enterprise_edition.feature_flags")
@@ -23,10 +23,23 @@ local function init_worker()
     return true
   end
 
-  local limit = tonumber(feature_flags.get_feature_value(VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE))
-  if limit then
-    rt_body_size_limit = limit
+  local res, _ = feature_flags.get_feature_value(VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE)
+  if not res then
+    ngx_log(ngx_ERR, string.format("\"%s\" is turned on but \"%s\" is not defined",
+                                FLAGS.REQUEST_TRANSFORMER_ADVANCED_ENABLE_LIMIT_BODY,
+                                VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE))
+    return false
   end
+
+  local limit = tonumber(res)
+  if not limit then
+    ngx_log(ngx_ERR, string.format("\"%s\" is not valid number for \"%s\"",
+                                res,
+                                VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE))
+    return false
+  end
+
+  rt_body_size_limit = limit
   return true
 end
 

diff --git a/spec/04-feature_flag_limit_body_spec.lua b/spec/04-feature_flag_limit_body_spec.lua
@@ -1,4 +1,8 @@
 local helpers = require "spec.helpers"
+local feature_flags = require "kong.enterprise_edition.feature_flags"
+local VALUES = feature_flags.VALUES
+
+local pl_file = require "pl.file"
 
 
 describe("Plugin: request-transformer-advanced(feature_flags) ", function()
@@ -23,8 +27,7 @@ describe("Plugin: request-transformer-advanced(feature_flags) ", function()
 
     assert(helpers.start_kong({
       nginx_conf = "spec/fixtures/custom_nginx.template",
-      feature_conf_path = "spec/fixtures/ee/feature_request_transformer_advanced_limit_body.conf",
-      enabled_plugins = "request-transformer-advanced",
+      feature_conf_path = "spec/fixtures/ee/request_transformer_advanced/feature_request_transformer_advanced_limit_body.conf",
       custom_plugins = "request-transformer-advanced",
     }))
   end)
@@ -62,6 +65,15 @@ describe("Plugin: request-transformer-advanced(feature_flags) ", function()
       assert.equals("world", value)
       local value = assert.request(res).has.formparam("p1")
       assert.equals("v1", value)
+
+      -- make sure there's no error
+      local err_log = pl_file.read(helpers.test_conf.nginx_err_logs)
+      assert.not_matches(string.format(
+                      "is not valid number for \"%s\"", VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE),
+                      err_log, nil, true)
+      assert.not_matches(string.format(
+                      "is turned on but \"%s\" is not defined", VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE),
+                      err_log, nil, true)
     end)
   end)
   it("doesn't change body if request body size is bigger than limit", function()
@@ -81,5 +93,159 @@ describe("Plugin: request-transformer-advanced(feature_flags) ", function()
     local value = assert.request(res).has.formparam("hello")
     assert.equals(payload, value)
     assert.request(res).has.no.formparam("p1")
+
+    -- make sure there's no error
+    local err_log = pl_file.read(helpers.test_conf.nginx_err_logs)
+    assert.not_matches(string.format(
+                    "is turned on but \"%s\" is not defined", VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE),
+                    err_log, nil, true)
+    assert.not_matches(string.format(
+                    "is not valid number for \"%s\"", VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE),
+                    err_log, nil, true)
+  end)
+end)
+
+describe("Plugin: request-transformer-advanced(feature_flags) ", function()
+  local proxy_client
+
+  setup(function()
+    local bp = helpers.get_db_utils()
+
+    local route1 = bp.routes:insert({
+      hosts = { "test1.com" },
+    })
+
+    bp.plugins:insert {
+      route_id = route1.id,
+      name     = "request-transformer-advanced",
+      config   = {
+        add = {
+          body        = {"p1:v1"}
+        }
+      }
+    }
+
+    assert(helpers.start_kong({
+      nginx_conf = "spec/fixtures/custom_nginx.template",
+      feature_conf_path = "spec/fixtures/ee/request_transformer_advanced/feature_request_transformer_advanced_limit_body-body_size_not_defined.conf",
+      custom_plugins = "request-transformer-advanced",
+    }))
+  end)
+
+  teardown(function()
+    helpers.stop_kong()
+  end)
+
+  before_each(function()
+    proxy_client = helpers.proxy_client()
+  end)
+
+  after_each(function()
+    if proxy_client then
+      proxy_client:close()
+    end
+  end)
+
+
+  describe("with feature_flag request_transformation_advanced_limit_body on", function()
+    it("doesn't enable if request_transformation_advanced_limit_body_size is not defined", function()
+      local payload = string.rep("*", 128)
+      local res = assert(proxy_client:send {
+        method  = "POST",
+        path    = "/request",
+        body    = {
+          hello = payload,
+        },
+        headers = {
+          ["Content-Type"] = "application/x-www-form-urlencoded",
+          host             = "test1.com"
+        }
+      })
+      assert.response(res).has.status(200)
+      -- sanity test
+      local value = assert.request(res).has.formparam("hello")
+      assert.equals(payload, value)
+      -- transforms body
+      local value = assert.request(res).has.formparam("p1")
+      assert.equals("v1", value)
+
+      local err_log = pl_file.read(helpers.test_conf.nginx_err_logs)
+      assert.matches(string.format(
+                        "is turned on but \"%s\" is not defined", VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE),
+                        err_log, nil, true)
+    end)
   end)
 end)
+
+describe("Plugin: request-transformer-advanced(feature_flags) ", function()
+  local proxy_client
+
+  setup(function()
+    local bp = helpers.get_db_utils()
+
+    local route1 = bp.routes:insert({
+      hosts = { "test1.com" },
+    })
+
+    bp.plugins:insert {
+      route_id = route1.id,
+      name     = "request-transformer-advanced",
+      config   = {
+        add = {
+          body        = {"p1:v1"}
+        }
+      }
+    }
+
+    assert(helpers.start_kong({
+      nginx_conf = "spec/fixtures/custom_nginx.template",
+      feature_conf_path = "spec/fixtures/ee/request_transformer_advanced/feature_request_transformer_advanced_limit_body-body_size_invalid.conf",
+      custom_plugins = "request-transformer-advanced",
+    }))
+  end)
+
+  teardown(function()
+    helpers.stop_kong()
+  end)
+
+  before_each(function()
+    proxy_client = helpers.proxy_client()
+  end)
+
+  after_each(function()
+    if proxy_client then
+      proxy_client:close()
+    end
+  end)
+
+
+  describe("with feature_flag request_transformation_advanced_limit_body on", function()
+    it("doesn't enable if request_transformation_advanced_limit_body_size is invalid", function()
+      local payload = string.rep("*", 128)
+      local res = assert(proxy_client:send {
+        method  = "POST",
+        path    = "/request",
+        body    = {
+          hello = payload,
+        },
+        headers = {
+          ["Content-Type"] = "application/x-www-form-urlencoded",
+          host             = "test1.com"
+        }
+      })
+      assert.response(res).has.status(200)
+      -- sanity test
+      local value = assert.request(res).has.formparam("hello")
+      assert.equals(payload, value)
+      -- transforms body
+      local value = assert.request(res).has.formparam("p1")
+      assert.equals("v1", value)
+
+      local err_log = pl_file.read(helpers.test_conf.nginx_err_logs)
+      assert.matches(string.format(
+                      "is not valid number for \"%s\"", VALUES.REQUEST_TRANSFORMER_ADVANCED_LIMIT_BODY_SIZE),
+                      err_log, nil, true)
+    end)
+  end)
+end)
+
diff --git a/...ansformer_advanced/feature_request_transformer_advanced_limit_body-body_size_invalid.conf b/...ansformer_advanced/feature_request_transformer_advanced_limit_body-body_size_invalid.conf
@@ -0,0 +1,3 @@
+request_transformation_advanced_enable_limit_body=on
+request_transformation_advanced_limit_body_size=wow
+
diff --git a/...ormer_advanced/feature_request_transformer_advanced_limit_body-body_size_not_defined.conf b/...ormer_advanced/feature_request_transformer_advanced_limit_body-body_size_not_defined.conf
@@ -0,0 +1,2 @@
+request_transformation_advanced_enable_limit_body=on
+
diff --git a/...uest_transformer_advanced_limit_body.conf → ...uest_transformer_advanced_limit_body.conf b/...uest_transformer_advanced_limit_body.conf → ...uest_transformer_advanced_limit_body.conf
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		request_transformation_advanced_enable_limit_body=on
		request_transformation_advanced_limit_body_size=wow
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		request_transformation_advanced_enable_limit_body=on