From b1cda3164bc4725bb01a3ee7b0944aafc3dace7d Mon Sep 17 00:00:00 2001 From: samugi Date: Wed, 7 Dec 2022 16:49:11 +0100 Subject: [PATCH 1/3] fix(declarative-config): set hash when config is loaded Ensure the configuration hash field is correctly configured when the declarative configuration is loaded on startup. --- kong/init.lua | 23 +++---- .../02-cmd/02-start_stop_spec.lua | 63 ++++++++++++++++++- .../04-admin_api/02-kong_routes_spec.lua | 9 ++- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/kong/init.lua b/kong/init.lua index a7e9b81d55c7..bf3cf118f2d7 100644 --- a/kong/init.lua +++ b/kong/init.lua @@ -134,6 +134,7 @@ local CTX_NREC = 50 -- normally Kong has ~32 keys in ctx local declarative_entities local declarative_meta +local declarative_hash local schema_state @@ -448,15 +449,15 @@ local function parse_declarative_config(kong_config) if not has_declarative_config(kong_config) then -- return an empty configuration, -- including only the default workspace - local entities, _, _, meta = dc:parse_table({ _format_version = "2.1" }) - return entities, nil, meta + local entities, _, _, meta, hash = dc:parse_table({ _format_version = "2.1" }) + return entities, nil, meta, hash end - local entities, err, _, meta + local entities, err, _, meta, hash if kong_config.declarative_config ~= nil then - entities, err, _, meta = dc:parse_file(kong_config.declarative_config) + entities, err, _, meta, hash = dc:parse_file(kong_config.declarative_config) elseif kong_config.declarative_config_string ~= nil then - entities, err, _, meta = dc:parse_string(kong_config.declarative_config_string) + entities, err, _, meta, hash = dc:parse_string(kong_config.declarative_config_string) end if not entities then @@ -469,7 +470,7 @@ local function parse_declarative_config(kong_config) end end - return entities, nil, meta + return entities, nil, meta, hash end @@ -491,7 +492,7 @@ local function declarative_init_build() end -local function load_declarative_config(kong_config, entities, meta) +local function load_declarative_config(kong_config, entities, meta, hash) local opts = { name = "declarative_config", } @@ -502,8 +503,7 @@ local function load_declarative_config(kong_config, entities, meta) if value then return true end - - local ok, err = declarative.load_into_cache(entities, meta) + local ok, err = declarative.load_into_cache(entities, meta, hash) if not ok then return nil, err end @@ -622,7 +622,7 @@ function Kong.init() #config.status_listeners == 0) then local err - declarative_entities, err, declarative_meta = parse_declarative_config(kong.configuration) + declarative_entities, err, declarative_meta, declarative_hash = parse_declarative_config(kong.configuration) if not declarative_entities then error(err) end @@ -752,7 +752,8 @@ function Kong.init_worker() elseif declarative_entities then ok, err = load_declarative_config(kong.configuration, declarative_entities, - declarative_meta) + declarative_meta, + declarative_hash) if not ok then stash_init_worker_error("failed to load declarative config file: " .. err) return diff --git a/spec/02-integration/02-cmd/02-start_stop_spec.lua b/spec/02-integration/02-cmd/02-start_stop_spec.lua index 858625477068..b3faca29c943 100644 --- a/spec/02-integration/02-cmd/02-start_stop_spec.lua +++ b/spec/02-integration/02-cmd/02-start_stop_spec.lua @@ -1,5 +1,6 @@ -local helpers = require "spec.helpers" - +local helpers = require "spec.helpers" +local constants = require "kong.constants" +local cjson = require "cjson" for _, strategy in helpers.each_strategy() do @@ -472,6 +473,64 @@ describe("kong start/stop #" .. strategy, function() return ok end, 10) end) + + it("configuration hash is set correctly", function() + local yaml_file = helpers.make_yaml_file [[ + _format_version: "1.1" + services: + - name: my-service + url: http://127.0.0.1:15555 + routes: + - name: example-route + hosts: + - example.test + ]] + + local admin_client, json_body + + finally(function() + os.remove(yaml_file) + helpers.stop_kong(helpers.test_conf.prefix) + if admin_client then + admin_client:close() + end + end) + + assert(helpers.start_kong({ + database = "off", + declarative_config = yaml_file, + nginx_conf = "spec/fixtures/custom_nginx.template", + })) + + helpers.wait_until(function() + helpers.wait_until(function() + local pok + pok, admin_client = pcall(helpers.admin_client) + return pok + end, 10) + + local res = assert(admin_client:send { + method = "GET", + path = "/status" + }) + if res.status ~= 200 then + return false + end + local body = assert.res_status(200, res) + json_body = cjson.decode(body) + + if admin_client then + admin_client:close() + admin_client = nil + end + + return true + end, 10) + + assert.is_string(json_body.configuration_hash) + assert.equals(32, #json_body.configuration_hash) + assert.not_equal(constants.DECLARATIVE_EMPTY_CONFIG_HASH, json_body.configuration_hash) + end) end) end diff --git a/spec/02-integration/04-admin_api/02-kong_routes_spec.lua b/spec/02-integration/04-admin_api/02-kong_routes_spec.lua index d2356072c7b5..98d63efcdc0c 100644 --- a/spec/02-integration/04-admin_api/02-kong_routes_spec.lua +++ b/spec/02-integration/04-admin_api/02-kong_routes_spec.lua @@ -1,5 +1,6 @@ local helpers = require "spec.helpers" local cjson = require "cjson" +local constants = require "kong.constants" local UUID_PATTERN = "%x%x%x%x%x%x%x%x%-%x%x%x%x%-%x%x%x%x%-%x%x%x%x%-%x%x%x%x%x%x%x%x%x%x%x%x" @@ -188,6 +189,8 @@ describe("Admin API - Kong routes with strategy #" .. strategy, function() end) describe("/status", function() + local empty_config_hash = constants.DECLARATIVE_EMPTY_CONFIG_HASH + it("returns status info", function() local res = assert(client:send { method = "GET", @@ -208,7 +211,9 @@ describe("Admin API - Kong routes with strategy #" .. strategy, function() assert.is_number(json.server.connections_waiting) assert.is_number(json.server.total_requests) if strategy == "off" then - assert.is_equal(string.rep("0", 32), json.configuration_hash) -- all 0 in DBLESS mode until configuration is applied + assert.is_string(json.configuration_hash) + assert.equal(32, #json.configuration_hash) + assert.is_not_equal(empty_config_hash, json.configuration_hash) else assert.is_nil(json.configuration_hash) -- not present in DB mode end @@ -251,7 +256,7 @@ describe("Admin API - Kong routes with strategy #" .. strategy, function() assert.is_number(json.server.total_requests) assert.is_string(json.configuration_hash) assert.equal(32, #json.configuration_hash) - + assert.is_not_equal(empty_config_hash, json.configuration_hash) end) it("database.reachable is `true` when DB connection is healthy", function() From ca8765f6800f8ab1d6e0b7f63a5d7906245f0ed7 Mon Sep 17 00:00:00 2001 From: samugi Date: Mon, 12 Dec 2022 11:35:08 +0100 Subject: [PATCH 2/3] fix(declarative-config): empty hash for default config ensure the default (empty) config returns the appropriate hash that identifies the empty configuration: 00000000000000000000000000000000 --- kong/db/declarative/import.lua | 10 +++- .../02-cmd/02-start_stop_spec.lua | 48 ++++++++++++++++++- .../04-admin_api/02-kong_routes_spec.lua | 4 +- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/kong/db/declarative/import.lua b/kong/db/declarative/import.lua index 4f0359bbae5c..93d30e44ac6f 100644 --- a/kong/db/declarative/import.lua +++ b/kong/db/declarative/import.lua @@ -10,7 +10,7 @@ local cjson_encode = require("cjson.safe").encode local deepcopy = require("pl.tablex").deepcopy local marshall = require("kong.db.declarative.marshaller").marshall local schema_topological_sort = require("kong.db.schema.topological_sort") - +local nkeys = require("table.nkeys") local assert = assert local sort = table.sort @@ -148,6 +148,12 @@ local function unique_field_key(schema_name, ws_id, field, value, unique_across_ end +local function config_is_empty(entities) + -- empty configuration has no entries other than workspaces + return entities.workspaces and nkeys(entities) == 1 +end + + -- entities format: -- { -- services: { @@ -174,7 +180,7 @@ local function load_into_cache(entities, meta, hash) assert(type(fallback_workspace) == "string") - if not hash or hash == "" then + if not hash or hash == "" or config_is_empty(entities) then hash = DECLARATIVE_EMPTY_CONFIG_HASH end diff --git a/spec/02-integration/02-cmd/02-start_stop_spec.lua b/spec/02-integration/02-cmd/02-start_stop_spec.lua index b3faca29c943..39320b4a210a 100644 --- a/spec/02-integration/02-cmd/02-start_stop_spec.lua +++ b/spec/02-integration/02-cmd/02-start_stop_spec.lua @@ -474,7 +474,7 @@ describe("kong start/stop #" .. strategy, function() end, 10) end) - it("configuration hash is set correctly", function() + it("hash is set correctly for a non-empty configuration", function() local yaml_file = helpers.make_yaml_file [[ _format_version: "1.1" services: @@ -531,6 +531,52 @@ describe("kong start/stop #" .. strategy, function() assert.equals(32, #json_body.configuration_hash) assert.not_equal(constants.DECLARATIVE_EMPTY_CONFIG_HASH, json_body.configuration_hash) end) + + it("hash is set correctly for an empty configuration", function() + + local admin_client, json_body + + finally(function() + helpers.stop_kong(helpers.test_conf.prefix) + if admin_client then + admin_client:close() + end + end) + + -- not specifying declarative_config this time + assert(helpers.start_kong({ + database = "off", + nginx_conf = "spec/fixtures/custom_nginx.template", + })) + + helpers.wait_until(function() + helpers.wait_until(function() + local pok + pok, admin_client = pcall(helpers.admin_client) + return pok + end, 10) + + local res = assert(admin_client:send { + method = "GET", + path = "/status" + }) + if res.status ~= 200 then + return false + end + local body = assert.res_status(200, res) + json_body = cjson.decode(body) + + if admin_client then + admin_client:close() + admin_client = nil + end + + return true + end, 10) + + assert.is_string(json_body.configuration_hash) + assert.equals(constants.DECLARATIVE_EMPTY_CONFIG_HASH, json_body.configuration_hash) + end) end) end diff --git a/spec/02-integration/04-admin_api/02-kong_routes_spec.lua b/spec/02-integration/04-admin_api/02-kong_routes_spec.lua index 98d63efcdc0c..a2b3e68f620a 100644 --- a/spec/02-integration/04-admin_api/02-kong_routes_spec.lua +++ b/spec/02-integration/04-admin_api/02-kong_routes_spec.lua @@ -211,9 +211,7 @@ describe("Admin API - Kong routes with strategy #" .. strategy, function() assert.is_number(json.server.connections_waiting) assert.is_number(json.server.total_requests) if strategy == "off" then - assert.is_string(json.configuration_hash) - assert.equal(32, #json.configuration_hash) - assert.is_not_equal(empty_config_hash, json.configuration_hash) + assert.is_equal(empty_config_hash, json.configuration_hash) -- all 0 in DBLESS mode until configuration is applied else assert.is_nil(json.configuration_hash) -- not present in DB mode end From 56aa4d1f66f189c6358b9708a7d2d269ddcdd502 Mon Sep 17 00:00:00 2001 From: samugi Date: Mon, 12 Dec 2022 16:18:05 +0100 Subject: [PATCH 3/3] fix(declarative-config): CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 478e6510adfe..2768cdb62d29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,12 @@ - **Zipkin**: Fix an issue where the global plugin's sample ratio overrides route-specific. [#9877](https://github.com/Kong/kong/pull/9877) +#### Core + +- Fix an issue where after a valid declarative configuration is loaded, + the configuration hash is incorrectly set to the value: `00000000000000000000000000000000`. + [#9911](https://github.com/Kong/kong/pull/9911) + ### Dependencies - Bumped luarocks from 3.9.1 to 3.9.2