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 2022/09/23 09:18:55 UTC

[GitHub] [apisix] kingluo opened a new pull request, #7980: feat: add consumer group

kingluo opened a new pull request, #7980:
URL: https://github.com/apache/apisix/pull/7980

   ### Description
   
   add consumer group
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [x] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


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


[GitHub] [apisix] SylviaBABY commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
SylviaBABY commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r990592475


##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.
+
+The example below illustrates how to create a Consumer Group and bind it to a Consumer:
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "plugins": {
+        "limit-count": {
+            "count": 200,
+            "time_window": 60,
+            "rejected_code": 503,
+            "group": "$consumer_group_id"
+        }
+    }
+}'
+```
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "username": "jack",
+    "plugins": {
+        "key-auth": {
+            "key": "auth-one"
+        }
+    },
+    "group_id": "company_a"
+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.

Review Comment:
   ```suggestion
   When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of `400`.
   ```
   



##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.
+
+The example below illustrates how to create a Consumer Group and bind it to a Consumer:
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "plugins": {
+        "limit-count": {
+            "count": 200,
+            "time_window": 60,
+            "rejected_code": 503,
+            "group": "$consumer_group_id"
+        }
+    }
+}'
+```
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "username": "jack",
+    "plugins": {
+        "key-auth": {
+            "key": "auth-one"
+        }
+    },
+    "group_id": "company_a"
+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.
+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.
+
+For example, if we configure a Consumer Group as shown below
+
+```
+{
+    "id": "bar",
+    "plugins": {
+        "response-rewrite": {
+            "body": "hello"
+        }
+    }
+}
+```
+
+to a Consumer as shown below,

Review Comment:
   ```suggestion
   To a Consumer as shown below.
   ```
   



##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.
+
+The example below illustrates how to create a Consumer Group and bind it to a Consumer:
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "plugins": {
+        "limit-count": {
+            "count": 200,
+            "time_window": 60,
+            "rejected_code": 503,
+            "group": "$consumer_group_id"
+        }
+    }
+}'
+```
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "username": "jack",
+    "plugins": {
+        "key-auth": {
+            "key": "auth-one"
+        }
+    },
+    "group_id": "company_a"
+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.
+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.
+
+For example, if we configure a Consumer Group as shown below

Review Comment:
   Add `.` or `:` in the end of sentence



##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.
+
+The example below illustrates how to create a Consumer Group and bind it to a Consumer:
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "plugins": {
+        "limit-count": {
+            "count": 200,
+            "time_window": 60,
+            "rejected_code": 503,
+            "group": "$consumer_group_id"
+        }
+    }
+}'
+```
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "username": "jack",
+    "plugins": {
+        "key-auth": {
+            "key": "auth-one"
+        }
+    },
+    "group_id": "company_a"
+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.
+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.
+
+For example, if we configure a Consumer Group as shown below
+
+```
+{
+    "id": "bar",
+    "plugins": {
+        "response-rewrite": {
+            "body": "hello"
+        }
+    }
+}
+```
+
+to a Consumer as shown below,
+
+```

Review Comment:
   need to add code language too



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


[GitHub] [apisix] spacewander commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r990590742


##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.
+
+The example below illustrates how to create a Consumer Group and bind it to a Consumer:
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "plugins": {
+        "limit-count": {
+            "count": 200,
+            "time_window": 60,
+            "rejected_code": 503,
+            "group": "$consumer_group_id"
+        }
+    }
+}'
+```
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "username": "jack",
+    "plugins": {
+        "key-auth": {
+            "key": "auth-one"
+        }
+    },
+    "group_id": "company_a"
+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.
+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.
+
+For example, if we configure a Consumer Group as shown below
+
+```

Review Comment:
   Let's add language mark like other examples



##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.
+
+The example below illustrates how to create a Consumer Group and bind it to a Consumer:
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "plugins": {
+        "limit-count": {
+            "count": 200,
+            "time_window": 60,
+            "rejected_code": 503,
+            "group": "$consumer_group_id"
+        }
+    }
+}'
+```
+
+```shell
+curl http://127.0.0.1:9180/apisix/admin/consumers -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "username": "jack",
+    "plugins": {
+        "key-auth": {
+            "key": "auth-one"
+        }
+    },
+    "group_id": "company_a"
+}'
+```
+
+When APISIX can't find the Consumer Group with the `group_id`, the Admin API is terminated with a status code of 400.
+
+If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged to it. The same plugin in the Consumer Group will not override the ones configured directly in the Consumer.

