Re: [PR] fix: consumer key duplication check [apisix]

["AlinsRan (via GitHub)" <gi...@apache.org>]

AlinsRan commented on code in PR #12040:
URL: https://github.com/apache/apisix/pull/12040#discussion_r2013377171


##########
apisix/admin/consumers.lua:
##########
@@ -34,6 +38,40 @@ local function check_conf(username, conf, need_username, schema)
         if not ok then
             return nil, {error_msg = "invalid plugins configuration: " .. err}
         end
+
+        -- check duplicate key
+        for plugin_name, plugin_conf in pairs(conf.plugins or {}) do
+            local plugin_obj = plugin.get(plugin_name)
+            if not plugin_obj then
+                return nil, {error_msg = "unknown plugin " .. plugin_name}
+            end
+
+            if plugin_obj.type == "auth" then
+                local plugin_conf_copy = core.table.deepcopy(plugin_conf)
+                plugin.decrypt_conf(plugin_name, plugin_conf_copy, core.schema.TYPE_CONSUMER)
+
+                local key_field = utils.plugin_key_map[plugin_name]
+
+                if key_field then
+                    local key_value = plugin_conf_copy[key_field]
+
+                    if key_value then
+                        local consumer, _, err = consumer
+                            .find_consumer(plugin_name, key_field, key_value)
+                        if err then
+                            core.log.warn("failed to find consumer: ", err)

Review Comment:
   ```suggestion
                           local consumer = consumer.find_consumer(plugin_name, key_field, key_value)
                           if consumer then
                              ...
   ```



##########
apisix/admin/credentials.lua:
##########
@@ -32,14 +34,41 @@ local function check_conf(_id, conf, _need_id, schema)
             return nil, {error_msg = "invalid plugins configuration: " .. err}
         end
 
-        for name, _ in pairs(conf.plugins) do
+        for name, plugin_conf in pairs(conf.plugins) do
             local plugin_obj = plugin.get(name)
             if not plugin_obj then
                 return nil, {error_msg = "unknown plugin " .. name}
             end
+
             if plugin_obj.type ~= "auth" then
                 return nil, {error_msg = "only supports auth type plugins in consumer credential"}
             end
+
+            -- check duplicate key
+            local decrypted_conf = core.table.deepcopy(plugin_conf)

Review Comment:
   These two pieces of code are basically the same. I suggest using a universal function.



##########
apisix/admin/consumers.lua:
##########
@@ -34,6 +38,40 @@ local function check_conf(username, conf, need_username, schema)
         if not ok then
             return nil, {error_msg = "invalid plugins configuration: " .. err}
         end
+
+        -- check duplicate key
+        for plugin_name, plugin_conf in pairs(conf.plugins or {}) do
+            local plugin_obj = plugin.get(plugin_name)
+            if not plugin_obj then
+                return nil, {error_msg = "unknown plugin " .. plugin_name}
+            end
+
+            if plugin_obj.type == "auth" then
+                local plugin_conf_copy = core.table.deepcopy(plugin_conf)
+                plugin.decrypt_conf(plugin_name, plugin_conf_copy, core.schema.TYPE_CONSUMER)
+
+                local key_field = utils.plugin_key_map[plugin_name]
+
+                if key_field then
+                    local key_value = plugin_conf_copy[key_field]
+
+                    if key_value then
+                        local consumer, _, err = consumer
+                            .find_consumer(plugin_name, key_field, key_value)
+                        if err then
+                            core.log.warn("failed to find consumer: ", err)

Review Comment:
   The expected outcome is that it cannot be found, so I believe there is no need to record any logs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org