You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/09/29 04:50:12 UTC
[GitHub] [apisix] imjoey opened a new pull request #2345: feat: Add lables for route object
imjoey opened a new pull request #2345:
URL: https://github.com/apache/apisix/pull/2345
Signed-off-by: imjoey <ma...@gmail.com>
### What this PR does / why we need it:
Following with #2279 , this PR is going to add `labels` property support for `Route`/`Consumer`/`Service`/`SSL` objects, as well as fix some typos introduced by #2279 .
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuxiran commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502122002
##########
File path: t/admin/consumers.t
##########
@@ -223,3 +223,78 @@ GET /t
{"error_msg":"missing consumer name"}
--- no_error_log
[error]
+
+
+
+=== TEST 7: add consumer with labels
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username":"jack",
+ "desc": "new consumer",
+ "labels": {
+ "build":"16",
+ "env":"production",
+ "version":"v2"
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "jack",
+ "desc": "new consumer",
+ "labels": {
Review comment:
node does not support `labels`, so here may need to remove `labels`?
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
> we could use label to distinguish among dozens of SSLs according to their usage
agree +1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r497144848
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
I think ssl may not need labels
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502129503
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
OK, I think it could work with SNI.But for management, labels is helpful for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuxiran commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502128374
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
> we could use label to distinguish among dozens of SSLs according to their usage
agree +1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
imjoey commented on pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#issuecomment-701047370
ping @membphis @nic-chen @gxthrj for reviewing, much appreciated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502127213
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
> we could use label to distinguish among dozens of SSLs according to their usage.
Agree this. Searching by category will be helpful.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuxiran merged pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
liuxiran merged pull request #2345:
URL: https://github.com/apache/apisix/pull/2345
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuxiran commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502122002
##########
File path: t/admin/consumers.t
##########
@@ -223,3 +223,78 @@ GET /t
{"error_msg":"missing consumer name"}
--- no_error_log
[error]
+
+
+
+=== TEST 7: add consumer with labels
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username":"jack",
+ "desc": "new consumer",
+ "labels": {
+ "build":"16",
+ "env":"production",
+ "version":"v2"
+ }
+ }]],
+ [[{
+ "node": {
+ "value": {
+ "username": "jack",
+ "desc": "new consumer",
+ "labels": {
Review comment:
node does not support `labels`, so here may need to remove `labels`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502123384
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
@nic-chen IMHO, we could use label to distinguish among dozens of SSLs according to their usage.
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
@membphis @liuxiran any thoughts on this? 😄
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
@nic-chen thanks for response. IMHO, we could use label to distinguish among dozens of SSLs according to their usage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502123559
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
@membphis @liuxiran any thoughts on 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502123384
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
@nic-chen IMHO, we could use label to distinguish among dozens of SSLs according to their usage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#issuecomment-701203439
nice PR. LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] liuxiran merged pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
liuxiran merged pull request #2345:
URL: https://github.com/apache/apisix/pull/2345
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502129503
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
OK, I think it could work with SNI.But for management, labels is helpful for 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502127213
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
> we could use label to distinguish among dozens of SSLs according to their usage.
Agree this. Searching by category will be helpful.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] imjoey commented on a change in pull request #2345: feat: Add lables for route/consumer/service/ssl objects
Posted by GitBox <gi...@apache.org>.
imjoey commented on a change in pull request #2345:
URL: https://github.com/apache/apisix/pull/2345#discussion_r502123384
##########
File path: apisix/schema_def.lua
##########
@@ -555,6 +580,14 @@ _M.ssl = {
type = "integer",
minimum = 1588262400, -- 2020/5/1 0:0:0
},
+ labels = {
Review comment:
@nic-chen thanks for response. IMHO, we could use label to distinguish among dozens of SSLs according to their usage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org