Review Comment:
   ```suggestion
   If a Consumer already has the `plugins` field configured, the plugins in the Consumer Group will effectively be merged into it. The same plugin in the Consumer Group will not override the one configured directly in the Consumer.
   ```



##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).

Review Comment:
   We should update the admin-api.md?



##########
apisix/core/ctx.lua:
##########
@@ -206,6 +206,7 @@ do
     local apisix_var_names = {
         balancer_ip = true,
         balancer_port = true,
+        consumer_group_id = true,

Review Comment:
   Need to update https://github.com/apache/apisix/blob/master/docs/en/latest/apisix-variable.md for the new variable.



##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).
+
+With consumer groups, you can define any number of plugins, e.g. rate limiting and apply them to a set of consumers,
+instead of managing each consumer individually.
+
+While configuring the same plugin for the same route, only one copy of the configuration is valid.
+The order of precedence is `Consumer` > `Consumer Group` > `Route` > `plugin_config` > `Service`.

Review Comment:
   Better to put the order to one place: https://github.com/apache/apisix/blob/61afafdd46fe40c03955b6cb058df3e6fad5db45/docs/en/latest/terminology/plugin.md?plain=1#L30



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


[GitHub] [apisix] kingluo commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979602766


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: GET
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: GET all
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "total": 1,
+                    "list": [
+                        {
+                            "key": "/apisix/consumer_groups/company_a",
+                            "value": {
+                                "plugins": {
+                                    "limit-count": {
+                                    "time_window": 60,
+                                    "policy": "local",
+                                    "count": 2,
+                                    "key": "remote_addr",
+                                    "rejected_code": 503
+                                    }
+                                }
+                            }
+                        }
+                    ]
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 4: PATCH
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PATCH,
+                [[{
+                    "plugins": {
+                    "limit-count": {
+                        "count": 3,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }}]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 3,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 5: PATCH (sub path)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a/plugins',
+                ngx.HTTP_PATCH,
+                [[{
+                    "limit-count": {
+                        "count": 2,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"

Review Comment:
   Then which way or which test file do you think I should take reference?



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


[GitHub] [apisix] kingluo commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979511016


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]

Review Comment:
   It is based on the `t/admin/plugin-configs.t`, where it associates plugins with plugin config for admin api test.
   So what's the reason to remove them?



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979512933


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: GET
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: GET all
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "total": 1,
+                    "list": [
+                        {
+                            "key": "/apisix/consumer_groups/company_a",
+                            "value": {
+                                "plugins": {
+                                    "limit-count": {
+                                    "time_window": 60,
+                                    "policy": "local",
+                                    "count": 2,
+                                    "key": "remote_addr",
+                                    "rejected_code": 503
+                                    }
+                                }
+                            }
+                        }
+                    ]
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 4: PATCH
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PATCH,
+                [[{
+                    "plugins": {
+                    "limit-count": {
+                        "count": 3,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }}]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 3,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 5: PATCH (sub path)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a/plugins',
+                ngx.HTTP_PATCH,
+                [[{
+                    "limit-count": {
+                        "count": 2,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"

Review Comment:
   We rarely validate return values in this way these days. Perhaps the `plugin-config.t` you referenced would be acceptable if left as is.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979408909


##########
apisix/core/ctx.lua:
##########
@@ -207,6 +207,7 @@ do
         balancer_ip = true,
         balancer_port = true,
         consumer_name = true,
+        consumer_group_id = true,

Review Comment:
   We would use this variable in plugins:
   
   For example:
   ```
   curl http://127.0.0.1:9080/apisix/admin/consumer_groups/company_a -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
   {
       "plugins": {
           "quota": {
               "rejected_code": 503,
               "minute": 2,
               "group": "$consumer_group_id"
           }
       }
   }'
   ```



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979508149


##########
apisix/plugin.lua:
##########
@@ -651,20 +665,33 @@ local function merge_consumer_route(route_conf, consumer_conf)
 end
 
 
-function _M.merge_consumer_route(route_conf, consumer_conf, api_ctx)
+function _M.merge_consumer_route(route_conf, consumer_conf, consumer_group_conf, api_ctx)
     core.log.info("route conf: ", core.json.delay_encode(route_conf))
     core.log.info("consumer conf: ", core.json.delay_encode(consumer_conf))
+    core.log.info("consumer group conf: ", core.json.delay_encode(consumer_group_conf))
 
     local flag = route_conf.value.id .. "#" .. route_conf.modifiedIndex
                  .. "#" .. consumer_conf.id .. "#" .. consumer_conf.modifiedIndex
+
+    if consumer_group_conf then
+        flag = flag .. "#" .. consumer_group_conf.value.id
+            .. "#" .. consumer_group_conf.modifiedIndex

Review Comment:
   flag should have `route_conf.value.id` and `route_conf.modifiedIndex`?



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


[GitHub] [apisix] spacewander commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r990737731


##########
docs/en/latest/apisix-variable.md:
##########
@@ -38,6 +38,7 @@ additional variables.
 | balancer_ip         | core       | The IP of picked upstream server.                                                   | 192.168.1.2    |
 | balancer_port       | core       | The port of picked upstream server.                                                 | 80             |
 | consumer_name       | core       | Username of Consumer.                                                               |                |
+| consumer_group_id   | core       | Group name of Consumer.                                                             |                |

Review Comment:
   ```suggestion
   | consumer_group_id   | core       | Group ID of Consumer.                                                             |                |
   ```



##########
docs/en/latest/admin-api.md:
##########
@@ -947,6 +948,35 @@ Sets Plugins which run globally. i.e these Plugins will be run before any Route/
 | create_time | False    | Epoch timestamp (in seconds) of the created time. If missing, this field will be populated automatically.             | 1602883670 |
 | update_time | False    | Epoch timestamp (in seconds) of the updated time. If missing, this field will be populated automatically.             | 1602883670 |
 
+## Consumer group

Review Comment:
   Let's update the `Resources that support paging queries:` list in this page



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


[GitHub] [apisix] spacewander commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r990933301


##########
docs/zh/latest/apisix-variable.md:
##########
@@ -37,6 +37,7 @@ APISIX 除了支持 [NGINX 变量](http://nginx.org/en/docs/varindex.html)外,
 | balancer_ip         | core       | 上游服务器的 IP 地址。                                                            | 192.168.1.2      |
 | balancer_port       | core       | 上游服务器的端口。                                                                | 80               |
 | consumer_name       | core       | 消费者的名称。                                                                    |                  |
+| consumer_group_id   | core       | 消费者所在的组的名称。                                                            |                  |

Review Comment:
   Let's sync the change to the Chinese version too



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


[GitHub] [apisix] spacewander commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979402341


##########
apisix/admin/consumer_group.lua:
##########
@@ -0,0 +1,195 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local consumers = require("apisix.consumer").consumers
+local utils = require("apisix.admin.utils")
+local schema_plugin = require("apisix.admin.plugins").check_schema
+local type = type
+local tostring = tostring
+local ipairs = ipairs
+
+
+local _M = {
+    need_v3_filter = true,
+}
+
+
+local function check_conf(id, conf, need_id)
+    if not conf then
+        return nil, {error_msg = "missing configurations"}
+    end
+
+    id = id or conf.id
+    if need_id and not id then
+        return nil, {error_msg = "missing id"}
+    end
+
+    if not need_id and id then
+        return nil, {error_msg = "wrong id, do not need it"}
+    end
+
+    if need_id and conf.id and tostring(conf.id) ~= tostring(id) then
+        return nil, {error_msg = "wrong id"}
+    end
+
+    conf.id = id
+
+    core.log.info("conf: ", core.json.delay_encode(conf))
+    local ok, err = core.schema.check(core.schema.plugin_config, conf)

Review Comment:
   Don't hijack the schema of plugin_config in other places.



##########
apisix/core/ctx.lua:
##########
@@ -207,6 +207,7 @@ do
         balancer_ip = true,
         balancer_port = true,
         consumer_name = true,
+        consumer_group_id = true,

Review Comment:
   Why should we expose it as a variable? It seems we also use `ctx.consumer.group_id` in some places.



##########
apisix/consumer_group.lua:
##########
@@ -0,0 +1,47 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local plugin_checker = require("apisix.plugin").plugin_checker
+local error = error
+
+
+local consumer_groups
+
+
+local _M = {
+}
+
+
+function _M.init_worker()
+    local err
+    consumer_groups, err = core.config.new("/consumer_groups", {
+        automatic = true,
+        item_schema = nil,

Review Comment:
   Why don't we use schema to check the item?



##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {

Review Comment:
   It is more common to use this style:
   https://github.com/apache/apisix/blob/3bbc7aefe36fa0bbc0748335d64a3ead02c53f87/t/admin/filter.t#L44



##########
apisix/admin/consumer_group.lua:
##########
@@ -0,0 +1,195 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local consumers = require("apisix.consumer").consumers
+local utils = require("apisix.admin.utils")
+local schema_plugin = require("apisix.admin.plugins").check_schema
+local type = type
+local tostring = tostring
+local ipairs = ipairs
+
+
+local _M = {
+    need_v3_filter = true,
+}
+
+
+local function check_conf(id, conf, need_id)
+    if not conf then
+        return nil, {error_msg = "missing configurations"}
+    end
+
+    id = id or conf.id
+    if need_id and not id then
+        return nil, {error_msg = "missing id"}
+    end
+
+    if not need_id and id then
+        return nil, {error_msg = "wrong id, do not need it"}
+    end
+
+    if need_id and conf.id and tostring(conf.id) ~= tostring(id) then
+        return nil, {error_msg = "wrong id"}
+    end
+
+    conf.id = id
+
+    core.log.info("conf: ", core.json.delay_encode(conf))
+    local ok, err = core.schema.check(core.schema.plugin_config, conf)
+    if not ok then
+        return nil, {error_msg = "invalid configuration: " .. err}
+    end
+
+    local ok, err = schema_plugin(conf.plugins)
+    if not ok then
+        return nil, {error_msg = err}
+    end
+
+    return true
+end
+
+
+function _M.put(id, conf)

Review Comment:
   We should also add a check in consumers. Let's make sure `consumer.group_id` points to an actual consumer group.



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r980657627


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: GET
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: GET all
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "total": 1,
+                    "list": [
+                        {
+                            "key": "/apisix/consumer_groups/company_a",
+                            "value": {
+                                "plugins": {
+                                    "limit-count": {
+                                    "time_window": 60,
+                                    "policy": "local",
+                                    "count": 2,
+                                    "key": "remote_addr",
+                                    "rejected_code": 503
+                                    }
+                                }
+                            }
+                        }
+                    ]
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 4: PATCH
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PATCH,
+                [[{
+                    "plugins": {
+                    "limit-count": {
+                        "count": 3,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }}]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 3,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 5: PATCH (sub path)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a/plugins',
+                ngx.HTTP_PATCH,
+                [[{
+                    "limit-count": {
+                        "count": 2,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"

Review Comment:
   I get it, let's keep it.



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979510321


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: GET
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: GET all
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "total": 1,
+                    "list": [
+                        {
+                            "key": "/apisix/consumer_groups/company_a",
+                            "value": {
+                                "plugins": {
+                                    "limit-count": {
+                                    "time_window": 60,
+                                    "policy": "local",
+                                    "count": 2,
+                                    "key": "remote_addr",
+                                    "rejected_code": 503
+                                    }
+                                }
+                            }
+                        }
+                    ]
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 4: PATCH
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PATCH,
+                [[{
+                    "plugins": {
+                    "limit-count": {
+                        "count": 3,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }}]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 3,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 5: PATCH (sub path)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a/plugins',
+                ngx.HTTP_PATCH,
+                [[{
+                    "limit-count": {
+                        "count": 2,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 6: invalid plugin
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "rejected_code": 503,
+                            "time_window": 60,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]]
+                )
+
+            ngx.status = code
+            ngx.print(body)
+        }
+    }
+--- response_body
+{"error_msg":"failed to check the configuration of plugin limit-count err: property \"count\" is required"}
+--- error_code: 400
+
+
+
+=== TEST 7: PUT (with non-plugin fields)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    },
+                    "labels": {
+                        "你好": "世界"
+                    },
+                    "desc": "blah"
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        },
+                        "labels": {
+                            "你好": "世界"
+                        },
+                        "desc": "blah"
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 8: GET (with non-plugin fields)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        },
+                        "labels": {
+                            "你好": "世界"
+                        },
+                        "desc": "blah"
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 9: invalid non-plugin fields
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "labels": "a",
+                    "plugins": {
+                    }
+                }]]
+                )
+
+            ngx.status = code
+            ngx.print(body)
+        }
+    }
+--- response_body
+{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
+--- error_code: 400
+
+
+
+=== TEST 10: set consumer-group
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 11: add consumer with group
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumers/foobar',
+                ngx.HTTP_PUT,
+                [[{
+                    "username": "foobar",
+                    "plugins": {
+                        "key-auth": {
+                            "key": "auth-two"
+                        }
+                    },
+                    "group_id": "company_a"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 12: delete-consumer group failed
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.sleep(0.3)
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                 ngx.HTTP_DELETE
+            )
+            ngx.print(body)
+        }
+    }
+--- response_body
+{"error_msg":"can not delete this consumer group, consumer [foobar] is still using it now"}
+
+
+
+=== TEST 13: delete consumer
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.sleep(0.3)
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumers/foobar',
+                 ngx.HTTP_DELETE
+            )
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 14: delete consumer-group
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.sleep(0.3)
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                 ngx.HTTP_DELETE
+            )
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed

Review Comment:
   1. we need to verify that the plugin works well in the consumer_group
   2. we need to verify that consumer_group and consumer work well together



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


[GitHub] [apisix] kingluo commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979510285


##########
apisix/plugin.lua:
##########
@@ -651,20 +665,33 @@ local function merge_consumer_route(route_conf, consumer_conf)
 end
 
 
-function _M.merge_consumer_route(route_conf, consumer_conf, api_ctx)
+function _M.merge_consumer_route(route_conf, consumer_conf, consumer_group_conf, api_ctx)
     core.log.info("route conf: ", core.json.delay_encode(route_conf))
     core.log.info("consumer conf: ", core.json.delay_encode(consumer_conf))
+    core.log.info("consumer group conf: ", core.json.delay_encode(consumer_group_conf))
 
     local flag = route_conf.value.id .. "#" .. route_conf.modifiedIndex
                  .. "#" .. consumer_conf.id .. "#" .. consumer_conf.modifiedIndex
+
+    if consumer_group_conf then
+        flag = flag .. "#" .. consumer_group_conf.value.id
+            .. "#" .. consumer_group_conf.modifiedIndex

Review Comment:
   I already does. It concats new identification to the exist flag, which already contains route stuff.
   
   ```lua
       -- check here...
       local flag = route_conf.value.id .. "#" .. route_conf.modifiedIndex
                    .. "#" .. consumer_conf.id .. "#" .. consumer_conf.modifiedIndex
   
       if consumer_group_conf then
           flag = flag .. "#" .. consumer_group_conf.value.id
               .. "#" .. consumer_group_conf.modifiedIndex
       end
   
   ```



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979509079


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: GET
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: GET all
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "total": 1,
+                    "list": [
+                        {
+                            "key": "/apisix/consumer_groups/company_a",
+                            "value": {
+                                "plugins": {
+                                    "limit-count": {
+                                    "time_window": 60,
+                                    "policy": "local",
+                                    "count": 2,
+                                    "key": "remote_addr",
+                                    "rejected_code": 503
+                                    }
+                                }
+                            }
+                        }
+                    ]
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 4: PATCH
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PATCH,
+                [[{
+                    "plugins": {
+                    "limit-count": {
+                        "count": 3,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }}]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 3,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 5: PATCH (sub path)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a/plugins',
+                ngx.HTTP_PATCH,
+                [[{
+                    "limit-count": {
+                        "count": 2,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"

Review Comment:
   ditto



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


[GitHub] [apisix] spacewander commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r980781612


##########
apisix/admin/consumers.lua:
##########
@@ -62,6 +63,12 @@ local function check_conf(username, conf)
         end
     end
 
+    if conf.group_id then
+        if consumer_group.get(conf.group_id) == nil then

Review Comment:
   For code style, I will recommend using the same solution in https://github.com/apache/apisix/blob/3c18031c6e70daa15db72fe06ae8644b04b4481e/apisix/admin/routes.lua#L79
   1. fetch the data from etcd if possible
   2. use a similar error message



##########
apisix/core/ctx.lua:
##########
@@ -207,6 +207,7 @@ do
         balancer_ip = true,
         balancer_port = true,
         consumer_name = true,
+        consumer_group_id = true,

Review Comment:
   For alphabetical order, the `consumer_group_id` is ahead of consumer_name



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


[GitHub] [apisix] spacewander commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r990590776


##########
docs/en/latest/terminology/consumer-group.md:
##########
@@ -0,0 +1,101 @@
+---
+title: Consumer Group
+keywords:
+  - API gateway
+  - Apache APISIX
+  - Consumer Group
+description: Consumer Group in Apache APISIX.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+Consumer Groups are used to extract commonly used [Plugin](./plugin.md) configurations and can be bound directly to a [Consumer](./consumer.md).

Review Comment:
   We should also update the admin-api.md?



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


[GitHub] [apisix] spacewander merged pull request #7980: feat: add consumer group

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


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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979508812


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]

Review Comment:
   don't need this.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #7980: feat: add consumer group

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #7980:
URL: https://github.com/apache/apisix/pull/7980#discussion_r979511891


##########
t/admin/consumer-group.t:
##########
@@ -0,0 +1,523 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!$block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+    if (!$block->no_error_log) {
+        $block->set_value("no_error_log", "[error]\n[alert]");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: PUT
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 2: GET
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 3: GET all
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "total": 1,
+                    "list": [
+                        {
+                            "key": "/apisix/consumer_groups/company_a",
+                            "value": {
+                                "plugins": {
+                                    "limit-count": {
+                                    "time_window": 60,
+                                    "policy": "local",
+                                    "count": 2,
+                                    "key": "remote_addr",
+                                    "rejected_code": 503
+                                    }
+                                }
+                            }
+                        }
+                    ]
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 4: PATCH
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PATCH,
+                [[{
+                    "plugins": {
+                    "limit-count": {
+                        "count": 3,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }}]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 3,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 5: PATCH (sub path)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local prev_create_time = res.body.node.value.create_time
+            assert(prev_create_time ~= nil, "create_time is nil")
+            local prev_update_time = res.body.node.value.update_time
+            assert(prev_update_time ~= nil, "update_time is nil")
+            ngx.sleep(1)
+
+            local code, body = t('/apisix/admin/consumer_groups/company_a/plugins',
+                ngx.HTTP_PATCH,
+                [[{
+                    "limit-count": {
+                        "count": 2,
+                        "time_window": 60,
+                        "rejected_code": 503,
+                        "key": "remote_addr"
+                    }
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        }
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(prev_create_time == create_time, "create_time mismatched")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+            assert(prev_update_time ~= update_time, "update_time should be changed")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 6: invalid plugin
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "rejected_code": 503,
+                            "time_window": 60,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]]
+                )
+
+            ngx.status = code
+            ngx.print(body)
+        }
+    }
+--- response_body
+{"error_msg":"failed to check the configuration of plugin limit-count err: property \"count\" is required"}
+--- error_code: 400
+
+
+
+=== TEST 7: PUT (with non-plugin fields)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    },
+                    "labels": {
+                        "你好": "世界"
+                    },
+                    "desc": "blah"
+                }]],
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        },
+                        "labels": {
+                            "你好": "世界"
+                        },
+                        "desc": "blah"
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 8: GET (with non-plugin fields)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_GET,
+                nil,
+                [[{
+                    "value": {
+                        "plugins": {
+                            "limit-count": {
+                                "count": 2,
+                                "time_window": 60,
+                                "rejected_code": 503,
+                                "key": "remote_addr"
+                            }
+                        },
+                        "labels": {
+                            "你好": "世界"
+                        },
+                        "desc": "blah"
+                    },
+                    "key": "/apisix/consumer_groups/company_a"
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 9: invalid non-plugin fields
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "labels": "a",
+                    "plugins": {
+                    }
+                }]]
+                )
+
+            ngx.status = code
+            ngx.print(body)
+        }
+    }
+--- response_body
+{"error_msg":"invalid configuration: property \"labels\" validation failed: wrong type: expected object, got string"}
+--- error_code: 400
+
+
+
+=== TEST 10: set consumer-group
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local etcd = require("apisix.core.etcd")
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "limit-count": {
+                            "count": 2,
+                            "time_window": 60,
+                            "rejected_code": 503,
+                            "key": "remote_addr"
+                        }
+                    }
+                }]]
+                )
+
+            ngx.status = code
+            ngx.say(body)
+
+            local res = assert(etcd.get('/consumer_groups/company_a'))
+            local create_time = res.body.node.value.create_time
+            assert(create_time ~= nil, "create_time is nil")
+            local update_time = res.body.node.value.update_time
+            assert(update_time ~= nil, "update_time is nil")
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 11: add consumer with group
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumers/foobar',
+                ngx.HTTP_PUT,
+                [[{
+                    "username": "foobar",
+                    "plugins": {
+                        "key-auth": {
+                            "key": "auth-two"
+                        }
+                    },
+                    "group_id": "company_a"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 12: delete-consumer group failed
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.sleep(0.3)
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                 ngx.HTTP_DELETE
+            )
+            ngx.print(body)
+        }
+    }
+--- response_body
+{"error_msg":"can not delete this consumer group, consumer [foobar] is still using it now"}
+
+
+
+=== TEST 13: delete consumer
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.sleep(0.3)
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumers/foobar',
+                 ngx.HTTP_DELETE
+            )
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed
+
+
+
+=== TEST 14: delete consumer-group
+--- config
+    location /t {
+        content_by_lua_block {
+            ngx.sleep(0.3)
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/consumer_groups/company_a',
+                 ngx.HTTP_DELETE
+            )
+            ngx.say(body)
+        }
+    }
+--- response_body
+passed

Review Comment:
   Just like what other resources does, e.g. plugin_config, consumer, this test file is only for admin api test.
   I would put plugin test and consumer stuff in `t/node/consumer-group.t`, and moreover, `t/config-center-yaml/consumer-group.t`, please keep in patient.



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