fix(runloop): the sni cache isn't invalidated when a sni is updated (#…

…13165)

Both `data.entity` and `data.old_entity` should be invalidated when
a sni is updated. A non-existent sni may also have been cached.

https://konghq.atlassian.net/browse/FTI-6009

changelog/unreleased/kong/fix-sni-cache-invalidate.yml

@@ -0,0 +1,4 @@
+message: |
+  Fixed an issue where the sni cache isn't invalidated when a sni is updated.
+type: bugfix
+scope: Core

kong/runloop/events.lua

@@ -277,26 +277,35 @@ local function crud_consumer_groups_handler(data)
 end
 
 
-local function crud_snis_handler(data)
-  workspaces.set_workspace(data.workspace)  -- XX EE
-
-  log(DEBUG, "[events] SNI updated, invalidating cached certificates")
-
-  local sni = data.old_entity or data.entity
-  local sni_name = sni.name
+local function invalidate_snis(sni_name)
   local sni_wild_pref, sni_wild_suf = certificate.produce_wild_snis(sni_name)
   core_cache:invalidate("snis:" .. sni_name)
 
-  if sni_wild_pref then
+  if sni_wild_pref and sni_wild_pref ~= sni_name then
     core_cache:invalidate("snis:" .. sni_wild_pref)
   end
 
-  if sni_wild_suf then
+  if sni_wild_suf and sni_wild_suf ~= sni_name then
     core_cache:invalidate("snis:" .. sni_wild_suf)
   end
 end
 
 
+local function crud_snis_handler(data)
+  workspaces.set_workspace(data.workspace)  -- XX EE
+
+  log(DEBUG, "[events] SNI updated, invalidating cached certificates")
+
+  local new_name = data.entity.name
+  local old_name = data.old_entity and data.old_entity.name
+
+  invalidate_snis(new_name)
+  if old_name and old_name ~= new_name then
+    invalidate_snis(old_name)
+  end
+end
+
+
 local function crud_consumers_handler(data)
   workspaces.set_workspace(data.workspace)
 

spec/02-integration/06-invalidations/02-core_entities_invalidations_spec.lua

@@ -482,6 +482,12 @@ for _, strategy in helpers.each_strategy() do
       end)
 
       it("on certificate delete+re-creation", function()
+        -- populate cache
+        get_cert(8443, "ssl-example.com")
+        get_cert(8443, "new-ssl-example.com")
+        get_cert(9443, "ssl-example.com")
+        get_cert(9443, "new-ssl-example.com")
+
         -- TODO: PATCH update are currently not possible
         -- with the admin API because snis have their name as their
         -- primary key and the DAO has limited support for such updates.
@@ -525,6 +531,10 @@ for _, strategy in helpers.each_strategy() do
       end)
 
       it("on certificate update", function()
+        -- populate cache
+        get_cert(8443, "new-ssl-example.com")
+        get_cert(9443, "new-ssl-example.com")
+
         -- update our certificate *without* updating the
         -- attached sni
 
@@ -559,6 +569,12 @@ for _, strategy in helpers.each_strategy() do
       end)
 
       it("on sni update via id", function()
+        -- populate cache
+        get_cert(8443, "new-ssl-example.com")
+        get_cert(8443, "updated-sn-via-id.com")
+        get_cert(9443, "new-ssl-example.com")
+        get_cert(9443, "updated-sn-via-id.com")
+
         local admin_res = admin_client_1:get("/snis")
         local body = assert.res_status(200, admin_res)
         local sni = assert(cjson.decode(body).data[1])
@@ -590,6 +606,12 @@ for _, strategy in helpers.each_strategy() do
       end)
 
       it("on sni update via name", function()
+        -- populate cache
+        get_cert(8443, "updated-sn-via-id.com")
+        get_cert(8443, "updated-sn.com")
+        get_cert(9443, "updated-sn-via-id.com")
+        get_cert(9443, "updated-sn.com")
+
         local admin_res = admin_client_1:patch("/snis/updated-sn-via-id.com", {
           body    = { name = "updated-sn.com" },
           headers = { ["Content-Type"] = "application/json" },
@@ -617,6 +639,10 @@ for _, strategy in helpers.each_strategy() do
       end)
 
       it("on certificate delete", function()
+        -- populate cache
+        get_cert(8443, "updated-sn.com")
+        get_cert(9443, "updated-sn.com")
+
         -- delete our certificate
 
         local admin_res = admin_client_1:delete("/certificates/updated-sn.com")
@@ -641,6 +667,14 @@ for _, strategy in helpers.each_strategy() do
 
       describe("wildcard snis", function()
         it("on create", function()
+          -- populate cache
+          get_cert(8443, "test.wildcard.com")
+          get_cert(8443, "test2.wildcard.com")
+          get_cert(8443, "wildcard.com")
+          get_cert(9443, "test.wildcard.com")
+          get_cert(9443, "test2.wildcard.com")
+          get_cert(9443, "wildcard.com")
+
           local admin_res = admin_client_1:post("/certificates", {
             body   = {
               cert = ssl_fixtures.cert_alt,
@@ -691,6 +725,12 @@ for _, strategy in helpers.each_strategy() do
         end)
 
         it("on certificate update", function()
+          -- populate cache
+          get_cert(8443, "test.wildcard.com")
+          get_cert(8443, "test2.wildcard.com")
+          get_cert(9443, "test.wildcard.com")
+          get_cert(9443, "test2.wildcard.com")
+
           -- update our certificate *without* updating the
           -- attached sni
 
@@ -734,6 +774,14 @@ for _, strategy in helpers.each_strategy() do
         end)
 
         it("on sni update via id", function()
+          -- populate cache
+          get_cert(8443, "test.wildcard.com")
+          get_cert(8443, "test2.wildcard.com")
+          get_cert(8443, "test.wildcard_updated.com")
+          get_cert(9443, "test.wildcard.com")
+          get_cert(9443, "test2.wildcard.com")
+          get_cert(9443, "test.wildcard_updated.com")
+
           local admin_res = admin_client_1:get("/snis/%2A.wildcard.com")
           local body = assert.res_status(200, admin_res)
           local sni = assert(cjson.decode(body))
@@ -773,6 +821,14 @@ for _, strategy in helpers.each_strategy() do
         end)
 
         it("on sni update via name", function()
+          -- populate cache
+          get_cert(8443, "test.wildcard.org")
+          get_cert(8443, "test2.wildcard.org")
+          get_cert(8443, "test.wildcard_updated.com")
+          get_cert(9443, "test.wildcard.org")
+          get_cert(9443, "test2.wildcard.org")
+          get_cert(9443, "test.wildcard_updated.com")
+
           local admin_res = admin_client_1:patch("/snis/%2A.wildcard_updated.com", {
             body    = { name = "*.wildcard.org" },
             headers = { ["Content-Type"] = "application/json" },
@@ -808,6 +864,12 @@ for _, strategy in helpers.each_strategy() do
         end)
 
         it("on certificate delete", function()
+          -- populate cache
+          get_cert(8443, "test.wildcard.org")
+          get_cert(8443, "test2.wildcard.org")
+          get_cert(9443, "test.wildcard.org")
+          get_cert(9443, "test2.wildcard.org")
+
           -- delete our certificate
 
           local admin_res = admin_client_1:delete("/certificates/%2A.wildcard.org")