refactor: extract reusable building blocks from K8s and Nacos discovery#13201
Open
nic-6443 wants to merge 7 commits intoapache:masterfrom
Open
refactor: extract reusable building blocks from K8s and Nacos discovery#13201nic-6443 wants to merge 7 commits intoapache:masterfrom
nic-6443 wants to merge 7 commits intoapache:masterfrom
Conversation
Extract kubernetes/core.lua and nacos/client.lua as composable building blocks from the existing init.lua modules. kubernetes/core.lua provides: - Config parsing (get_apiserver, setup_namespace_selector, etc.) - Parameterized endpoint callbacks via create_endpoint_callbacks() with key_prefix for multi-registry dict isolation and duplicate_port_number for flexible port key storage - Handle factory (create_handle) and lifecycle (start_fetch) - Node resolution with LRU cache (resolve_nodes) - Dict helpers (dump_endpoints_from_dict) nacos/client.lua provides: - Stateless HTTP client primitives (request, get_token_param) - HMAC-SHA1 signing (get_signed_param) - URL building (build_base_uri) - Instance fetching (fetch_from_host) with configurable key_builder, preserve_metadata, and timeout - Service scanning (get_nacos_services) with optional filter informer_factory.lua enhancements: - Graceful stop support (informer.stop flag) checked in watch() and list_watch() loops - Configurable ssl_verify (from apiserver config, default false) - ssl_server_name support for HTTPS connections schema.lua: - Add ssl_verify option to service config for both single and multiple mode Both init.lua modules are refactored to use the new building blocks with no behavior change for existing users.
Prevent a stopped informer from continuing paginated list requests and from running post_list cleanup (which could race with a replacement informer for the same registry prefix).
There was a problem hiding this comment.
Pull request overview
This PR refactors APISIX service discovery by extracting reusable core logic from the Kubernetes and Nacos discovery implementations into standalone modules, enabling external consumers to reuse the underlying primitives while keeping existing discovery module APIs stable.
Changes:
- Added reusable Kubernetes discovery building blocks (
apisix/discovery/kubernetes/core.lua) and refactored the Kubernetes discovery init module to use them. - Added a reusable Nacos HTTP client module (
apisix/discovery/nacos/client.lua) and refactored the Nacos discovery init module to use it. - Enhanced Kubernetes informer factory to support graceful stop and configurable TLS behavior (
ssl_verify,ssl_server_name), and extended the Kubernetes discovery schema withssl_verify.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| apisix/discovery/nacos/init.lua | Refactored to use the extracted Nacos client for service scanning and instance fetching. |
| apisix/discovery/nacos/client.lua | New reusable Nacos HTTP client and discovery primitives (auth, signing, URL building, fetching). |
| apisix/discovery/kubernetes/schema.lua | Adds service.ssl_verify to schema (single + multi mode). |
| apisix/discovery/kubernetes/init.lua | Refactored to use new kubernetes.core helpers for handle creation, fetch lifecycle, and node resolution. |
| apisix/discovery/kubernetes/informer_factory.lua | Adds stop-flag checks and makes TLS verification + SNI configurable via apiserver settings. |
| apisix/discovery/kubernetes/core.lua | New reusable Kubernetes discovery core (config parsing, callbacks, handle factory, lifecycle, node resolution, dict helpers). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove unused 'type' variable in kubernetes/core.lua - Remove unused 'pairs' variable in nacos/client.lua - Shorten long comment line in nacos/client.lua
- core.lua: return empty string (not nil) from token __index on read failure - client.lua: redact credentials from log output - client.lua: validate accessToken response before concatenating - client.lua: always populate nodes_cache[key] even when empty (fix stale nodes)
Split on first colon only instead of all colons, so passwords like 'pass:word' are parsed correctly.
Add nil checks for endpoint.metadata, endpoint.metadata.namespace, and endpoint.metadata.name in on_endpoint_modified and on_endpoint_deleted, matching the existing EndpointSlice guards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extract composable building blocks from Kubernetes and Nacos discovery modules so external consumers can reuse core logic without duplicating internal implementation.
Changes
kubernetes/core.lua(new):get_apiserver,setup_namespace_selector, etc.)create_endpoint_callbacks(options)— supportskey_prefixfor multi-cluster dict isolation andduplicate_port_numberfor flexible port key storagecreate_handle) and lifecycle (start_fetch) with graceful stopresolve_nodes)dump_endpoints_from_dict)nacos/client.lua(new):request,get_token_param)get_signed_param)build_base_uri) with inline credential parsingfetch_from_host) with configurablekey_builder,preserve_metadata, andtimeoutget_nacos_services) with optional filter callbackinformer_factory.luaenhancements:informer.stopflag — checked in watch loops, list pagination, and before post_list cleanupssl_verify(from apiserver config, default false)ssl_server_namesupport for HTTPS connectionskubernetes/schema.lua:ssl_verifyoption to service config (single and multiple mode)Both
init.luamodules are refactored to use the new building blocks. No behavior change for existing users — all public APIs (init_worker,nodes,dump_data,check_discovery_ready) remain identical.Motivation
This makes the discovery modules more composable. External consumers (e.g., dynamic discovery managers that receive registry configs at runtime) can create custom handles, reuse endpoint processing callbacks, and build on the Nacos HTTP client without duplicating hundreds of lines of internal logic.
Testing
All existing
t/discovery/andt/kubernetes/discovery/tests should pass without modification.