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

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

@@ -261,17 +261,24 @@ end
 local function crud_snis_handler(data)
   log(DEBUG, "[events] SNI updated, invalidating cached certificates")
 
-  local sni = data.old_entity or data.entity
-  local sni_name = 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
-    core_cache:invalidate("snis:" .. sni_wild_pref)
+  local new_name = data.entity.name
+  local old_name = data.old_entity and data.old_entity.name
+  if new_name == old_name then
+    old_name = nil
   end
 
-  if sni_wild_suf then
-    core_cache:invalidate("snis:" .. sni_wild_suf)
+  -- new_name won't be nil so we put it in the front
+  for _, sni_name in ipairs { new_name, old_name } do
+    local sni_wild_pref, sni_wild_suf = certificate.produce_wild_snis(sni_name)
+    core_cache:invalidate("snis:" .. sni_name)
+
+    if sni_wild_pref and sni_wild_pref ~= sni_name then
+      core_cache:invalidate("snis:" .. sni_wild_pref)
+    end
+
+    if sni_wild_suf and sni_wild_suf ~= sni_name then
+      core_cache:invalidate("snis:" .. sni_wild_suf)
+    end
   end
 end
 

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

@@ -471,6 +471,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.
@@ -514,6 +520,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
 
@@ -548,6 +558,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])
@@ -579,6 +595,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" },
@@ -606,6 +628,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")
@@ -630,6 +656,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,
@@ -680,6 +714,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
 
@@ -723,6 +763,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))
@@ -762,6 +810,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" },
@@ -797,6 +853,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")