feat(*) make plugins backwards-compatible with APIs

Contains squashed changes to plugins from: - Aapo Talvensaari <[email protected]> - Enrique García Cota <[email protected]> - Hisham Muhammad <[email protected]> - Thibault Charbonnier <[email protected]> Signed-off-by: Thibault Charbonnier <[email protected]>
mengxk · Feb 20, 2018 · f50d654 · f50d654
1 parent b5b3804
commit f50d654
Show file tree

Hide file tree

Showing 21 changed files with 488 additions and 97 deletions.
diff --git a/kong/core/api_router.lua b/kong/core/api_router.lua
@@ -794,8 +794,6 @@ function _M.new(apis)
 
             local match_t     = {
               api             = matched_api.api,
-              service         = {},
-              route           = {},
               headers         = matched_api.headers,
               upstream_url_t  = upstream_url_t,
               upstream_scheme = upstream_url_t.scheme,

diff --git a/kong/core/handler.lua b/kong/core/handler.lua
@@ -474,8 +474,8 @@ return {
       end
 
       local api                = match_t.api or EMPTY_T
-      local route              = match_t.route
-      local service            = match_t.service
+      local route              = match_t.route or EMPTY_T
+      local service            = match_t.service or EMPTY_T
       local upstream_url_t     = match_t.upstream_url_t
 
       local realip_remote_addr = var.realip_remote_addr
@@ -565,10 +565,7 @@ return {
       end
 
       -- TODO: this needs to be removed when references to ctx.api are removed
-      ctx.api = {
-        id = api.id or route.id
-      }
-
+      ctx.api              = api
       ctx.service          = service
       ctx.route            = route
       ctx.router_matches   = match_t.matches

diff --git a/kong/plugins/datadog/handler.lua b/kong/plugins/datadog/handler.lua
@@ -88,9 +88,28 @@ local function log(premature, conf, message)
     return
   end
 
-  local name = string_gsub(message.service.name ~= ngx.null and
-                           message.service.name or message.service.host,
-                           "%.", "_")
+  local name
+
+  if message.service and message.service.name then
+    name = string_gsub(message.service.name ~= ngx.null and
+                       message.service.name or message.service.host,
+                       "%.", "_")
+
+  elseif message.api and message.api.name then
+    name = string_gsub(message.api.name, "%.", "_")
+
+  else
+    -- TODO: this follows the pattern used by
+    -- https://github.com/Kong/kong/pull/2702 (which prevents an error from
+    -- being thrown and avoids confusing reports as per our metrics keys), but
+    -- as it stands, hides traffic from monitoring tools when the plugin is
+    -- configured globally. In fact, this basically disables this plugin when
+    -- it is configured to run globally, or per-consumer without an
+    -- API/Route/Service.
+    ngx_log(ngx.DEBUG,
+            "[statsd] no Route/Service/API in context, skipping logging")
+    return
+  end
 
   local stat_name  = {
     request_size     = name .. ".request.size",
@@ -142,7 +161,8 @@ end
 function DatadogHandler:log(conf)
   DatadogHandler.super.log(self)
 
-  if not ngx.ctx.service then
+  if not ngx.ctx.service and
+     not ngx.ctx.api     then
     return
   end
 

diff --git a/kong/plugins/galileo/handler.lua b/kong/plugins/galileo/handler.lua
@@ -10,7 +10,7 @@ local Buffer = require "kong.plugins.galileo.buffer"
 local read_body = ngx.req.read_body
 local get_body_data = ngx.req.get_body_data
 
-local _alf_buffers = {} -- buffers per-route
+local _alf_buffers = {} -- buffers per-route / -api
 local _server_addr
 
 local GalileoHandler = BasePlugin:extend()
@@ -50,8 +50,8 @@ end
 function GalileoHandler:log(conf)
   GalileoHandler.super.log(self)
 
-  local ctx = ngx.ctx
-  local route_id = ctx.route.id
+
+  local route_id = conf.route_id or conf.api_id
 
   local buf = _alf_buffers[route_id]
   if not buf then
@@ -66,6 +66,8 @@ function GalileoHandler:log(conf)
   end
 
   local req_body, res_body
+
+  local ctx = ngx.ctx
   if ctx.galileo then
     req_body = ctx.galileo.req_body
     res_body = ctx.galileo.res_body

diff --git a/kong/plugins/ldap-auth/access.lua b/kong/plugins/ldap-auth/access.lua
@@ -90,7 +90,8 @@ local function authenticate(conf, given_credentials)
     return false
   end
 
-  local cache_key = "ldap_auth_cache:" .. ngx.ctx.route.id .. ":" .. given_username
+  local route_id = conf.route_id or conf.api_id
+  local cache_key = "ldap_auth_cache:" .. route_id .. ":" .. given_username
   local credential, err = singletons.cache:get(cache_key, {
     ttl = conf.cache_ttl,
     neg_ttl = conf.cache_ttl,

diff --git a/kong/plugins/log-serializers/basic.lua b/kong/plugins/log-serializers/basic.lua
@@ -40,6 +40,7 @@ function _M.serialize(ngx)
     authenticated_entity = authenticated_entity,
     route = ngx.ctx.route,
     service = ngx.ctx.service,
+    api = ngx.ctx.api,
     consumer = ngx.ctx.authenticated_consumer,
     client_ip = ngx.var.remote_addr,
     started_at = ngx.req.start_time() * 1000

diff --git a/kong/plugins/oauth2/access.lua b/kong/plugins/oauth2/access.lua
@@ -36,7 +36,7 @@ local GRANT_PASSWORD = "password"
 local ERROR = "error"
 local AUTHENTICATED_USERID = "authenticated_userid"
 
-local function generate_token(conf, service, credential, authenticated_userid, scope, state, expiration, disable_refresh)
+local function generate_token(conf, service, api, credential, authenticated_userid, scope, state, expiration, disable_refresh)
   local token_expiration = expiration or conf.token_expiration
 
   local refresh_token
@@ -49,12 +49,15 @@ local function generate_token(conf, service, credential, authenticated_userid, s
     refresh_token_ttl = conf.refresh_token_ttl
   end
 
-  local service_id
+  local service_id, api_id
   if not conf.global_credentials then
     service_id = service.id
+    api_id = api.id
   end
+
   local token, err = singletons.dao.oauth2_tokens:insert({
     service_id = service_id,
+    api_id = api_id,
     credential_id = credential.id,
     authenticated_userid = authenticated_userid,
     expires_in = token_expiration,
@@ -180,12 +183,14 @@ local function authorize(conf)
       -- If there are no errors, keep processing the request
       if not response_params[ERROR] then
         if response_type == CODE then
-          local service_id
+          local service_id, api_id
           if not conf.global_credentials then
             service_id = ngx.ctx.service.id
+            api_id = ngx.ctx.api.id
           end
           local authorization_code, err = singletons.dao.oauth2_authorization_codes:insert({
             service_id = service_id,
+            api_id = api_id,
             credential_id = client.id,
             authenticated_userid = parameters[AUTHENTICATED_USERID],
             scope = table.concat(scopes, " ")
@@ -200,7 +205,7 @@ local function authorize(conf)
           }
         else
           -- Implicit grant, override expiration to zero
-          response_params = generate_token(conf, ngx.ctx.service, client, parameters[AUTHENTICATED_USERID],  table.concat(scopes, " "), state, nil, true)
+          response_params = generate_token(conf, ngx.ctx.service, ngx.ctx.api, client, parameters[AUTHENTICATED_USERID],  table.concat(scopes, " "), state, nil, true)
           is_implicit_grant = true
         end
       end
@@ -312,17 +317,24 @@ local function issue_token(conf)
     if not response_params[ERROR] then
       if grant_type == GRANT_AUTHORIZATION_CODE then
         local code = parameters[CODE]
-        local service_id
+        local service_id, api_id
         if not conf.global_credentials then
           service_id = ngx.ctx.service.id
+          api_id = ngx.ctx.api.id
         end
-        local authorization_code = code and singletons.dao.oauth2_authorization_codes:find_all({service_id = service_id, code = code})[1]
+        local authorization_code = code and
+                                   singletons.dao.oauth2_authorization_codes:find_all({
+                                     code       = code,
+                                     api_id     = api_id,
+                                     service_id = service_id,
+                                   })[1]
         if not authorization_code then
           response_params = {[ERROR] = "invalid_request", error_description = "Invalid " .. CODE}
         elseif authorization_code.credential_id ~= client.id then
           response_params = {[ERROR] = "invalid_request", error_description = "Invalid " .. CODE}
         else
-          response_params = generate_token(conf, ngx.ctx.service, client, authorization_code.authenticated_userid, authorization_code.scope, state)
+          response_params = generate_token(conf, ngx.ctx.service, ngx.ctx.api, client,
+                                           authorization_code.authenticated_userid, authorization_code.scope, state)
           singletons.dao.oauth2_authorization_codes:delete({id=authorization_code.id}) -- Delete authorization code so it cannot be reused
         end
       elseif grant_type == GRANT_CLIENT_CREDENTIALS then
@@ -335,7 +347,8 @@ local function issue_token(conf)
           if not ok then
             response_params = scopes -- If it's not ok, then this is the error message
           else
-            response_params = generate_token(conf, ngx.ctx.service, client, parameters.authenticated_userid, table.concat(scopes, " "), state, nil, true)
+            response_params = generate_token(conf, ngx.ctx.service, ngx.ctx.api, client,
+                                             parameters.authenticated_userid, table.concat(scopes, " "), state, nil, true)
           end
         end
       elseif grant_type == GRANT_PASSWORD then
@@ -350,24 +363,32 @@ local function issue_token(conf)
           if not ok then
             response_params = scopes -- If it's not ok, then this is the error message
           else
-            response_params = generate_token(conf, ngx.ctx.service, client, parameters.authenticated_userid, table.concat(scopes, " "), state)
+            response_params = generate_token(conf, ngx.ctx.service, ngx.ctx.api, client,
+                                             parameters.authenticated_userid, table.concat(scopes, " "), state)
           end
         end
       elseif grant_type == GRANT_REFRESH_TOKEN then
         local refresh_token = parameters[REFRESH_TOKEN]
-        local service_id
+        local service_id, api_id
         if not conf.global_credentials then
           service_id = ngx.ctx.service.id
+          api_id = ngx.ctx.api.id
         end
-        local token = refresh_token and singletons.dao.oauth2_tokens:find_all({service_id = service_id, refresh_token = refresh_token})[1]
+        local token = refresh_token and
+                      singletons.dao.oauth2_tokens:find_all({
+                        refresh_token = refresh_token,
+                        api_id        = api_id,
+                        service_id    = service_id,
+                      })[1]
         if not token then
           response_params = {[ERROR] = "invalid_request", error_description = "Invalid " .. REFRESH_TOKEN}
         else
           -- Check that the token belongs to the client application
           if token.credential_id ~= client.id then
             response_params = {[ERROR] = "invalid_client", error_description = "Invalid client authentication"}
           else
-            response_params = generate_token(conf, ngx.ctx.service, client, token.authenticated_userid, token.scope, state)
+            response_params = generate_token(conf, ngx.ctx.service, ngx.ctx.api, client,
+                                             token.authenticated_userid, token.scope, state)
             singletons.dao.oauth2_tokens:delete({id=token.id}) -- Delete old token
           end
         end
@@ -387,12 +408,17 @@ local function issue_token(conf)
   })
 end
 
-local function load_token_into_memory(conf, service, access_token)
-  local service_id
+local function load_token_into_memory(conf, service, api, access_token)
+  local service_id, api_id
   if not conf.global_credentials then
     service_id = service.id
+    api_id     = api.id
   end
-  local credentials, err = singletons.dao.oauth2_tokens:find_all { service_id = service_id, access_token = access_token }
+  local credentials, err = singletons.dao.oauth2_tokens:find_all {
+    access_token = access_token,
+    service_id   = service_id,
+    api_id       = api_id,
+  }
   local result
   if err then
     return nil, err
@@ -407,7 +433,8 @@ local function retrieve_token(conf, access_token)
   if access_token then
     local token_cache_key = singletons.dao.oauth2_tokens:cache_key(access_token)
     token, err = singletons.cache:get(token_cache_key, nil,
-                                      load_token_into_memory, conf, ngx.ctx.service,
+                                      load_token_into_memory, conf,
+                                      ngx.ctx.service, ngx.ctx.api,
                                       access_token)
     if err then
       return responses.send_HTTP_INTERNAL_SERVER_ERROR(err)
@@ -506,7 +533,10 @@ local function do_authentication(conf)
     return false, {status = 401, message = {[ERROR] = "invalid_token", error_description = "The access token is invalid or has expired"}, headers = {["WWW-Authenticate"] = 'Bearer realm="service" error="invalid_token" error_description="The access token is invalid or has expired"'}}
   end
 
-  if (token.service_id and ngx.ctx.service.id ~= token.service_id) or (token.service_id == nil and not conf.global_credentials) then
+  if (token.service_id and ngx.ctx.service.id ~= token.service_id)
+  or (token.api_id and ngx.ctx.api.id ~= token.api_id)
+  or (token.service_id == nil and token.api_id == nil and not conf.global_credentials)
+  then
     return false, {status = 401, message = {[ERROR] = "invalid_token", error_description = "The access token is invalid or has expired"}, headers = {["WWW-Authenticate"] = 'Bearer realm="service" error="invalid_token" error_description="The access token is invalid or has expired"'}}
   end
 
@@ -544,6 +574,7 @@ end
 
 function _M.execute(conf)
 
+
   if ngx.ctx.authenticated_credential and conf.anonymous ~= "" then
     -- we're already authenticated, and we're configured for using anonymous,
     -- hence we're in a logical OR between auth methods and we're already done.

diff --git a/kong/plugins/rate-limiting/migrations/cassandra.lua b/kong/plugins/rate-limiting/migrations/cassandra.lua
@@ -62,11 +62,12 @@ return {
       CREATE TABLE ratelimiting_metrics(
         route_id uuid,
         service_id uuid,
+        api_id uuid,
         identifier text,
         period text,
         period_date timestamp,
         value counter,
-        PRIMARY KEY ((route_id, service_id, identifier, period_date, period))
+        PRIMARY KEY ((route_id, service_id, api_id, identifier, period_date, period))
       );
     ]],
     down = nil,

diff --git a/kong/plugins/rate-limiting/migrations/postgres.lua b/kong/plugins/rate-limiting/migrations/postgres.lua
@@ -76,8 +76,11 @@ return {
   {
     name = "2017-11-30-120000_add_route_and_service_id",
     up = [[
-      ALTER TABLE ratelimiting_metrics ADD COLUMN route_id uuid;
-      ALTER TABLE ratelimiting_metrics ADD COLUMN service_id uuid;
+      ALTER TABLE ratelimiting_metrics DROP CONSTRAINT ratelimiting_metrics_pkey;
+      ALTER TABLE ratelimiting_metrics ALTER COLUMN api_id SET DEFAULT '00000000000000000000000000000000';
+      ALTER TABLE ratelimiting_metrics ADD COLUMN route_id uuid NOT NULL DEFAULT '00000000000000000000000000000000';
+      ALTER TABLE ratelimiting_metrics ADD COLUMN service_id uuid NOT NULL DEFAULT '00000000000000000000000000000000';
+      ALTER TABLE ratelimiting_metrics ADD PRIMARY KEY (api_id, route_id, service_id, identifier, period_date, period);
 
       CREATE OR REPLACE FUNCTION increment_rate_limits(r_id uuid, s_id uuid, i text, p text, p_date timestamp with time zone, v integer) RETURNS VOID AS $$
       BEGIN
@@ -101,14 +104,31 @@ return {
         END LOOP;
       END;
       $$ LANGUAGE 'plpgsql';
+      CREATE OR REPLACE FUNCTION increment_rate_limits_api(a_id uuid, i text, p text, p_date timestamp with time zone, v integer) RETURNS VOID AS $$
+      BEGIN
+        LOOP
+          UPDATE ratelimiting_metrics SET value = value + v WHERE api_id = a_id AND identifier = i AND period = p AND period_date = p_date;
+          IF found then
+            RETURN;
+          END IF;
+
+          BEGIN
+            INSERT INTO ratelimiting_metrics(api_id, period, period_date, identifier, value) VALUES(a_id, p, p_date, i, v);
+            RETURN;
+          EXCEPTION WHEN unique_violation THEN
+
+          END;
+        END LOOP;
+      END;
+      $$ LANGUAGE 'plpgsql';
     ]],
     down = nil,
   },
-  {
-    name = "2017-11-30-130000_remove_api_id",
-    up = [[
-      ALTER TABLE ratelimiting_metrics DROP COLUMN api_id;
-    ]],
-    down = nil,
-  },
+--  {
+--    name = "2017-11-30-130000_remove_api_id",
+--    up = [[
+--      ALTER TABLE ratelimiting_metrics DROP COLUMN api_id;
+--    ]],
+--    down = nil,
+--  },
 }