Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,38 @@ jobs:
if: matrix.suite == 'workspace'
run: bash scripts/check_native_store_dependency_contract.sh

- name: Geo dependency isolation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 Geo steps run in every host-test matrix leg

Every neighbouring step here is guarded with if: matrix.suite == 'workspace', but the nine new Geo steps (lines 189-219) have no guard. Run 42924133045 shows all nine executed in the workspace, cli, axum and app-demo legs, so the dependency script and four distinct Axum feature builds run four times per push.

Add the guard to each step:

      - name: Geo dependency isolation
        if: matrix.suite == 'workspace'
        run: bash scripts/check_geo_dependency_contract.sh

(or matrix.suite == 'axum' if they are meant to live with the Axum leg).

run: bash scripts/check_geo_dependency_contract.sh

- name: Immutable core Geo outcome
run: scripts/run_test_nonzero.sh geo_owned_outcome_survives_public_request_mutation cargo test --offline --locked -p edgezero-core --lib geo_

- name: Native Geo disabled implementation and policy unit contracts
run: |
scripts/run_test_nonzero.sh geo_protected_policy_reuses_network_bounds cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features axum --lib geo::
scripts/run_test_nonzero.sh geo_unavailable_selection_fails_before_runner_resources cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features axum --lib geo_

- name: Native Geo supplied-provider unit contract without built-in
run: scripts/run_test_nonzero.sh source_admission_precedes_header_inspection_and_provider_calls cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features axum --lib source_admission_precedes_header_inspection_and_provider_calls

- name: Native Geo enabled implementation and managed dispatch unit contracts
run: |
scripts/run_test_nonzero.sh geo_cloudfront_parsing_matches_spec cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features geo-cloudfront --lib geo::
scripts/run_test_nonzero.sh geo_managed_resolution_runs_once_from_owned_ingress cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features geo-cloudfront --lib geo_
cargo test --offline --locked -p edgezero-adapter-axum --features geo-cloudfront --lib service::tests

- name: Native Geo disabled built-in hosting (loopback only)
run: scripts/run_test_nonzero.sh geo_hosting_rejects_unavailable_cloudfront cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features axum,test-utils --test production_hosting geo_hosting_rejects_unavailable_cloudfront

- name: Native Geo supplied provider without built-in (loopback only)
run: scripts/run_test_nonzero.sh geo_hosting_supplied_provider_without_builtin cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features axum,test-utils --test production_hosting geo_hosting_supplied_provider_without_builtin

- name: Native Geo real-peer hosting (loopback only)
run: scripts/run_test_nonzero.sh geo_hosting_proves_real_peer_and_shared_outcome cargo test --offline --locked -p edgezero-adapter-axum --no-default-features --features geo-cloudfront,test-utils --test production_hosting geo_hosting_proves_real_peer_and_shared_outcome

- name: Native Geo public examples
run: cargo test --offline --locked -p edgezero-adapter-axum --features geo-cloudfront --doc

