You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/26 09:39:49 UTC

[GitHub] [apisix] spacewander opened a new pull request #2856: fix: validate plugin configuration in the DP

spacewander opened a new pull request #2856:
URL: https://github.com/apache/apisix/pull/2856


   Fix #2834
   
   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   
   ### Pre-submission checklist:
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
   


----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on pull request #2856: fix: validate plugin configuration in the DP

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2856:
URL: https://github.com/apache/apisix/pull/2856#issuecomment-734195990


   @spacewander  Please update the checklist


----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander commented on a change in pull request #2856: fix: validate plugin configuration in the DP

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2856:
URL: https://github.com/apache/apisix/pull/2856#discussion_r531563032



##########
File path: apisix/core/config_etcd.lua
##########
@@ -256,6 +264,14 @@ local function sync_data(self)
                     end
                 end
 
+                if data_valid and self.checker then
+                    data_valid, err = self.checker(item.value)
+                    if not data_valid then
+                        log.error("failed to check item data of [", self.key,
+                                "] err:", err, " ,val: ", json.delay_encode(item.value))

Review comment:
       Updated.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on pull request #2856: fix: validate plugin configuration in the DP

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2856:
URL: https://github.com/apache/apisix/pull/2856#issuecomment-734195603


   wow,  That is a nice PR


----------------------------------------------------------------
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.

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



[GitHub] [apisix] tokers commented on a change in pull request #2856: fix: validate plugin configuration in the DP

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2856:
URL: https://github.com/apache/apisix/pull/2856#discussion_r531541188



##########
File path: apisix/plugin.lua
##########
@@ -498,4 +498,102 @@ function _M.get(name)
 end
 
 
+local function check_schema(plugins_conf, schema_type, skip_disabled_plugin)
+    for name, plugin_conf in pairs(plugins_conf) do
+        core.log.info("check plugin scheme, name: ", name, ", configurations: ",
+                      core.json.delay_encode(plugin_conf, true))
+        if type(plugin_conf) ~= "table" then
+            return false, "invalid plugin conf " ..
+                core.json.encode(plugin_conf, true) ..
+                " for plugin [" .. name .. "]"
+        end
+
+        local plugin_obj = local_plugins_hash[name]
+        if not plugin_obj then
+            if skip_disabled_plugin then
+                goto CONTINUE
+            else
+                return false, "unknown plugin [" .. name .. "]"
+            end
+        end
+
+        if plugin_obj.check_schema then
+            local disable = plugin_conf.disable
+            plugin_conf.disable = nil
+
+            local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
+            if not ok then
+                return false, "failed to check the configuration of plugin "
+                              .. name .. " err: " .. err
+            end
+
+            plugin_conf.disable = disable
+        end
+
+        ::CONTINUE::
+    end
+
+    return true
+end
+_M.check_schema = check_schema
+
+
+local function stream_check_schema(plugins_conf, schema_type, skip_disabled_plugin)
+    for name, plugin_conf in pairs(plugins_conf) do
+        core.log.info("check stream plugin scheme, name: ", name,

Review comment:
       Ditto.

##########
File path: apisix/core/config_etcd.lua
##########
@@ -256,6 +264,14 @@ local function sync_data(self)
                     end
                 end
 
+                if data_valid and self.checker then
+                    data_valid, err = self.checker(item.value)
+                    if not data_valid then
+                        log.error("failed to check item data of [", self.key,
+                                "] err:", err, " ,val: ", json.delay_encode(item.value))

Review comment:
       Bad indentation.

##########
File path: apisix/plugin.lua
##########
@@ -498,4 +498,102 @@ function _M.get(name)
 end
 
 
+local function check_schema(plugins_conf, schema_type, skip_disabled_plugin)
+    for name, plugin_conf in pairs(plugins_conf) do
+        core.log.info("check plugin scheme, name: ", name, ", configurations: ",

Review comment:
       Typo: `scheme` => `schema`.




----------------------------------------------------------------
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.

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



[GitHub] [apisix] spacewander merged pull request #2856: fix: validate plugin configuration in the DP

Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #2856:
URL: https://github.com/apache/apisix/pull/2856


   


----------------------------------------------------------------
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.

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



[GitHub] [apisix] membphis commented on pull request #2856: fix: validate plugin configuration in the DP

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2856:
URL: https://github.com/apache/apisix/pull/2856#issuecomment-734514790


   @spacewander you can continue this issue


----------------------------------------------------------------
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.

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