You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "lingsamuel (via GitHub)" <gi...@apache.org> on 2023/10/07 01:03:25 UTC
[PR] feat: support config stream_route upstream ain service [apisix]
lingsamuel opened a new pull request, #10298:
URL: https://github.com/apache/apisix/pull/10298
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Support `service_id` for stream_route, allows config upstream in service.
### 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
- [ ] 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)
--
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
Re: [PR] feat: support config stream_route upstream ain service [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1350537794
##########
apisix/init.lua:
##########
@@ -1078,6 +1079,34 @@ function _M.stream_preread_phase()
api_ctx.matched_upstream = upstream
+ elseif matched_route.value.service_id then
+ local service = service_fetch(matched_route.value.service_id)
+ if not service then
+ core.log.error("failed to fetch service configuration by ",
+ "id: ", matched_route.value.service_id)
+ return core.response.exit(404)
+ end
+
+ matched_route = plugin.merge_service_stream_route(service, matched_route)
+ api_ctx.matched_route = matched_route
+ api_ctx.conf_type = "stream_route&service"
+ api_ctx.conf_version = matched_route.modifiedIndex .. "&" .. service.modifiedIndex
+ api_ctx.conf_id = matched_route.value.id .. "&" .. service.value.id
+ api_ctx.service_id = service.value.id
+ api_ctx.service_name = service.value.name
+ api_ctx.matched_upstream = matched_route.value.upstream
+ if matched_route.value.upstream_id and not matched_route.value.upstream then
+ local upstream = apisix_upstream.get_by_id(matched_route.value.upstream_id)
+ if not upstream then
+ if is_http then
+ return core.response.exit(502)
+ end
+
+ return ngx_exit(1)
Review Comment:
but in two lines above, there is: `response.exit(502)`. Why is it not sufficient? Why do you further check for `is_http` and only then return 502?
--
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
Re: [PR] feat: support config stream_route upstream ain service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10298:
URL: https://github.com/apache/apisix/pull/10298#issuecomment-1751589555
Please make the ci pass
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "lingsamuel (via GitHub)" <gi...@apache.org>.
lingsamuel commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1357833902
##########
apisix/init.lua:
##########
@@ -1021,6 +1021,7 @@ function _M.stream_init_worker()
plugin.init_worker()
xrpc.init_worker()
router.stream_init_worker()
+ require("apisix.http.service").init_worker()
Review Comment:
no, obviously service is designed for http
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10298:
URL: https://github.com/apache/apisix/pull/10298#issuecomment-1759504148
Please update the related doc: https://github.com/apache/apisix/blob/master/docs/en/latest/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
Re: [PR] feat: support config stream_route upstream ain service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10298:
URL: https://github.com/apache/apisix/pull/10298#issuecomment-1752297819
Please make the ci pass
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1356769164
##########
apisix/init.lua:
##########
@@ -1021,6 +1021,7 @@ function _M.stream_init_worker()
plugin.init_worker()
xrpc.init_worker()
router.stream_init_worker()
+ require("apisix.http.service").init_worker()
Review Comment:
Can the `service.lua` be moved out of the `http` directory?
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1356791649
##########
t/stream-node/sanity-with-service.t:
##########
@@ -0,0 +1,367 @@
+#
+# 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';
+
+log_level('info');
+no_root_location();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set stream route(id: 1) -> service(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/services/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1995": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+
+ code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.1",
+ "service_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route
+--- stream_request eval
+mmm
+--- stream_response
+hello world
+
+
+
+=== TEST 3: set stream route(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.2",
+ "service_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 4: not hit route
+--- stream_enable
+--- stream_response
+
+
+
+=== TEST 5: delete route(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_DELETE
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 6: set stream route(id: 1) with server port
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.1",
+ "server_port": 1995,
+ "service_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 7: set service upstream (id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1995": 1
+ },
+ "type": "roundrobin"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+
+ code, body = t('/apisix/admin/services/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 8: set stream route (id: 1) with service (id: 1) which uses upstream_id
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.1",
+ "service_id": 1
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 9: hit route
+--- stream_request eval
+mmm
+--- stream_response
+hello world
+
+
+
+=== TEST 10: set stream route (id: 1) which uses upstream_id and remote address with IP CIDR
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.1/26",
+ "service_id": "1"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 11: hit route
+--- stream_request eval
+mmm
+--- stream_response
+hello world
+
+
+
+=== TEST 12: reject bad CIDR
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": ":/8",
+ "service_id": "1"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.print(body)
+ }
+ }
+--- request
+GET /t
+--- error_code: 400
+--- response_body
+{"error_msg":"invalid remote_addr: :/8"}
+
+
+
+=== TEST 13: skip upstream http host check in stream subsystem
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "nodes": {
+ "127.0.0.1:1995": 1,
+ "127.0.0.2:1995": 1
+ },
+ "pass_host": "node",
+ "type": "roundrobin"
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 14: hit route
+--- stream_request eval
+mmm
+--- stream_response
+hello world
+
+
+
+=== TEST 15: reuse ctx and more
Review Comment:
Why need this test case?
--
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
Re: [PR] feat: support config stream_route upstream ain service [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1349514409
##########
apisix/init.lua:
##########
@@ -1078,6 +1079,34 @@ function _M.stream_preread_phase()
api_ctx.matched_upstream = upstream
+ elseif matched_route.value.service_id then
+ local service = service_fetch(matched_route.value.service_id)
+ if not service then
+ core.log.error("failed to fetch service configuration by ",
+ "id: ", matched_route.value.service_id)
+ return core.response.exit(404)
+ end
+
+ matched_route = plugin.merge_service_stream_route(service, matched_route)
+ api_ctx.matched_route = matched_route
+ api_ctx.conf_type = "stream_route&service"
+ api_ctx.conf_version = matched_route.modifiedIndex .. "&" .. service.modifiedIndex
+ api_ctx.conf_id = matched_route.value.id .. "&" .. service.value.id
+ api_ctx.service_id = service.value.id
+ api_ctx.service_name = service.value.name
+ api_ctx.matched_upstream = matched_route.value.upstream
+ if matched_route.value.upstream_id and not matched_route.value.upstream then
+ local upstream = apisix_upstream.get_by_id(matched_route.value.upstream_id)
+ if not upstream then
+ if is_http then
+ return core.response.exit(502)
+ end
+
+ return ngx_exit(1)
Review Comment:
why `return ngx_exit(1)`
##########
apisix/plugin.lua:
##########
@@ -636,6 +639,47 @@ function _M.merge_service_route(service_conf, route_conf)
service_conf, route_conf)
end
+local function merge_service_stream_route(service_conf, route_conf)
+ -- because many fields in Service are not supported by stream route,
+ -- so we copy the stream route as base object
+ local new_conf = core.table.deepcopy(route_conf)
+ if service_conf.value.plugins then
+ for name, conf in pairs(service_conf.value.plugins) do
+ if not new_conf.value.plugins then
+ new_conf.value.plugins = {}
+ end
+
+ if not new_conf.value.plugins[name] then
+ new_conf.value.plugins[name] = conf
+ end
+ end
+ end
+
+ new_conf.value.service_id = nil
+
+ if not new_conf.value.upstream and service_conf.value.upstream then
+ new_conf.value.upstream = service_conf.value.upstream
+ end
+
+ if not new_conf.value.upstream_id and service_conf.value.upstream_id then
+ new_conf.value.upstream_id = service_conf.value.upstream_id
+ end
+
+ --core.log.info("merged stream_route conf : ", require("inspect")(new_conf))
Review Comment:
remove this line
##########
apisix/http/service.lua:
##########
@@ -61,7 +61,7 @@ function _M.init_worker()
filter = filter,
})
if not services then
- error("failed to create etcd instance for fetching upstream: " .. err)
+ error("failed to create etcd instance for fetching /services: " .. err)
Review Comment:
```suggestion
error("failed to create etcd instance for fetching services: " .. err)
```
--
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
Re: [PR] feat: support config stream_route upstream ain service [apisix]
Posted by "lingsamuel (via GitHub)" <gi...@apache.org>.
lingsamuel commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1349606036
##########
apisix/http/service.lua:
##########
@@ -61,7 +61,7 @@ function _M.init_worker()
filter = filter,
})
if not services then
- error("failed to create etcd instance for fetching upstream: " .. err)
+ error("failed to create etcd instance for fetching /services: " .. err)
Review Comment:
other resources have similar format. it indicates the etcd key
##########
apisix/init.lua:
##########
@@ -1078,6 +1079,34 @@ function _M.stream_preread_phase()
api_ctx.matched_upstream = upstream
+ elseif matched_route.value.service_id then
+ local service = service_fetch(matched_route.value.service_id)
+ if not service then
+ core.log.error("failed to fetch service configuration by ",
+ "id: ", matched_route.value.service_id)
+ return core.response.exit(404)
+ end
+
+ matched_route = plugin.merge_service_stream_route(service, matched_route)
+ api_ctx.matched_route = matched_route
+ api_ctx.conf_type = "stream_route&service"
+ api_ctx.conf_version = matched_route.modifiedIndex .. "&" .. service.modifiedIndex
+ api_ctx.conf_id = matched_route.value.id .. "&" .. service.value.id
+ api_ctx.service_id = service.value.id
+ api_ctx.service_name = service.value.name
+ api_ctx.matched_upstream = matched_route.value.upstream
+ if matched_route.value.upstream_id and not matched_route.value.upstream then
+ local upstream = apisix_upstream.get_by_id(matched_route.value.upstream_id)
+ if not upstream then
+ if is_http then
+ return core.response.exit(502)
+ end
+
+ return ngx_exit(1)
Review Comment:
because no upstream available here
--
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
Re: [PR] feat: support config stream_route upstream ain service [apisix]
Posted by "lingsamuel (via GitHub)" <gi...@apache.org>.
lingsamuel commented on PR #10298:
URL: https://github.com/apache/apisix/pull/10298#issuecomment-1751881337
> Please make the ci pass
it can't pass in master
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1356782535
##########
t/stream-node/sanity-with-service.t:
##########
@@ -0,0 +1,367 @@
+#
+# 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';
+
+log_level('info');
+no_root_location();
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: set stream route(id: 1) -> service(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/services/1',
+ ngx.HTTP_PUT,
+ [[{
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1995": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+
+ code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.1",
+ "service_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route
+--- stream_request eval
+mmm
+--- stream_response
+hello world
+
+
+
+=== TEST 3: set stream route(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "remote_addr": "127.0.0.2",
+ "service_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 4: not hit route
+--- stream_enable
+--- stream_response
+
+
+
+=== TEST 5: delete route(id: 1)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/1',
+ ngx.HTTP_DELETE
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 6: set stream route(id: 1) with server port
Review Comment:
Why need this test case?
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1356778306
##########
apisix/plugin.lua:
##########
@@ -636,6 +639,46 @@ function _M.merge_service_route(service_conf, route_conf)
service_conf, route_conf)
end
+local function merge_service_stream_route(service_conf, route_conf)
+ -- because many fields in Service are not supported by stream route,
+ -- so we copy the stream route as base object
+ local new_conf = core.table.deepcopy(route_conf)
+ if service_conf.value.plugins then
+ for name, conf in pairs(service_conf.value.plugins) do
+ if not new_conf.value.plugins then
+ new_conf.value.plugins = {}
+ end
+
+ if not new_conf.value.plugins[name] then
+ new_conf.value.plugins[name] = conf
+ end
+ end
+ end
+
+ new_conf.value.service_id = nil
+
+ if not new_conf.value.upstream and service_conf.value.upstream then
+ new_conf.value.upstream = service_conf.value.upstream
+ end
+
+ if not new_conf.value.upstream_id and service_conf.value.upstream_id then
+ new_conf.value.upstream_id = service_conf.value.upstream_id
+ end
+
+ return new_conf
+end
+
+function _M.merge_service_stream_route(service_conf, route_conf)
Review Comment:
Two blanks are needed between functions
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #10298:
URL: https://github.com/apache/apisix/pull/10298#discussion_r1356815739
##########
apisix/init.lua:
##########
@@ -1078,6 +1079,34 @@ function _M.stream_preread_phase()
api_ctx.matched_upstream = upstream
+ elseif matched_route.value.service_id then
+ local service = service_fetch(matched_route.value.service_id)
+ if not service then
+ core.log.error("failed to fetch service configuration by ",
+ "id: ", matched_route.value.service_id)
+ return core.response.exit(404)
+ end
+
+ matched_route = plugin.merge_service_stream_route(service, matched_route)
+ api_ctx.matched_route = matched_route
+ api_ctx.conf_type = "stream_route&service"
+ api_ctx.conf_version = matched_route.modifiedIndex .. "&" .. service.modifiedIndex
+ api_ctx.conf_id = matched_route.value.id .. "&" .. service.value.id
+ api_ctx.service_id = service.value.id
+ api_ctx.service_name = service.value.name
+ api_ctx.matched_upstream = matched_route.value.upstream
+ if matched_route.value.upstream_id and not matched_route.value.upstream then
+ local upstream = apisix_upstream.get_by_id(matched_route.value.upstream_id)
+ if not upstream then
+ if is_http then
+ return core.response.exit(502)
+ end
+
+ return ngx_exit(1)
Review Comment:
@shreemaan-abhishek it's just copy from the code above
```
local up_id = matched_route.value.upstream_id
if up_id then
local upstream = apisix_upstream.get_by_id(up_id)
if not upstream then
if is_http then
return core.response.exit(502)
end
return ngx_exit(1)
end
api_ctx.matched_upstream = upstream
```
--
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
Re: [PR] feat: support config stream_route upstream in service [apisix]
Posted by "soulbird (via GitHub)" <gi...@apache.org>.
soulbird merged PR #10298:
URL: https://github.com/apache/apisix/pull/10298
--
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