feat: rust backend apisix standalone - #559
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
bzp2010
added
the
test/apisix-standalone
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
rust/crates/adc-sdk/src/backend/mod.rs (1)
44-48: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAlign the trait documentation with the optional event contract.
BackendSyncResultdocuments batch/server-level results withevent: None, but theBackend::syncdocumentation still describes failures as per-event results. State that a backend may return one result per batch or server and thatserveridentifies the target. This prevents callers from assuming that every result maps to one input event.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-sdk/src/backend/mod.rs` around lines 44 - 48, Update the Backend::sync documentation to state that results may be emitted per batch or server rather than per input event, and clarify that the server field identifies the target. Keep the existing optional event contract in BackendSyncResult consistent with this description.rust/crates/adc-backend-apisix-standalone/Cargo.toml (1)
25-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider moving
dashmapandindexmapto workspace dependencies.Every other dependency in this manifest uses
workspace = true.dashmapandindexmappin versions inline. If another crate later needs either one, the versions can drift. Declare both in the root[workspace.dependencies]and reference them withworkspace = true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/Cargo.toml` around lines 25 - 26, Move the version declarations for dashmap and indexmap from this crate’s dependencies into the root [workspace.dependencies] table, then update the manifest entries for dashmap and indexmap to use workspace = true, matching the existing workspace dependency pattern.rust/crates/adc-backend-apisix-standalone/src/backend.rs (1)
69-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReject an ambiguous token count instead of silently reusing
tokens[0].
paired_tokensis true only whentokens.len() == servers.len(). For any other non-empty count, every server receivestokens[0]. A caller that supplies 2 tokens for 3 servers gets no error here. The third server then rejects the admin API request with a 401, and the cause is not visible from that error.Accept exactly one token or exactly
servers.len()tokens, and reject every other count.♻️ Proposed change
let servers_count = opts.servers.len(); + if opts.tokens.is_empty() { + return Err(BackendError::Other( + "apisix-standalone backend requires at least one token".into(), + )); + } + if opts.tokens.len() != 1 && opts.tokens.len() != servers_count { + return Err(BackendError::Other(format!( + "apisix-standalone backend requires either 1 token shared by every server or exactly {servers_count} tokens, got {}", + opts.tokens.len() + ))); + } // A `token` per `server`, positionally paired, when the two lists // are the same length; otherwise every server shares `tokens[0]` — // matches the TS backend's own `opts.token.split(',')` convention. let paired_tokens = opts.tokens.len() == servers_count;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/src/backend.rs` around lines 69 - 82, Update the token validation in the server construction flow around paired_tokens to accept only one token or exactly servers_count tokens; reject all other counts with a BackendError before mapping servers. Preserve positional token selection for the exact-length case and shared-token behavior for the single-token case.rust/crates/adc-backend-apisix-standalone/src/fetcher.rs (1)
83-91: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winOne unreachable server fails the whole
dump.
find_latestprobes every server and line 87 propagates the firstErrit finds. A standalone cluster is deployed as n redundant instances. If one instance is down or slow enough to time out,dumpreturns an error even though the remaining instances hold a readable config.The doc comment at lines 62-65 states that this is intentional. Confirm that the TypeScript backend behaves the same way. If it tolerates partial probe failures, consider skipping failed probes and returning
Erronly when every probe fails.♻️ Proposed change if partial failures should be tolerated
let results = concurrent_map(self.servers.clone(), None, probe).await; let mut latest: Option<(String, i64)> = None; + let mut last_error: Option<BackendError> = None; + let mut probed = 0usize; for result in results { - let (server, timestamp) = result?; + let (server, timestamp) = match result { + Ok(value) => value, + Err(error) => { + last_error = Some(error); + continue; + } + }; + probed += 1; if latest.as_ref().is_none_or(|(_, best)| timestamp >= *best) { latest = Some((server, timestamp)); } } + if probed == 0 { + if let Some(error) = last_error { + return Err(error); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/src/fetcher.rs` around lines 83 - 91, Update find_latest to tolerate individual probe failures by skipping Err results from concurrent_map and considering only successful (server, timestamp) pairs. Track whether any probe succeeded and return an error only when all probes fail, while preserving the latest-timestamp selection and the documented behavior consistent with the TypeScript backend.rust/crates/adc-backend-apisix-standalone/src/transformer.rs (2)
180-227: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
to_adcscans every collection once per service.For each service, this block iterates
input.upstreamstwice,input.routesonce, andinput.stream_routesonce. The total cost is O(services × (upstreams + routes + stream_routes)). A standalone document with 500 services and 5000 routes performs 2.5 million comparisons on everydump, anddumpalso runs after everysyncto refresh the cachedConfiguration.Build the per-service groupings once before the loop.
♻️ Proposed refactor sketch
+ use std::collections::HashMap; + + let mut routes_by_service: HashMap<&str, Vec<adc::Route>> = HashMap::new(); + for route in input.routes.iter().flatten() { + routes_by_service.entry(route.service_id.as_str()).or_default().push(route_to_adc(route)); + } + let mut stream_routes_by_service: HashMap<&str, Vec<adc::StreamRoute>> = HashMap::new(); + for route in input.stream_routes.iter().flatten() { + stream_routes_by_service + .entry(route.service_id.as_str()) + .or_default() + .push(stream_route_to_adc(route)); + } + let upstreams_by_id: HashMap<&str, &typing::Upstream> = + input.upstreams.iter().flatten().map(|u| (u.id.as_str(), u)).collect(); + let mut named_by_service: HashMap<&str, Vec<&typing::Upstream>> = HashMap::new(); + for upstream in input.upstreams.iter().flatten() { + if let Some(owner) = upstream + .labels + .as_ref() + .and_then(|labels| labels.get(typing::ADC_UPSTREAM_SERVICE_ID_LABEL)) + { + named_by_service.entry(owner.as_str()).or_default().push(upstream); + } + }Then look each one up by
service.idinside themap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/src/transformer.rs` around lines 180 - 227, Refactor to_adc so the input.upstreams, input.routes, and input.stream_routes collections are grouped by owning service ID once before the service map, rather than filtered inside it. Within the service closure, look up each precomputed grouping using service.id and preserve the existing upstream conversion, label handling, and route mapping behavior.
145-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePass the credentials prefix in instead of rebuilding it twice.
Line 153 builds
format!("{username}/credentials/"), and line 265 builds the identical string inside a filter closure. The closure version allocates once for every (consumer, credential) pair. The two sites must also stay in agreement, because line 265 selects the credentials and line 154 strips the prefix from them.Compute the prefix once per consumer and pass it to
credential_to_adc.♻️ Proposed refactor
-fn credential_to_adc(credential: &typing::ConsumerCredential, username: &str) -> Option<adc::ConsumerCredential> { +fn credential_to_adc(credential: &typing::ConsumerCredential, prefix: &str) -> Option<adc::ConsumerCredential> { let plugins = credential.plugins.clone()?; let (plugin_name, config) = plugins.into_iter().next()?; let config = match config { Value::Object(map) => map, _ => Map::new(), }; - let prefix = format!("{username}/credentials/"); - let id = credential.id.strip_prefix(&prefix).unwrap_or(&credential.id).to_string(); + let id = credential.id.strip_prefix(prefix).unwrap_or(&credential.id).to_string();At the call site:
.map(|consumer| { + let prefix = format!("{}/credentials/", consumer.username); let owned: Vec<adc::ConsumerCredential> = credentials .iter() - .filter(|credential| credential.id.starts_with(&format!("{}/credentials/", consumer.username))) - .filter_map(|credential| credential_to_adc(credential, &consumer.username)) + .filter(|credential| credential.id.starts_with(&prefix)) + .filter_map(|credential| credential_to_adc(credential, &prefix)) .collect();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/src/transformer.rs` around lines 145 - 164, Compute the credentials prefix once per consumer at the call site and pass it into credential_to_adc instead of constructing it inside that function. Update credential_to_adc’s signature and use the supplied prefix for strip_prefix, while reusing the same prefix in the credential-selection filter so both paths remain consistent and avoid per-pair allocations.rust/crates/adc-backend-apisix-standalone/src/typing.rs (1)
245-257: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider defaulting
statusso one legacy SSL entry cannot fail the whole document.
snis,cert,key, andstatushave no#[serde(default)]. The document is deserialized as a single unit. If any one SSL entry omitsstatus, the entireApisixStandalonedeserialization fails andFetcher::dumpreturns an error for every resource, not just that SSL.Route.statusat line 73 is alreadyOption<i64>, so the treatment is inconsistent between the two models.♻️ Proposed change
#[serde(default, skip_serializing_if = "Option::is_none")] pub ssl_protocols: Option<Vec<SslProtocol>>, - pub status: i64, + #[serde(default = "default_ssl_status")] + pub status: i64, } + +fn default_ssl_status() -> i64 { + 1 +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/src/typing.rs` around lines 245 - 257, Add serde default handling for the status field in the SSL model so entries that omit status deserialize successfully, matching the optional treatment used by Route.status. Update the status declaration near certs, keys, client, and ssl_protocols while preserving the existing status type and serialization behavior.rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_upstream.rs (1)
85-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPositional assertions on
upstreamsmake the test depend on differ event order.Lines 86-88 assert that
upstreams[0],upstreams[1], andupstreams[2]are the default upstream,nd-upstream1, andnd-upstream2, in that order. That order comes from the order in whichOperator::syncapplies the differ's events, which is not part of any asserted contract.Line 136 and line 141 repeat the dependency, and line 148 checks
named[0].nodes[0].hostwithout first confirming thatnamed[0]isnd-upstream1. If the order changes, line 148 asserts against the wrong upstream and the test either fails for an unrelated reason or passes for the wrong one.Look each upstream up by name.
♻️ Proposed change
+fn by_name<'a>( + upstreams: &'a [adc_backend_apisix_standalone::tests::typing::Upstream], + name: &str, +) -> &'a adc_backend_apisix_standalone::tests::typing::Upstream { + upstreams.iter().find(|u| u.name == name).unwrap_or_else(|| panic!("no upstream named {name}")) +}let upstreams = raw.upstreams.unwrap(); assert_eq!(upstreams.len(), 3); - assert_eq!(upstreams[1].name, "nd-upstream1"); + let nd1 = by_name(&upstreams, "nd-upstream1"); assert_eq!( - upstreams[1].labels.as_ref().and_then(|l| l.get(ADC_UPSTREAM_SERVICE_ID_LABEL)), + nd1.labels.as_ref().and_then(|l| l.get(ADC_UPSTREAM_SERVICE_ID_LABEL)), Some(&generate_id("test")) ); - assert_eq!(upstreams[1].nodes.as_ref().unwrap()[0].host, "8.8.8.8"); + assert_eq!(nd1.nodes.as_ref().unwrap()[0].host, "8.8.8.8");Apply the same lookup to the ADC-facing
namedlist at lines 145-148.Also applies to: 133-148
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_upstream.rs` around lines 85 - 91, Replace positional indexing in the upstream assertions with name-based lookups, covering both the `upstreams` collection and the ADC-facing `named` list. Update the checks around the existing upstream assertions and node-host validation to first locate `test`, `nd-upstream1`, and `nd-upstream2` by name, then assert their labels and nodes without relying on event ordering..github/workflows/e2e.yaml (1)
144-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSet up the Rust toolchain before
Swatinem/rust-cache.The cache key includes the installed Rust toolchains.
rustup update stablecan change that state after the cache key is created, which causes cache misses and rebuilds. Move the existingrustup update stableandrustup default stablecommands before the cache action. The repository has norust-toolchainpin.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/e2e.yaml around lines 144 - 152, Move the existing rustup update stable and rustup default stable commands from the Run Rust E2E tests step to a setup step before Swatinem/rust-cache. Keep the cache action after the stable toolchain is installed and retain the cargo test command unchanged.rust/crates/adc-backend-apisix-standalone/src/operator.rs (1)
403-407: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider making
Createidempotent.
EventType::Createpushes unconditionally. If the base config already contains an entry with the same id, the collection gets two entries with that id. This can happen whenold_raw_configis stale relative to the servers. Replacing an existing entry with the same identity would keep the document well-formed in that case.♻️ Proposed change
EventType::Create => { - field.get_or_insert_with(Vec::new).push(build()?); + let target_id = generate_id_from_event(event)?; + let vec = field.get_or_insert_with(Vec::new); + match vec.iter_mut().find(|item| identity(item) == target_id) { + Some(slot) => *slot = build()?, + None => vec.push(build()?), + } Ok(true) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/src/operator.rs` around lines 403 - 407, Update the EventType::Create branch in the event handling match to replace an existing collection entry with the same identity instead of unconditionally appending build()?; retain the append behavior when no matching entry exists and preserve the existing Ok(true) result.rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_global_rule.rs (1)
85-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the raw conf-version reader into
common.
raw_global_rules_conf_versionduplicatesraw_consumers_conf_versioninrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_consumer.rs(Lines 26-37). Only the JSON key differs. A single helper intests/common/mod.rsthat takes the field name would remove the duplication.♻️ Proposed helper for tests/common/mod.rs
/// Reads a `*_conf_version` field straight off the admin API — bypasses /// this crate's own cache entirely. pub async fn raw_conf_version(field: &str) -> Option<i64> { let client = HttpClient::new(HttpClientConfig { server: SERVER1.to_string(), token: TOKEN.to_string(), timeout: None, tls: TlsConfig::default(), }) .unwrap(); let request = client.request(Method::GET, "/apisix/admin/configs").unwrap(); let body: Value = client.send_json(request).await.unwrap(); body.get(field).and_then(|v| v.as_i64()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_global_rule.rs` around lines 85 - 96, Move the shared admin API reading logic from raw_global_rules_conf_version and raw_consumers_conf_version into a common::raw_conf_version(field: &str) helper in tests/common/mod.rs. Have the helper construct the request and return the requested JSON field as Option<i64>, then replace both resource-specific readers with calls using their respective field names and remove the duplicated implementations.rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs (1)
25-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the
adc::Upstreamandadc::Servicefixture builders. Four test files each spell out every field ofadc::Upstreamandadc::Service. The shared root cause is thattests/common/mod.rsprovides event helpers but no resource fixture builders. Each new field added to either SDK struct now forces four edits.
rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs#L25-L64: movebase_upstreamandbase_serviceintotests/common/mod.rsaspub fn, and import them here.rust/crates/adc-backend-apisix-standalone/tests/e2e_cache.rs#L36-L75: delete the local copies and use thecommonversions.rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_inline_upstream.rs#L38-L77: delete the local copies; set the pinnedSERVICE_NAMEwith struct update syntax oncommon::base_service().rust/crates/adc-backend-apisix-standalone/tests/e2e_validate.rs#L49-L99: replace both inlineadc::Service/adc::Upstreamliterals withcommon::base_service()andcommon::base_upstream()plus the fields each test actually sets.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs` around lines 25 - 64, Centralize the adc::Upstream and adc::Service fixture builders in tests/common/mod.rs as public base_upstream and base_service functions, then import and reuse them. In rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs#L25-L64, move the local builders and import the common versions; in rust/crates/adc-backend-apisix-standalone/tests/e2e_cache.rs#L36-L75 and rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_inline_upstream.rs#L38-L77, remove local copies and use common::base_* (setting SERVICE_NAME via struct update in the latter); in rust/crates/adc-backend-apisix-standalone/tests/e2e_validate.rs#L49-L99, replace inline literals with common::base_service() and common::base_upstream() while overriding only test-specific fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/crates/adc-backend-apisix-standalone/src/backend.rs`:
- Around line 183-192: Update ApisixStandalone::sync to handle a missing
Cache::global().raw_config entry by re-fetching the current raw document through
the existing Fetcher::dump flow before constructing Operator, rather than using
unwrap_or_default. Preserve the cached document path when present, and propagate
any re-fetch failure as the sync error so an empty base is never PUT to the
servers.
- Around line 127-138: Define a shared constant for the unknown APISIX version
sentinel instead of repeating Version::new(999, 999, 999). Update the
request-selection logic to use GET whenever the parsed version equals that
sentinel, while retaining the existing HEAD behavior for known versions; also
use the constant in the cache guard near the version-fetch flow.
In `@rust/crates/adc-backend-apisix-standalone/src/cache.rs`:
- Around line 80-89: Update Cache::get_live so expired-entry cleanup uses the
concurrent map’s remove_if operation, rechecking the entry’s expiry predicate at
removal time instead of unconditionally deleting by key. Preserve returning None
for the expired lookup and returning a cloned live entry, while ensuring a
concurrently refreshed entry is retained.
In `@rust/crates/adc-backend-apisix-standalone/src/operator.rs`:
- Around line 541-555: Update the Upstream update and delete branches in the
event handling logic to use config.upstreams.as_mut() instead of
get_or_insert_with(Vec::new), returning early when upstreams is None. Preserve
the existing replacement/removal and version-increment behavior when the
collection exists, while keeping absent upstreams as None for serialization and
raw-cache storage.
- Around line 81-105: Update the exit_on_failure branch in the sync method
around concurrent_map_until_err so any error path invalidates the affected cache
entry before propagating the error. Ensure in-flight PUT failures cannot leave
the previous configuration, raw config, or version available to later
non-bypassing dump calls, while preserving the existing successful-result cache
updates.
In `@rust/crates/adc-backend-apisix-standalone/src/typing.rs`:
- Around line 29-35: Correct the documentation for ADC_UPSTREAM_SERVICE_ID_LABEL
to acknowledge this crate’s dependency on adc-backend-apisix, while explaining
that the constant remains duplicated because the wire shapes differ and shared
code is not suitable. Leave the constant and its behavior unchanged.
- Around line 76-90: Update deserialize_upstream_nodes to accept non-empty JSON
objects representing map-form nodes, converting each map entry into the
corresponding UpstreamNode collection format used by the regular APISIX backend.
Preserve the existing None/null handling, normalize an empty object to an empty
vector, and retain array-form deserialization.
---
Nitpick comments:
In @.github/workflows/e2e.yaml:
- Around line 144-152: Move the existing rustup update stable and rustup default
stable commands from the Run Rust E2E tests step to a setup step before
Swatinem/rust-cache. Keep the cache action after the stable toolchain is
installed and retain the cargo test command unchanged.
In `@rust/crates/adc-backend-apisix-standalone/Cargo.toml`:
- Around line 25-26: Move the version declarations for dashmap and indexmap from
this crate’s dependencies into the root [workspace.dependencies] table, then
update the manifest entries for dashmap and indexmap to use workspace = true,
matching the existing workspace dependency pattern.
In `@rust/crates/adc-backend-apisix-standalone/src/backend.rs`:
- Around line 69-82: Update the token validation in the server construction flow
around paired_tokens to accept only one token or exactly servers_count tokens;
reject all other counts with a BackendError before mapping servers. Preserve
positional token selection for the exact-length case and shared-token behavior
for the single-token case.
In `@rust/crates/adc-backend-apisix-standalone/src/fetcher.rs`:
- Around line 83-91: Update find_latest to tolerate individual probe failures by
skipping Err results from concurrent_map and considering only successful
(server, timestamp) pairs. Track whether any probe succeeded and return an error
only when all probes fail, while preserving the latest-timestamp selection and
the documented behavior consistent with the TypeScript backend.
In `@rust/crates/adc-backend-apisix-standalone/src/operator.rs`:
- Around line 403-407: Update the EventType::Create branch in the event handling
match to replace an existing collection entry with the same identity instead of
unconditionally appending build()?; retain the append behavior when no matching
entry exists and preserve the existing Ok(true) result.
In `@rust/crates/adc-backend-apisix-standalone/src/transformer.rs`:
- Around line 180-227: Refactor to_adc so the input.upstreams, input.routes, and
input.stream_routes collections are grouped by owning service ID once before the
service map, rather than filtered inside it. Within the service closure, look up
each precomputed grouping using service.id and preserve the existing upstream
conversion, label handling, and route mapping behavior.
- Around line 145-164: Compute the credentials prefix once per consumer at the
call site and pass it into credential_to_adc instead of constructing it inside
that function. Update credential_to_adc’s signature and use the supplied prefix
for strip_prefix, while reusing the same prefix in the credential-selection
filter so both paths remain consistent and avoid per-pair allocations.
In `@rust/crates/adc-backend-apisix-standalone/src/typing.rs`:
- Around line 245-257: Add serde default handling for the status field in the
SSL model so entries that omit status deserialize successfully, matching the
optional treatment used by Route.status. Update the status declaration near
certs, keys, client, and ssl_protocols while preserving the existing status type
and serialization behavior.
In `@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_global_rule.rs`:
- Around line 85-96: Move the shared admin API reading logic from
raw_global_rules_conf_version and raw_consumers_conf_version into a
common::raw_conf_version(field: &str) helper in tests/common/mod.rs. Have the
helper construct the request and return the requested JSON field as Option<i64>,
then replace both resource-specific readers with calls using their respective
field names and remove the duplicated implementations.
In
`@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_upstream.rs`:
- Around line 85-91: Replace positional indexing in the upstream assertions with
name-based lookups, covering both the `upstreams` collection and the ADC-facing
`named` list. Update the checks around the existing upstream assertions and
node-host validation to first locate `test`, `nd-upstream1`, and `nd-upstream2`
by name, then assert their labels and nodes without relying on event ordering.
In `@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs`:
- Around line 25-64: Centralize the adc::Upstream and adc::Service fixture
builders in tests/common/mod.rs as public base_upstream and base_service
functions, then import and reuse them. In
rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs#L25-L64,
move the local builders and import the common versions; in
rust/crates/adc-backend-apisix-standalone/tests/e2e_cache.rs#L36-L75 and
rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_inline_upstream.rs#L38-L77,
remove local copies and use common::base_* (setting SERVICE_NAME via struct
update in the latter); in
rust/crates/adc-backend-apisix-standalone/tests/e2e_validate.rs#L49-L99, replace
inline literals with common::base_service() and common::base_upstream() while
overriding only test-specific fields.
In `@rust/crates/adc-sdk/src/backend/mod.rs`:
- Around line 44-48: Update the Backend::sync documentation to state that
results may be emitted per batch or server rather than per input event, and
clarify that the server field identifies the target. Keep the existing optional
event contract in BackendSyncResult consistent with this description.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99cc9298-3030-46c7-841e-57cc5cbda928
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
.github/workflows/e2e.yamlrust/Cargo.tomlrust/crates/adc-backend-api7/src/operator.rsrust/crates/adc-backend-apisix-standalone/Cargo.tomlrust/crates/adc-backend-apisix-standalone/src/backend.rsrust/crates/adc-backend-apisix-standalone/src/cache.rsrust/crates/adc-backend-apisix-standalone/src/fetcher.rsrust/crates/adc-backend-apisix-standalone/src/lib.rsrust/crates/adc-backend-apisix-standalone/src/operator.rsrust/crates/adc-backend-apisix-standalone/src/transformer.rsrust/crates/adc-backend-apisix-standalone/src/typing.rsrust/crates/adc-backend-apisix-standalone/src/utils.rsrust/crates/adc-backend-apisix-standalone/tests/common/mod.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_cache.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_consumer.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_global_rule.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_inline_upstream.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_upstream.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_validate.rsrust/crates/adc-backend-apisix/src/lib.rsrust/crates/adc-backend-apisix/src/operator.rsrust/crates/adc-backend-apisix/tests/e2e_apisix.rsrust/crates/adc-backend-apisix/tests/e2e_operator.rsrust/crates/adc-backend-apisix/tests/e2e_resource_service_upstream.rsrust/crates/adc-backend-apisix/tests/e2e_sync_and_dump.rsrust/crates/adc-cli/src/main.rsrust/crates/adc-sdk/src/backend/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/crates/adc-backend-apisix-standalone/tests/common/mod.rs (1)
63-68: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFail closed when
BACKEND_APISIX_VERSIONis unset. CI sets the variable, but the standalone compose file defaults onlyBACKEND_APISIX_IMAGEtodev. Local runs can therefore use an image whose version does not match999.999.999, bypassingskip_below_3_17_0!().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/tests/common/mod.rs` around lines 63 - 68, Update apisix_version so an unset BACKEND_APISIX_VERSION fails closed instead of returning semver::Version::new(999, 999, 999). Preserve the existing invalid-value panic for configured versions, and make the missing-variable path fail explicitly so skip_below_3_17_0!() cannot be bypassed.
🧹 Nitpick comments (1)
rust/crates/adc-backend-apisix-standalone/tests/common/mod.rs (1)
144-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the shared utility extraction.
rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rsstill defines local copies ofwait_until_ready,raw_conf_version,base_upstream, andbase_serviceat Lines [159]-[188], [214]-[225], [230]-[253], and [257]-[271]. Update that test to usecommon::*and remove the local copies. Otherwise utility fixes can diverge between test binaries.Also applies to: 210-271
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/crates/adc-backend-apisix-standalone/tests/common/mod.rs` around lines 144 - 188, Update e2e_resource_service.rs to import and use the shared utilities from common::* instead of defining local copies of wait_until_ready, raw_conf_version, base_upstream, and base_service. Remove those four local definitions while preserving all existing call sites and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_global_rule.rs`:
- Around line 55-58: Update the version assertions in the unchanged global-rules
resync test to require both raw_conf_version results to be present before
comparing values. Unwrap or otherwise validate version_before and version_after,
then compare the resulting i64 values while preserving the existing no-bump
assertion message.
---
Outside diff comments:
In `@rust/crates/adc-backend-apisix-standalone/tests/common/mod.rs`:
- Around line 63-68: Update apisix_version so an unset BACKEND_APISIX_VERSION
fails closed instead of returning semver::Version::new(999, 999, 999). Preserve
the existing invalid-value panic for configured versions, and make the
missing-variable path fail explicitly so skip_below_3_17_0!() cannot be
bypassed.
---
Nitpick comments:
In `@rust/crates/adc-backend-apisix-standalone/tests/common/mod.rs`:
- Around line 144-188: Update e2e_resource_service.rs to import and use the
shared utilities from common::* instead of defining local copies of
wait_until_ready, raw_conf_version, base_upstream, and base_service. Remove
those four local definitions while preserving all existing call sites and
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 995729f0-f575-4e44-84ad-add3e41ccf48
📒 Files selected for processing (13)
rust/crates/adc-backend-apisix-standalone/src/backend.rsrust/crates/adc-backend-apisix-standalone/src/cache.rsrust/crates/adc-backend-apisix-standalone/src/operator.rsrust/crates/adc-backend-apisix-standalone/src/transformer.rsrust/crates/adc-backend-apisix-standalone/src/typing.rsrust/crates/adc-backend-apisix-standalone/tests/common/mod.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_cache.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_consumer.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_global_rule.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_inline_upstream.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_upstream.rsrust/crates/adc-backend-apisix-standalone/tests/e2e_validate.rs
🚧 Files skipped from review as they are similar to previous changes (11)
- rust/crates/adc-backend-apisix-standalone/tests/e2e_validate.rs
- rust/crates/adc-backend-apisix-standalone/tests/e2e_cache.rs
- rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_inline_upstream.rs
- rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service.rs
- rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_consumer.rs
- rust/crates/adc-backend-apisix-standalone/src/cache.rs
- rust/crates/adc-backend-apisix-standalone/src/backend.rs
- rust/crates/adc-backend-apisix-standalone/src/typing.rs
- rust/crates/adc-backend-apisix-standalone/src/transformer.rs
- rust/crates/adc-backend-apisix-standalone/tests/e2e_resource_service_upstream.rs
- rust/crates/adc-backend-apisix-standalone/src/operator.rs
1 participant
Description
Backend for apisix standalone.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests