feat(nacos): add metadata filtering support to nacos discovery #12445
Conversation
- Add metadata validation schema in schema_def.lua - Implement metadata filtering logic in nacos discovery - Add comprehensive test cases for metadata validation - Update documentation for metadata filtering feature - Fix lint issues and code formatting
dosubot
bot
added
size:L
|
There seems to be duplicate work #12448 (coincidentally, we have similar demands). According to my understanding of apisix, all routes share the same service discovery results, so it is not suitable to filter in the discovery. |
I don't think it's a duplicate at the moment. Using metadata to filter nacos services is a very useful feature. For more information, you can check the related issue. |
Got it, thanks for you reply :) This is used to filter when pulling the service list from service discovery to avoid pulling a large number of useless instances. We also have similar demands when using Consul. I will try to add the following. |
docs keep same with apache#12445, but use metadata_match and array values
docs keep same with apache#12445, but use metadata_match and array values
There was a problem hiding this comment.
After checking #12392 and #12464, I have a question:
I haven't seen any discussions that should be implemented as single-value matching or multi-value matching?
#12464 Propose:
Currently, someone in Nacos is also implementing the same function #12392 #12445, but only supports single value matching. However, we hope to support multi-value matching.
This is because user requests often do not have any tags, so if we want to allow multiple instances with different metadata to provide services together, we must register a special metadata to group them, but this grouping cannot be changed dynamically.
I think it makes sense.
But I am not an expert in this field, so what do you think?
apisix/discovery/nacos/init.lua
Outdated
| local function metadata_contains(host_metadata, route_metadata) | ||
| if not route_metadata or not next(route_metadata) then | ||
| return true | ||
| end | ||
| if not host_metadata or not next(host_metadata) then | ||
| return false | ||
| end | ||
|
|
||
| for k, v in pairs(route_metadata) do | ||
| if host_metadata[k] ~= v then | ||
| return false | ||
| end | ||
| end | ||
| return true | ||
| end |
There was a problem hiding this comment.
Can u abstract this func into a separate file for reuse?
| -- Apply metadata filtering if specified | ||
| local route_metadata = discovery_args and discovery_args.metadata | ||
| if route_metadata and next(route_metadata) then | ||
| local filtered_nodes = {} | ||
| for _, node in ipairs(nodes) do | ||
| if metadata_contains(node.metadata, route_metadata) then | ||
| core.table.insert(filtered_nodes, node) | ||
| end | ||
| end | ||
| return filtered_nodes | ||
| end | ||
|
|
Baoyuantop
added
the
wait for update
|
@beardnick ping |
Certainly, that sounds very reasonable. I’m happy to support multi-value matching. |
|
@beardnick Are you planning to implement this in this PR, or in another one? If you want to implement it in other PRs, then this PR is ready for review. |
In this PR. |
…pport multi-value arrays while preserving prior structure This updates the previous metadata validation tests to assert array-of-string values (multi-value matching) and keeps the original table-driven layout for easier review.
|
@beardnick Please fix the errors reported in the CI. |
The failed tests seem unrelated to my PR. Could you please rerun them? |
Description
Add metadata filtering support to nacos discovery.
Which issue(s) this PR fixes:
Fixes #12392
Checklist