diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 514c9987..629457c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,6 +94,16 @@ jobs: env: ETCD_LISTEN_CLIENT_URLS: http://0.0.0.0:2379 ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379 + # Auth-enabled etcd for token-refresh regression tests (separate + # container with short TTL so the tests don't wait 5+ minutes). + # Port 2380 to avoid colliding with the main etcd above. + etcd-auth: + image: quay.io/coreos/etcd:v3.5.18 + ports: ["2380:2379"] + env: + ETCD_LISTEN_CLIENT_URLS: http://0.0.0.0:2379 + ETCD_ADVERTISE_CLIENT_URLS: http://0.0.0.0:2379 + ETCD_AUTH_TOKEN_TTL: "5" env: # Picked up by crates/aisix-cache/tests/redis_integration.rs. # The tests no-op when this is unset (local dev), so absence is safe. @@ -129,6 +139,20 @@ jobs: ADMIN_TEST_ETCD_URL: http://127.0.0.1:2379 # Picked up by crates/aisix-etcd/tests/watch_integration.rs. ETCD_TEST_URL: http://127.0.0.1:2379 + # Auth-token-refresh regression tests below, against the + # `etcd-auth` service (short --auth-token-ttl). User/password + # created by the "Enable auth on etcd-auth" step. The tests + # themselves pass a 2s refresh interval directly to + # EtcdConfigProvider::connect/EtcdConfigStore::new — well under + # the 5s TTL, with a 3s margin absorbing scheduler/RPC jitter on + # a busy CI runner so the refresh tick reliably lands before the + # original token expires. + ETCD_TEST_AUTH_URL: http://127.0.0.1:2380 + ETCD_TEST_USER: citest + ETCD_TEST_PASSWORD: citestpw + ADMIN_TEST_ETCD_AUTH_URL: http://127.0.0.1:2380 + ADMIN_TEST_ETCD_USER: citest + ADMIN_TEST_ETCD_PASSWORD: citestpw steps: - uses: actions/checkout@v6 - uses: dtolnay/rust-toolchain@stable @@ -186,6 +210,27 @@ jobs: sleep 2 docker exec redis-fo-sentinel redis-cli -p 26380 sentinel get-master-addr-by-name myfo | tr -d '\r' | tr '\n' ' ' | grep -qE '127\.0\.0\.1 +6381' \ || { echo "::error::Failover sentinel 26380 did not resolve myfo to the master"; exit 1; } + - name: Enable auth on etcd-auth + # The etcd-auth service (port 2380) has --auth-token-ttl=5 but + # ships with no users. etcd requires a user named "root" to exist + # before `auth enable` (hardcoded precondition). Create root user, + # then the test user, grant both the root role, then enable auth. + # The token-refresh tests need this to exercise the "write past TTL" + # regression scenario (production incident: "etcdserver: invalid + # auth token"). The image's default entrypoint is `etcd` itself, + # so `etcdctl` must be invoked via an explicit --entrypoint override. + run: | + docker run --rm --network host --entrypoint etcdctl \ + quay.io/coreos/etcd:v3.5.18 --endpoints=http://127.0.0.1:2380 user add root:rootpw + docker run --rm --network host --entrypoint etcdctl \ + quay.io/coreos/etcd:v3.5.18 --endpoints=http://127.0.0.1:2380 user grant-role root root + docker run --rm --network host --entrypoint etcdctl \ + quay.io/coreos/etcd:v3.5.18 --endpoints=http://127.0.0.1:2380 user add citest:citestpw + docker run --rm --network host --entrypoint etcdctl \ + quay.io/coreos/etcd:v3.5.18 --endpoints=http://127.0.0.1:2380 user grant-role citest root + docker run --rm --network host --entrypoint etcdctl \ + quay.io/coreos/etcd:v3.5.18 --endpoints=http://127.0.0.1:2380 auth enable + echo "etcd-auth (port 2380) now requires authentication; root and citest users both have root role" - name: unit tests with coverage run: cargo llvm-cov --workspace --all-features --lcov --output-path lcov-unit.info - uses: actions/upload-artifact@v7 diff --git a/Cargo.lock b/Cargo.lock index 1101c01c..34923865 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,7 +37,7 @@ version = "0.3.0" dependencies = [ "aisix-core", "async-trait", - "axum", + "axum 0.7.9", "futures", "http 1.4.0", "reqwest 0.12.28", @@ -59,7 +59,7 @@ dependencies = [ "aisix-provider-openai", "aisix-proxy", "async-trait", - "axum", + "axum 0.7.9", "chrono", "dashmap", "etcd-client", @@ -69,7 +69,7 @@ dependencies = [ "testcontainers", "thiserror 1.0.69", "tokio", - "tower 0.5.3", + "tower", "tracing", "uuid", "wiremock", @@ -180,7 +180,7 @@ version = "0.3.0" dependencies = [ "aisix-core", "async-trait", - "axum", + "axum 0.7.9", "futures", "hex", "http 1.4.0", @@ -343,7 +343,7 @@ dependencies = [ "aisix-ratelimit", "async-stream", "async-trait", - "axum", + "axum 0.7.9", "base64 0.22.1", "bytes", "chrono", @@ -359,7 +359,7 @@ dependencies = [ "thiserror 1.0.69", "tokio", "tokio-tungstenite", - "tower 0.5.3", + "tower", "tower-http", "tracing", "uuid", @@ -411,7 +411,7 @@ dependencies = [ "aisix-proxy", "aisix-ratelimit", "anyhow", - "axum", + "axum 0.7.9", "axum-server", "clap", "config", @@ -437,7 +437,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bdcae66c6be087b827ddb61938cd752305f0ee15e75deb4ad98bf6b2fb69a067" dependencies = [ "getset", - "prost-build", + "prost-build 0.13.5", "quick-protobuf", "thiserror 2.0.18", ] @@ -892,7 +892,7 @@ dependencies = [ "rustls-pki-types", "tokio", "tokio-rustls 0.26.4", - "tower 0.5.3", + "tower", "tracing", ] @@ -1034,7 +1034,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edca88bc138befd0323b20752846e6587272d3b03b0343c8ea28a6f819e6e71f" dependencies = [ "async-trait", - "axum-core", + "axum-core 0.4.5", "axum-macros", "base64 0.22.1", "bytes", @@ -1045,7 +1045,7 @@ dependencies = [ "hyper 1.9.0", "hyper-util", "itoa", - "matchit", + "matchit 0.7.3", "memchr", "mime", "multer", @@ -1060,12 +1060,37 @@ dependencies = [ "sync_wrapper", "tokio", "tokio-tungstenite", - "tower 0.5.3", + "tower", "tower-layer", "tower-service", "tracing", ] +[[package]] +name = "axum" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "31b698c5f9a010f6573133b09e0de5408834d0c82f8d7475a89fc1867a71cd90" +dependencies = [ + "axum-core 0.5.6", + "bytes", + "futures-util", + "http 1.4.0", + "http-body 1.0.1", + "http-body-util", + "itoa", + "matchit 0.8.4", + "memchr", + "mime", + "percent-encoding", + "pin-project-lite", + "serde_core", + "sync_wrapper", + "tower", + "tower-layer", + "tower-service", +] + [[package]] name = "axum-core" version = "0.4.5" @@ -1087,6 +1112,24 @@ dependencies = [ "tracing", ] +[[package]] +name = "axum-core" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08c78f31d7b1291f7ee735c1c6780ccde7785daae9a9206026862dab7d8792d1" +dependencies = [ + "bytes", + "futures-core", + "http 1.4.0", + "http-body 1.0.1", + "http-body-util", + "mime", + "pin-project-lite", + "sync_wrapper", + "tower-layer", + "tower-service", +] + [[package]] name = "axum-macros" version = "0.4.2" @@ -1752,17 +1795,19 @@ dependencies = [ [[package]] name = "etcd-client" -version = "0.14.1" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc0452bcc559431b16f472b7ab86e2f9ccd5f3c2da3795afbd6b773665e047fe" +checksum = "ef5da6e9a6ae89f4a91f80ba1caae45a5a924397a19947e18f5121a43285e9bc" dependencies = [ "http 1.4.0", - "prost", + "prost 0.14.4", "tokio", "tokio-stream", "tonic", "tonic-build", - "tower 0.4.13", + "tonic-prost", + "tonic-prost-build", + "tower", "tower-service", ] @@ -2833,6 +2878,12 @@ version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0e7465ac9959cc2b1404e8e2367b43684a6d13790fe23056cc8c6c5a6b7bcb94" +[[package]] +name = "matchit" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47e1ffaa40ddd1f3ed91f717a33c8c0ee23fff369e3aa8772b9605cc1d22f4c3" + [[package]] name = "md-5" version = "0.10.6" @@ -3266,6 +3317,17 @@ dependencies = [ "indexmap 2.14.0", ] +[[package]] +name = "petgraph" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8701b58ea97060d5e5b155d383a69952a60943f0e6dfe30b04c287beb0b27455" +dependencies = [ + "fixedbitset", + "hashbrown 0.15.5", + "indexmap 2.14.0", +] + [[package]] name = "pin-project" version = "1.1.11" @@ -3382,7 +3444,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2796faa41db3ec313a31f7624d9286acf277b52de526150b7e69f3debf891ee5" dependencies = [ "bytes", - "prost-derive", + "prost-derive 0.13.5", +] + +[[package]] +name = "prost" +version = "0.14.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "528ac67416ff8646872a3c02cad9cc4ee5dc9f9540c9b10771855c95cb2e5ae1" +dependencies = [ + "bytes", + "prost-derive 0.14.4", ] [[package]] @@ -3396,10 +3468,31 @@ dependencies = [ "log", "multimap", "once_cell", - "petgraph", + "petgraph 0.7.1", + "prettyplease", + "prost 0.13.5", + "prost-types 0.13.5", + "regex", + "syn", + "tempfile", +] + +[[package]] +name = "prost-build" +version = "0.14.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03da047801ff44bb6a4d407d4860c05fd70bb81714e6b2f3812603d5b145b042" +dependencies = [ + "heck", + "itertools 0.14.0", + "log", + "multimap", + "petgraph 0.8.3", "prettyplease", - "prost", - "prost-types", + "prost 0.14.4", + "prost-types 0.14.4", + "pulldown-cmark", + "pulldown-cmark-to-cmark", "regex", "syn", "tempfile", @@ -3418,13 +3511,55 @@ dependencies = [ "syn", ] +[[package]] +name = "prost-derive" +version = "0.14.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b570b25f7617e43d59005d0990ccb79e950a423952cea19671b7a876da390adf" +dependencies = [ + "anyhow", + "itertools 0.14.0", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "prost-types" version = "0.13.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "52c2c1bf36ddb1a1c396b3601a3cec27c2462e45f07c386894ec3ccf5332bd16" dependencies = [ - "prost", + "prost 0.13.5", +] + +[[package]] +name = "prost-types" +version = "0.14.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f94967dc7688f3054c7fac87473ffae4cc4c3904800e2d9f5b857246d8963b0a" +dependencies = [ + "prost 0.14.4", +] + +[[package]] +name = "pulldown-cmark" +version = "0.13.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e" +dependencies = [ + "bitflags 2.11.1", + "memchr", + "unicase", +] + +[[package]] +name = "pulldown-cmark-to-cmark" +version = "22.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50793def1b900256624a709439404384204a5dc3a6ec580281bfaac35e882e90" +dependencies = [ + "pulldown-cmark", ] [[package]] @@ -3803,7 +3938,7 @@ dependencies = [ "tokio", "tokio-rustls 0.26.4", "tokio-util", - "tower 0.5.3", + "tower", "tower-http", "tower-service", "url", @@ -3844,7 +3979,7 @@ dependencies = [ "tokio", "tokio-rustls 0.26.4", "tokio-util", - "tower 0.5.3", + "tower", "tower-http", "tower-service", "url", @@ -4013,7 +4148,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4842,13 +4977,12 @@ checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" [[package]] name = "tonic" -version = "0.12.3" +version = "0.14.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "877c5b330756d856ffcc4553ab34a5684481ade925ecc54bcd1bf02b1d0d4d52" +checksum = "ac2a5518c70fa84342385732db33fb3f44bc4cc748936eb5833d2df34d6445ef" dependencies = [ - "async-stream", "async-trait", - "axum", + "axum 0.8.9", "base64 0.22.1", "bytes", "h2 0.4.13", @@ -4860,13 +4994,12 @@ dependencies = [ "hyper-util", "percent-encoding", "pin-project", - "prost", - "rustls-pemfile", - "socket2 0.5.10", + "socket2 0.6.3", + "sync_wrapper", "tokio", "tokio-rustls 0.26.4", "tokio-stream", - "tower 0.4.13", + "tower", "tower-layer", "tower-service", "tracing", @@ -4874,36 +5007,41 @@ dependencies = [ [[package]] name = "tonic-build" -version = "0.12.3" +version = "0.14.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9557ce109ea773b399c9b9e5dca39294110b74f1f342cb347a80d1fce8c26a11" +checksum = "c68f61875ac5293cf72e6c8cf0158086428c82c37229e98c840878f1706b0322" dependencies = [ "prettyplease", "proc-macro2", - "prost-build", - "prost-types", "quote", "syn", ] [[package]] -name = "tower" -version = "0.4.13" +name = "tonic-prost" +version = "0.14.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8fa9be0de6cf49e536ce1851f987bd21a43b771b09473c3549a6c853db37c1c" +checksum = "50849f68853be452acf590cde0b146665b8d507b3b8af17261df47e02c209ea0" dependencies = [ - "futures-core", - "futures-util", - "indexmap 1.9.3", - "pin-project", - "pin-project-lite", - "rand 0.8.5", - "slab", - "tokio", - "tokio-util", - "tower-layer", - "tower-service", - "tracing", + "bytes", + "prost 0.14.4", + "tonic", +] + +[[package]] +name = "tonic-prost-build" +version = "0.14.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "654e5643eff75d7f8c99197ce1440ed19a3474eada74c12bbac488b2cafdae27" +dependencies = [ + "prettyplease", + "proc-macro2", + "prost-build 0.14.4", + "prost-types 0.14.4", + "quote", + "syn", + "tempfile", + "tonic-build", ] [[package]] @@ -4914,9 +5052,12 @@ checksum = "ebe5ef63511595f1344e2d5cfa636d973292adc0eec1f0ad45fae9f0851ab1d4" dependencies = [ "futures-core", "futures-util", + "indexmap 2.14.0", "pin-project-lite", + "slab", "sync_wrapper", "tokio", + "tokio-util", "tower-layer", "tower-service", "tracing", @@ -4940,7 +5081,7 @@ dependencies = [ "pin-project-lite", "tokio", "tokio-util", - "tower 0.5.3", + "tower", "tower-layer", "tower-service", "tracing", @@ -5401,7 +5542,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4cd75358..f565b5a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,7 +73,7 @@ once_cell = "1.20" parking_lot = "0.12" # etcd -etcd-client = { version = "0.14", features = ["tls"] } +etcd-client = { version = "0.19", features = ["tls"] } # Config config = { version = "0.14", default-features = false, features = ["yaml", "toml", "json"] } diff --git a/config.example.yaml b/config.example.yaml index 6d1bc331..daec1084 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -7,6 +7,7 @@ etcd: prefix: "/aisix" # user: "aisix" # password_env: "AISIX_ETCD_PASSWORD" + # auth_token_refresh_secs: 240 dial_timeout_ms: 5000 request_timeout_ms: 5000 # Optional TLS / mTLS bundle. Required when connecting to an diff --git a/config.managed.yaml b/config.managed.yaml index 775b17de..30edb3b3 100644 --- a/config.managed.yaml +++ b/config.managed.yaml @@ -36,6 +36,7 @@ etcd: endpoints: - "https://placeholder-overridden-at-register:2379" prefix: "/aisix" + # auth_token_refresh_secs: 240 dial_timeout_ms: 5000 request_timeout_ms: 5000 diff --git a/crates/aisix-admin/src/etcd_store.rs b/crates/aisix-admin/src/etcd_store.rs index 1d01363d..3f4083b2 100644 --- a/crates/aisix-admin/src/etcd_store.rs +++ b/crates/aisix-admin/src/etcd_store.rs @@ -54,8 +54,12 @@ impl std::fmt::Debug for EtcdConfigStore { } impl EtcdConfigStore { - pub fn new(client: Client, prefix: impl Into) -> Self { + /// `refresh_secs` sets the auth-token refresh interval (typically + /// `EtcdConfig.auth_token_refresh_secs`); `None` or `0` uses the + /// hardcoded default — see `aisix_etcd::start_token_refresh_task`. + pub fn new(client: Client, prefix: impl Into, refresh_secs: Option) -> Self { let prefix = prefix.into().trim_end_matches('/').to_string(); + aisix_etcd::start_token_refresh_task(client.clone(), refresh_secs); Self { client: Mutex::new(client), prefix, @@ -411,7 +415,7 @@ mod tests { .unwrap() .block_on(client_fut) .expect("lazy connect never fails synchronously"); - EtcdConfigStore::new(client, "/aisix") + EtcdConfigStore::new(client, "/aisix", None) } #[test] @@ -451,7 +455,7 @@ mod tests { None, )) .expect("lazy connect never fails synchronously"); - let store = EtcdConfigStore::new(client, "/aisix/"); + let store = EtcdConfigStore::new(client, "/aisix/", None); assert_eq!(store.prefix(), "/aisix"); assert_eq!(store.key_for("models", "a"), "/aisix/models/a"); } @@ -481,7 +485,7 @@ mod tests { let client = etcd_client::Client::connect([endpoint], None) .await .expect("etcd client"); - let store = EtcdConfigStore::new(client, "/aisix-it"); + let store = EtcdConfigStore::new(client, "/aisix-it", None); let model: Model = serde_json::from_str( r#"{ diff --git a/crates/aisix-admin/tests/etcd_integration.rs b/crates/aisix-admin/tests/etcd_integration.rs index 027ab443..8a7438b4 100644 --- a/crates/aisix-admin/tests/etcd_integration.rs +++ b/crates/aisix-admin/tests/etcd_integration.rs @@ -37,6 +37,11 @@ fn etcd_url() -> Option { std::env::var("ADMIN_TEST_ETCD_URL").ok() } +/// Separate from `ADMIN_TEST_ETCD_URL`: CI's etcd container has no auth. +fn auth_etcd_url() -> Option { + std::env::var("ADMIN_TEST_ETCD_AUTH_URL").ok() +} + /// Per-test prefix so concurrent tests in this binary don't collide. fn unique_prefix() -> String { let nanos = SystemTime::now() @@ -54,7 +59,7 @@ async fn build_state_with_real_etcd(url: &str, prefix: &str) -> AdminState { let client = etcd_client::Client::connect([url], None) .await .expect("etcd connect"); - let store: Arc = Arc::new(EtcdConfigStore::new(client, prefix)); + let store: Arc = Arc::new(EtcdConfigStore::new(client, prefix, None)); let handle = SnapshotHandle::new(AisixSnapshot::new()); let cfg = AdminConfig { addr: "127.0.0.1:0".into(), @@ -403,3 +408,79 @@ async fn loader_picks_up_every_admin_write() { assert_eq!(snap.observability_exporters.len(), 1); assert_eq!(snap.mcp_servers.len(), 1); } + +// ─────────────────────────── Auth token refresh ─────────────────────────── +// +// Regression test for a production incident: Admin API calls made long +// after boot failed with "etcdserver: invalid auth token". Root cause: +// `EtcdConfigStore`'s write-path client authenticated once at connect +// time and never refreshed, so any write issued past +// `--auth-token-ttl` (default 300s) hit UNAUTHENTICATED. +// `EtcdConfigStore::new` now spawns `aisix_etcd::start_token_refresh_task` +// to fix this. +// +// Requires `ADMIN_TEST_ETCD_AUTH_URL` (auth-enabled etcd, short TTL) + +// `ADMIN_TEST_ETCD_USER` / `ADMIN_TEST_ETCD_PASSWORD`; no-ops otherwise. +// +// Local run: +// docker run --rm -p 2379:2379 quay.io/coreos/etcd:v3.5.18 \ +// --listen-client-urls=http://0.0.0.0:2379 \ +// --advertise-client-urls=http://0.0.0.0:2379 --auth-token-ttl=5 +// etcdctl user add root:rootpw && etcdctl user grant-role root root \ +// && etcdctl user add test:testpw && etcdctl user grant-role test root \ +// && etcdctl auth enable +// ADMIN_TEST_ETCD_AUTH_URL=http://127.0.0.1:2379 ADMIN_TEST_ETCD_USER=test \ +// ADMIN_TEST_ETCD_PASSWORD=testpw \ +// cargo test -p aisix-admin --test etcd_integration \ +// admin_write_survives_token_expiry -- --nocapture +#[tokio::test] +async fn admin_write_survives_token_expiry() { + let Some(url) = auth_etcd_url() else { + eprintln!("skipping: ADMIN_TEST_ETCD_AUTH_URL not set"); + return; + }; + let user = std::env::var("ADMIN_TEST_ETCD_USER").expect("ADMIN_TEST_ETCD_USER required"); + let password = + std::env::var("ADMIN_TEST_ETCD_PASSWORD").expect("ADMIN_TEST_ETCD_PASSWORD required"); + + let prefix = unique_prefix(); + let options = etcd_client::ConnectOptions::new().with_user(user, password); + let client = etcd_client::Client::connect([url.as_str()], Some(options)) + .await + .expect("etcd connect with auth"); + // Mirrors main.rs's production bootstrap call, but with a 2s refresh + // interval — well under the 5s --auth-token-ttl the local-run etcd is + // started with above, so the loop refreshes before expiry. + let store: Arc = Arc::new(EtcdConfigStore::new(client, prefix, Some(2))); + let handle = SnapshotHandle::new(AisixSnapshot::new()); + let cfg = AdminConfig { + addr: "127.0.0.1:0".into(), + admin_keys: vec![ADMIN_KEY.into()], + tls: None, + }; + let state = AdminState::new(handle, store, &cfg); + + // Wait past the 5s TTL window (1s margin absorbs scheduler jitter); + // without refresh this POST would 401. + tokio::time::sleep(std::time::Duration::from_secs(6)).await; + + let app = build_router(state); + let resp = app + .oneshot(auth_post( + "/admin/v1/models", + json!({ + "display_name": "post-expiry", + "provider": "openai", + "model_name": "gpt-4o", + "provider_key_id": "11111111-1111-1111-1111-111111111111" + }), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "admin write after token-TTL window should succeed; a non-OK \ + status here means the auth-token refresh loop isn't running", + ); +} diff --git a/crates/aisix-core/src/config.rs b/crates/aisix-core/src/config.rs index fd603d92..613412ad 100644 --- a/crates/aisix-core/src/config.rs +++ b/crates/aisix-core/src/config.rs @@ -113,6 +113,12 @@ pub struct EtcdConfig { pub dial_timeout_ms: u64, #[serde(default = "EtcdConfig::default_request_timeout")] pub request_timeout_ms: u64, + /// How often the auth-token refresh loop calls `Client::refresh_token` + /// (only relevant when `user` is set). Must stay well under the + /// etcd cluster's `--auth-token-ttl`, or a write between refreshes + /// can hit "etcdserver: invalid auth token". + #[serde(default = "EtcdConfig::default_auth_refresh_secs")] + pub auth_token_refresh_secs: u64, /// Optional TLS / mTLS bundle used to authenticate to the etcd /// endpoint. Required when talking to an aisix.cloud DP Manager /// (see prd-09 §9.3.3 — the CP issues a 10-year client cert via @@ -313,6 +319,7 @@ impl Default for EtcdConfig { password_env: None, dial_timeout_ms: Self::default_dial_timeout(), request_timeout_ms: Self::default_request_timeout(), + auth_token_refresh_secs: Self::default_auth_refresh_secs(), tls: None, } } @@ -328,6 +335,9 @@ impl EtcdConfig { const fn default_request_timeout() -> u64 { 5_000 } + const fn default_auth_refresh_secs() -> u64 { + 240 + } pub const fn dial_timeout(&self) -> Duration { Duration::from_millis(self.dial_timeout_ms) @@ -902,6 +912,7 @@ admin: ); let cfg = Config::load_from_path(Some(f.path())).unwrap(); assert_eq!(cfg.etcd.endpoints, vec!["http://127.0.0.1:2379"]); + assert_eq!(cfg.etcd.auth_token_refresh_secs, 240); assert_eq!(cfg.proxy.request_body_limit_bytes, 10 * 1024 * 1024); assert!(cfg.observability.metrics.prometheus.enabled); // The dedicated metrics listener defaults to 0.0.0.0:9090 in @@ -942,6 +953,25 @@ managed: assert_eq!(cfg.managed.heartbeat_interval_secs, 5); } + #[test] + fn etcd_auth_token_refresh_secs_can_be_overridden() { + let f = write_yaml( + r#" +etcd: + endpoints: ["http://127.0.0.1:2379"] + prefix: "/aisix" + auth_token_refresh_secs: 60 +proxy: + addr: "0.0.0.0:3000" +admin: + addr: "127.0.0.1:3001" + admin_keys: ["k1"] +"#, + ); + let cfg = Config::load_from_path(Some(f.path())).unwrap(); + assert_eq!(cfg.etcd.auth_token_refresh_secs, 60); + } + #[test] fn loads_real_ip_block() { let f = write_yaml( diff --git a/crates/aisix-etcd/src/etcd_provider.rs b/crates/aisix-etcd/src/etcd_provider.rs index 30654c85..e96a22f8 100644 --- a/crates/aisix-etcd/src/etcd_provider.rs +++ b/crates/aisix-etcd/src/etcd_provider.rs @@ -77,23 +77,105 @@ impl std::fmt::Debug for EtcdConfigProvider { } } +/// etcd's default `--auth-token-ttl` is 300s; 240s leaves a 60s margin. +/// Override via `EtcdConfig.auth_token_refresh_secs` for a shorter TTL. +const DEFAULT_TOKEN_REFRESH_SECS: u64 = 240; + +/// `config_secs` is `EtcdConfig.auth_token_refresh_secs`; `None` or `0` +/// (an operator-supplied 0, or the pre-migration config default) falls +/// back to the hardcoded default. +fn resolve_refresh_secs(config_secs: Option) -> u64 { + config_secs + .filter(|&secs| secs > 0) + .unwrap_or(DEFAULT_TOKEN_REFRESH_SECS) +} + +/// Spawn a background task that refreshes the etcd auth token at the +/// given interval. Safe to call unconditionally: `Client::refresh_token` +/// (v0.18+) no-ops when no credentials were configured. +/// +/// `config_secs` (typically `EtcdConfig.auth_token_refresh_secs`) sets the +/// interval; `None` or `0` falls back to the hardcoded default (240s). +/// `Client` is `Clone`-cheap (internally `Arc`'d) and shares its +/// auth-token cell across clones, so this works on any clone of the +/// caller's client. +/// +/// No-ops outside a Tokio runtime (e.g. a sync unit test building a +/// `Client`/store via `Runtime::block_on` and returning it) — `tokio::spawn` +/// requires an active runtime, and such contexts never carry credentials. +pub fn start_token_refresh_task(client: Client, config_secs: Option) { + if tokio::runtime::Handle::try_current().is_err() { + return; + } + + if config_secs == Some(0) { + tracing::warn!( + interval_secs = DEFAULT_TOKEN_REFRESH_SECS, + "etcd.auth_token_refresh_secs = 0 is not a valid interval — falling back to the default", + ); + } + + let refresh_interval = Duration::from_secs(resolve_refresh_secs(config_secs)); + + tracing::info!( + interval_secs = refresh_interval.as_secs(), + "etcd auth token refresh loop started", + ); + tokio::spawn(async move { + let mut interval = tokio::time::interval(refresh_interval); + // First tick fires immediately — skip it; `Client::connect` + // already authenticated once. + interval.tick().await; + loop { + interval.tick().await; + match client.refresh_token().await { + Ok(()) => { + tracing::debug!("etcd auth token refreshed"); + } + Err(e) => { + tracing::warn!( + error = %format_error_chain(&e), + "etcd auth token refresh failed — watch may see \ + UNAUTHENTICATED on next expiry", + ); + } + } + } + }); +} + impl EtcdConfigProvider { - /// Connect with the spec §2 default retry policy. + /// Connect with the spec §2 default retry policy. `refresh_secs` + /// overrides the auth-token refresh interval (typically + /// `EtcdConfig.auth_token_refresh_secs`); `None` uses the default — + /// see `start_token_refresh_task`. pub async fn connect( endpoints: &[String], prefix: impl Into, options: Option, + refresh_secs: Option, ) -> Result { - Self::connect_with_policy(endpoints, prefix, options, ConnectPolicy::default()).await + Self::connect_with_policy( + endpoints, + prefix, + options, + ConnectPolicy::default(), + refresh_secs, + ) + .await } - /// Connect with a caller-chosen retry policy. Returns the last-seen - /// error on failure to surface useful context in the bootstrap logs. + /// Connect with a caller-chosen retry policy and auth-token refresh + /// interval override (`refresh_secs`, typically + /// `EtcdConfig.auth_token_refresh_secs` — `None` uses the default). + /// Returns the last-seen error on failure to surface useful context + /// in the bootstrap logs. pub async fn connect_with_policy( endpoints: &[String], prefix: impl Into, options: Option, policy: ConnectPolicy, + refresh_secs: Option, ) -> Result { let prefix = prefix.into(); let mut last_err: Option = None; @@ -101,6 +183,7 @@ impl EtcdConfigProvider { match Client::connect(endpoints, options.clone()).await { Ok(client) => { tracing::info!(attempt, prefix = %prefix, "etcd connected"); + start_token_refresh_task(client.clone(), refresh_secs); return Ok(Self { client: Mutex::new(client), prefix, @@ -171,30 +254,27 @@ impl ConfigProvider for EtcdConfigProvider { let opts = WatchOptions::new() .with_prefix() .with_start_revision(start_revision); - let (watcher, stream) = client + let stream = client .watch(self.prefix.as_bytes(), Some(opts)) .await .map_err(|e| ProviderError::Watch(format_error_chain(&e)))?; Ok(Box::new(EtcdWatchStream { inner: stream, - _watcher: watcher, buf: VecDeque::new(), })) } } /// Adapter from `etcd-client`'s WatchStream to our typed [`WatchEvent`]. +/// A `VecDeque` buffer drains multi-event responses across successive +/// `poll_next` calls so no events are silently dropped. /// -/// Each `WatchResponse` carries a batch of events; we flatten them -/// into individual `WatchEvent` items. A `VecDeque` buffer drains -/// multi-event responses across successive `poll_next` calls so that -/// no events are silently dropped. +/// Pre-0.18 `etcd-client` required a separate `Watcher` handle alongside +/// the stream (dropping it killed the watch — issue #237). Since 0.18 +/// `WatchStream` owns both halves, so `inner` alone keeps it alive. pub struct EtcdWatchStream { inner: etcd_client::WatchStream, - // Must outlive `inner` — dropping the Watcher closes the client→server - // half of the gRPC stream, causing the server to tear down the watch. - _watcher: etcd_client::Watcher, buf: VecDeque, } @@ -288,7 +368,7 @@ mod tests { attempts: 1, }; let endpoints: Vec = vec![]; - let err = EtcdConfigProvider::connect_with_policy(&endpoints, "/aisix", None, policy) + let err = EtcdConfigProvider::connect_with_policy(&endpoints, "/aisix", None, policy, None) .await .unwrap_err(); assert!(matches!(err, ProviderError::Connect(_))); @@ -329,4 +409,19 @@ mod tests { let err = std::io::Error::other("bare"); assert_eq!(format_error_chain(&err), "bare"); } + + #[test] + fn resolve_refresh_secs_defaults_when_unset() { + assert_eq!(resolve_refresh_secs(None), DEFAULT_TOKEN_REFRESH_SECS); + } + + #[test] + fn resolve_refresh_secs_honors_config_value() { + assert_eq!(resolve_refresh_secs(Some(90)), 90); + } + + #[test] + fn resolve_refresh_secs_falls_back_on_zero() { + assert_eq!(resolve_refresh_secs(Some(0)), DEFAULT_TOKEN_REFRESH_SECS); + } } diff --git a/crates/aisix-etcd/src/lib.rs b/crates/aisix-etcd/src/lib.rs index 64461de3..bf6aed35 100644 --- a/crates/aisix-etcd/src/lib.rs +++ b/crates/aisix-etcd/src/lib.rs @@ -29,8 +29,8 @@ pub mod supervisor; pub use backoff::{ExpBackoff, BASE_MS, MAX_MS}; pub use etcd_provider::{ - ConnectPolicy, EtcdConfigProvider, EtcdWatchStream, CONNECT_MAX_ATTEMPTS, - CONNECT_RETRY_INTERVAL, + start_token_refresh_task, ConnectPolicy, EtcdConfigProvider, EtcdWatchStream, + CONNECT_MAX_ATTEMPTS, CONNECT_RETRY_INTERVAL, }; pub use key::{parse as parse_key, KeyError, ResourceKey}; pub use loader::{build_snapshot, BuildStats}; diff --git a/crates/aisix-etcd/tests/watch_integration.rs b/crates/aisix-etcd/tests/watch_integration.rs index b5569eda..9050dc5a 100644 --- a/crates/aisix-etcd/tests/watch_integration.rs +++ b/crates/aisix-etcd/tests/watch_integration.rs @@ -2,12 +2,16 @@ //! //! Requires a real etcd — gated by `ETCD_TEST_URL`. CI sets this via //! the etcd service container; local runs without the var are no-ops. +//! +//! For auth-token-refresh tests, set `ETCD_TEST_AUTH_URL` pointing to an +//! etcd with `--auth-token-ttl` set short (e.g. 5s for test speed) and +//! `ETCD_TEST_USER` / `ETCD_TEST_PASSWORD` for a valid user. use std::time::{Duration, SystemTime, UNIX_EPOCH}; use aisix_etcd::provider::ConfigProvider; use aisix_etcd::EtcdConfigProvider; -use etcd_client::Client; +use etcd_client::{Client, ConnectOptions}; use futures::StreamExt; use tokio::time::timeout; @@ -48,7 +52,7 @@ async fn watch_stream_delivers_events_after_put() { let prefix = unique_prefix(); let endpoints = vec![url.clone()]; - let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), None) + let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), None, None) .await .expect("connect"); @@ -92,6 +96,54 @@ async fn watch_stream_delivers_events_after_put() { .expect("cleanup delete"); } +/// Regression test: the token refresh loop must keep a `user`/`password` +/// client authenticated past etcd's `--auth-token-ttl` window. +/// +/// Requires `ETCD_TEST_AUTH_URL` (auth-enabled etcd) + `ETCD_TEST_USER` / +/// `ETCD_TEST_PASSWORD`; no-ops otherwise (CI's etcd has no auth). +/// +/// Local run: +/// docker run --rm -p 2379:2379 quay.io/coreos/etcd:v3.5.18 \ +/// --listen-client-urls=http://0.0.0.0:2379 \ +/// --advertise-client-urls=http://0.0.0.0:2379 --auth-token-ttl=5 +/// etcdctl user add root:rootpw && etcdctl user grant-role root root \ +/// && etcdctl user add test:testpw && etcdctl user grant-role test root \ +/// && etcdctl auth enable +/// ETCD_TEST_AUTH_URL=http://127.0.0.1:2379 ETCD_TEST_USER=test \ +/// ETCD_TEST_PASSWORD=testpw \ +/// cargo test --test watch_integration token_refresh -- --nocapture +#[tokio::test] +async fn token_refresh_prevents_auth_expiry() { + let auth_url = match std::env::var("ETCD_TEST_AUTH_URL").ok() { + Some(u) => u, + None => { + eprintln!("ETCD_TEST_AUTH_URL not set — skipping"); + return; + } + }; + let user = std::env::var("ETCD_TEST_USER").expect("ETCD_TEST_USER required"); + let password = std::env::var("ETCD_TEST_PASSWORD").expect("ETCD_TEST_PASSWORD required"); + + let prefix = unique_prefix(); + let endpoints = vec![auth_url]; + let options = ConnectOptions::new().with_user(user, password); + + // Refresh every 2s — well under the 5s --auth-token-ttl the local-run + // etcd is started with above, so the loop refreshes before expiry. + let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), Some(options), Some(2)) + .await + .expect("connect with auth"); + + // Wait past the 5s TTL window (1s margin absorbs scheduler jitter); + // without refresh this load_all would 401. + tokio::time::sleep(Duration::from_secs(6)).await; + let (entries, _rev) = provider + .load_all() + .await + .expect("load_all should succeed after token refresh"); + assert_eq!(entries.len(), 0, "unique prefix should be empty"); +} + /// #519 B.3: the supervisor's applied revision (read by the heartbeat as /// `applied_revision`) must catch up to the header revision returned to a /// writer — for puts AND deletes. This is the exact comparison cp-api @@ -109,7 +161,7 @@ async fn supervisor_applied_revision_catches_up_to_writer_revision() { let prefix = unique_prefix(); let endpoints = vec![url.clone()]; - let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), None) + let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), None, None) .await .expect("connect"); let supervisor = std::sync::Arc::new(aisix_etcd::Supervisor::new( @@ -178,7 +230,7 @@ async fn watch_stream_delivers_all_events_from_batched_response() { let prefix = unique_prefix(); let endpoints = vec![url.clone()]; - let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), None) + let provider = EtcdConfigProvider::connect(&endpoints, prefix.clone(), None, None) .await .expect("connect"); diff --git a/crates/aisix-server/src/main.rs b/crates/aisix-server/src/main.rs index 9724316d..a0470250 100644 --- a/crates/aisix-server/src/main.rs +++ b/crates/aisix-server/src/main.rs @@ -463,6 +463,7 @@ async fn run(mut cfg: Config) -> anyhow::Result<()> { &cfg.etcd.endpoints, etcd_prefix.clone(), connect_options.clone(), + Some(cfg.etcd.auth_token_refresh_secs), ) .await .map_err(|e| anyhow::anyhow!("etcd connect failed: {e}"))?, @@ -733,7 +734,11 @@ async fn run(mut cfg: Config) -> anyhow::Result<()> { // gets a read-only view of the file-loaded snapshot (writes return // 409 pointing at the file). let admin_store: Option> = match (admin_client, &file_source_path) { - (Some((client, prefix)), _) => Some(Arc::new(EtcdConfigStore::new(client, prefix))), + (Some((client, prefix)), _) => Some(Arc::new(EtcdConfigStore::new( + client, + prefix, + Some(cfg.etcd.auth_token_refresh_secs), + ))), (None, Some(path)) if !cfg.managed.is_managed() => Some(Arc::new(FileManagedStore::new( snapshot_handle.clone(), path.display().to_string(), @@ -1389,6 +1394,7 @@ models: password_env: None, dial_timeout_ms: 5000, request_timeout_ms: 5000, + auth_token_refresh_secs: 240, tls: None, }; let opts = build_etcd_connect_options(&etcd).unwrap(); @@ -1408,6 +1414,7 @@ models: password_env: None, dial_timeout_ms: 5000, request_timeout_ms: 5000, + auth_token_refresh_secs: 240, tls: Some(aisix_core::EtcdTlsConfig { ca_cert_file: "/definitely/does/not/exist/ca.crt".into(), client_cert_file: "/tmp/c.crt".into(),