- name: Native AWS provider and startup contracts (loopback only)
if: matrix.suite == 'workspace'
run: |
Expand Down
3 changes: 3 additions & 0 deletions crates/edgezero-adapter-axum/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ axum = [
"dep:tokio",
"dep:tower",
"dep:futures-util",
"dep:getrandom",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏ Duplicate feature entries

axum already lists dep:ipnet and dep:getrandom at lines 27-28 (from the base), so these two additions do nothing. Cargo deduplicates them silently, but the diff reads as if Geo added dependencies. Remove lines 22-23.

"dep:ipnet",
"dep:http-body",
"dep:hyper",
"dep:hyper-util",
Expand All @@ -35,6 +37,7 @@ aws-stores = ["aws-appconfig-agent", "aws-secrets-manager"]
aws-dynamodb = ["axum", "edgezero-store-aws/dynamodb-kv"]
celld-config = ["axum", "edgezero-store-celld/config"]
celld-kv = ["axum", "edgezero-store-celld/kv"]
geo-cloudfront = ["axum"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Does the geo-cloudfront feature pay for itself?

It gates geo/cloudfront.rs, about 45 non-test lines with no dependencies; check_geo_dependency_contract.sh proves the package set is identical with it on or off. Geo is already off at runtime until with_geo selects it, so the feature is a second off switch. It adds NativeGeoConfigError::UnavailableCloudFront, the cfg-split tests in dev_server.rs and production_hosting.rs, the cfg_attr on the module, and four of the nine new CI steps.

Design section 7 asks for the built-in to be default-off, and NativeGeoSelection::Off already provides that. Consider compiling the built-in under axum and dropping the feature. Non-blocking, since this is a spec-level choice.

cli = [
"native-bindings",
"dep:edgezero-adapter",
Expand Down
90 changes: 73 additions & 17 deletions crates/edgezero-adapter-axum/src/dev_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ use tokio::time::{Instant as NativeInstant, sleep_until};
use crate::config_store::AxumConfigStore;
use crate::config_store_limits::ConfigStoreLimits;
use crate::connection::{ConnectionExit, serve_http1_with_shutdown};
use crate::geo::NativeGeo;
use crate::ingress_config::AxumIngressConfig;
use crate::key_value_store::PersistentKvStore;
use crate::native_stores::{self, NativeSourcePlan, NativeStoreOverrides, NativeStoreSource};
Expand Down Expand Up @@ -510,9 +511,10 @@ fn serve_local(
)
}

#[derive(Clone, Debug)]
#[derive(Clone)]
struct NativeHosting {
diagnostics: NativeDiagnosticsHandle,
geo: NativeGeo,
policy: TrustedProxyPolicy,
request_records: bool,
}
Expand All @@ -523,6 +525,7 @@ impl Default for NativeHosting {
diagnostics: NativeDiagnosticsHandle::new(),
policy: TrustedProxyPolicy::no_trust(),
request_records: true,
geo: NativeGeo::default(),
}
}
}
Expand Down Expand Up @@ -746,9 +749,15 @@ where
log::warn!(target: BOOT_LOG_TARGET, "application head limits exceed Axum raw ingress limits; the parser rejects first");
}
let session = NativeSession::new(native.diagnostics, native.request_records)?;
let mut service =
AxumServiceState::with_transport(app, transport, json_body_limit, session, native.policy)
.with_phase(reader.clone());
let mut service = AxumServiceState::with_native_transport(
app,
transport,
json_body_limit,
session,
native.policy,
native.geo,
)
.with_phase(reader.clone());
if let Some(registry) = stores.config {
service = service.with_config_registry(registry);
}
Expand Down Expand Up @@ -1109,6 +1118,7 @@ where
policy,
request_records,
diagnostics: NativeDiagnosticsHandle::new(),
geo: NativeGeo::default(),
},
))
}
Expand Down Expand Up @@ -1285,6 +1295,13 @@ where
&'startup PreparedStores,
) -> LocalStartupFuture<'startup>,
{
let geo = options.native_geo()?;
let native = NativeHosting {
diagnostics: options.diagnostics(),
policy: options.trusted_proxy_policy(),
request_records: options.request_records(),
geo,
};
run_local(
ListenerSource::Bind(options.addr()),
signals,
Expand All @@ -1307,17 +1324,7 @@ where
);
failure("initializer", "application", &category)
})?;
Ok((
app,
stores,
transport,
options.json_body_limit(),
NativeHosting {
diagnostics: options.diagnostics(),
policy: options.trusted_proxy_policy(),
request_records: options.request_records(),
},
))
Ok((app, stores, transport, options.json_body_limit(), native))
},
)
}
Expand Down Expand Up @@ -2372,7 +2379,8 @@ mod tests {
OsString::from("EDGEZERO__ADAPTER__JSON_BODY_LIMIT_BYTES"),
binary,
)])
.expect_err("recognized binary setting");
.err()
.expect("recognized binary setting");
assert!(error.to_string().contains("JSON_BODY_LIMIT_BYTES"));
assert_eq!(error.chain().count(), 1);
for (path, category) in [
Expand All @@ -2383,7 +2391,8 @@ mod tests {
OsString::from(format!("EDGEZERO__ADAPTER__{path}")),
OsString::from_vec(b"PRIVATE_SENTINEL\xff".to_vec()),
)])
.expect_err("owned binary limit must fail");
.err()
.expect("owned binary limit must fail");
assert!(owned_error.to_string().contains(category));
assert!(!format!("{owned_error:?}").contains("PRIVATE_SENTINEL"));
assert_eq!(owned_error.chain().count(), 1);
Expand Down Expand Up @@ -4262,6 +4271,53 @@ mod native_store_tests {
Ok(format!("{}:{state}", value.as_str()))
}

#[cfg(not(feature = "geo-cloudfront"))]
#[tokio::test(flavor = "current_thread")]
async fn geo_unavailable_selection_fails_before_runner_resources() {
use crate::geo::{NativeGeoSelection, ProtectedCloudFrontAlb};
let root = tempfile::tempdir().unwrap();
let options = options()
.with_config_dir(root.path())
.unwrap()
.with_data_dir(root.path())
.unwrap()
.with_geo(NativeGeoSelection::CloudFront {
policy: ProtectedCloudFrontAlb::new(["127.0.0.1/32"], "x-origin-verify-fixture")
.unwrap(),
});
let address = options.addr();
let prepared = Rc::new(Cell::new(false));
let initialized = Rc::new(Cell::new(false));
let prepare_observed = Rc::clone(&prepared);
let initialize_observed = Rc::clone(&initialized);
let mut overrides = NativeStoreOverrides::default();
overrides
.prepare_config("first", async move {
prepare_observed.set(true);
Err(NativeStorePreparationError::Unavailable)
})
.unwrap();
// Existing Tokio runtime would reject nested runner construction if validation were late.
let error = run_app_with_native_stores::<CustomApp, _, _>(
options,
overrides,
future::pending::<()>(),
move |_app, _stores| {
Box::pin(async move {
initialize_observed.set(true);
Ok(())
})
},
)
.unwrap_err();
assert!(error.to_string().contains("unavailable"));
assert!(!prepared.get());
assert!(!initialized.get());
assert_eq!(fs::read_dir(root.path()).unwrap().count(), 0);
let listener = StdTcpListener::bind(address).expect("runner never bound listener");
drop(listener);
}

#[test]
fn ready_and_rc_deferred_inputs_share_completed_registries_with_requests() {
let options = options();
Expand Down
Loading
Loading