From c3d83d2a698f13a217e2ae0048462fdb9e25628d Mon Sep 17 00:00:00 2001 From: Simon Scatton Date: Thu, 25 Jun 2026 14:17:11 +0200 Subject: [PATCH] test(e2e): stop using custom e2e binary builds Remove the production-crate dev-settings feature so e2e tests build the same gateway, CLI, and supervisor binaries that we ship. Move settings coverage from dummy test-only keys to the production ocsf_json_enabled setting. Signed-off-by: Simon Scatton --- .github/workflows/docker-build.yml | 4 +- .github/workflows/rust-native-build.yml | 2 +- crates/openshell-cli/Cargo.toml | 1 - crates/openshell-cli/src/run.rs | 20 +---- crates/openshell-core/Cargo.toml | 4 - crates/openshell-core/src/settings.rs | 23 ------ crates/openshell-server/Cargo.toml | 1 - crates/openshell-server/src/grpc/policy.rs | 94 ++++++++++++---------- e2e/rust/e2e-docker.sh | 2 +- e2e/rust/e2e-kubernetes.sh | 2 +- e2e/rust/e2e-podman.sh | 2 +- e2e/rust/e2e-vm.sh | 3 +- e2e/rust/tests/settings_management.rs | 6 +- e2e/support/gateway-common.sh | 6 +- tasks/scripts/stage-prebuilt-binaries.sh | 2 +- tasks/test.toml | 6 +- 16 files changed, 73 insertions(+), 105 deletions(-) diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index aae0a05b9b..4c92bc3857 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -84,13 +84,13 @@ jobs: gateway) binary_component=gateway binary_name=openshell-gateway - features="openshell-core/dev-settings bundled-z3" + features="bundled-z3" has_image=true ;; supervisor) binary_component=sandbox binary_name=openshell-sandbox - features="openshell-core/dev-settings" + features="" has_image=true ;; cli) diff --git a/.github/workflows/rust-native-build.yml b/.github/workflows/rust-native-build.yml index 65d691e39c..89993ba12d 100644 --- a/.github/workflows/rust-native-build.yml +++ b/.github/workflows/rust-native-build.yml @@ -30,7 +30,7 @@ on: description: "Cargo features to enable" required: false type: string - default: "openshell-core/dev-settings" + default: "" retention-days: description: "Artifact retention period" required: false diff --git a/crates/openshell-cli/Cargo.toml b/crates/openshell-cli/Cargo.toml index 577fb73b93..8e4cb3fb25 100644 --- a/crates/openshell-cli/Cargo.toml +++ b/crates/openshell-cli/Cargo.toml @@ -86,7 +86,6 @@ workspace = true [features] bundled-z3 = ["openshell-prover/bundled-z3"] -dev-settings = ["openshell-core/dev-settings"] [dev-dependencies] futures = { workspace = true } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 1c3fd8a820..29468aa49f 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -8097,16 +8097,15 @@ mod tests { )); } - #[cfg(feature = "dev-settings")] #[test] fn parse_cli_setting_value_parses_bool_aliases() { - let yes_value = parse_cli_setting_value("dummy_bool", "yes").expect("parse yes"); + let yes_value = parse_cli_setting_value("ocsf_json_enabled", "yes").expect("parse yes"); assert_eq!( yes_value.value, Some(openshell_core::proto::setting_value::Value::BoolValue(true)) ); - let zero_value = parse_cli_setting_value("dummy_bool", "0").expect("parse 0"); + let zero_value = parse_cli_setting_value("ocsf_json_enabled", "0").expect("parse 0"); assert_eq!( zero_value.value, Some(openshell_core::proto::setting_value::Value::BoolValue( @@ -8115,21 +8114,10 @@ mod tests { ); } - #[cfg(feature = "dev-settings")] - #[test] - fn parse_cli_setting_value_parses_int_key() { - let int_value = parse_cli_setting_value("dummy_int", "42").expect("parse int"); - assert_eq!( - int_value.value, - Some(openshell_core::proto::setting_value::Value::IntValue(42)) - ); - } - - #[cfg(feature = "dev-settings")] #[test] fn parse_cli_setting_value_rejects_invalid_bool() { - let err = - parse_cli_setting_value("dummy_bool", "maybe").expect_err("invalid bool should fail"); + let err = parse_cli_setting_value("ocsf_json_enabled", "maybe") + .expect_err("invalid bool should fail"); assert!(err.to_string().contains("invalid bool value")); } diff --git a/crates/openshell-core/Cargo.toml b/crates/openshell-core/Cargo.toml index bf35811643..0ff6d06d6c 100644 --- a/crates/openshell-core/Cargo.toml +++ b/crates/openshell-core/Cargo.toml @@ -33,10 +33,6 @@ default = ["telemetry"] ## `--no-default-features` (plus any other features you need) for a build that ## contains no telemetry endpoint, no HTTP client, and no emission code at all. telemetry = ["dep:reqwest", "dep:chrono"] -## Include test-only settings (dummy_bool, dummy_int) in the registry. -## Off by default so production builds have an empty registry. -## Enabled by e2e tests and during development. -dev-settings = [] ## Expose proposals::test_helpers (`ProposalsFlagGuard`) to downstream test ## code in other crates. Enabled by openshell-sandbox and ## openshell-supervisor-network dev builds. diff --git a/crates/openshell-core/src/settings.rs b/crates/openshell-core/src/settings.rs index 3ff1f36f8e..156e4c3845 100644 --- a/crates/openshell-core/src/settings.rs +++ b/crates/openshell-core/src/settings.rs @@ -137,20 +137,6 @@ pub const REGISTERED_SETTINGS: &[RegisteredSetting] = &[ kind: SettingValueKind::String, allowed_string_values: Some(PROPOSAL_APPROVAL_MODE_VALUES), }, - // Test-only keys live behind the `dev-settings` feature flag so they - // don't appear in production builds. - #[cfg(feature = "dev-settings")] - RegisteredSetting { - key: "dummy_int", - kind: SettingValueKind::Int, - allowed_string_values: None, - }, - #[cfg(feature = "dev-settings")] - RegisteredSetting { - key: "dummy_bool", - kind: SettingValueKind::Bool, - allowed_string_values: None, - }, ]; /// Resolve a setting descriptor from the registry by key. @@ -187,15 +173,6 @@ mod tests { registered_keys_csv, setting_for_key, }; - #[cfg(feature = "dev-settings")] - #[test] - fn setting_for_key_returns_dev_entries() { - let setting = setting_for_key("dummy_bool").expect("dummy_bool should be registered"); - assert_eq!(setting.kind, SettingValueKind::Bool); - let setting = setting_for_key("dummy_int").expect("dummy_int should be registered"); - assert_eq!(setting.kind, SettingValueKind::Int); - } - #[test] fn setting_for_key_returns_none_for_unknown() { assert!(setting_for_key("nonexistent_key").is_none()); diff --git a/crates/openshell-server/Cargo.toml b/crates/openshell-server/Cargo.toml index 39a26b14e7..b5c9b34d71 100644 --- a/crates/openshell-server/Cargo.toml +++ b/crates/openshell-server/Cargo.toml @@ -105,7 +105,6 @@ default = ["telemetry"] ## that contains no telemetry endpoint, HTTP client, or emission code. telemetry = ["openshell-core/telemetry"] bundled-z3 = ["openshell-prover/bundled-z3"] -dev-settings = ["openshell-core/dev-settings"] test-support = [] [dev-dependencies] diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 770bb71cca..b4fd093ea2 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -8895,24 +8895,22 @@ mod tests { assert!(err.message().contains("unknown setting key")); } - #[cfg(feature = "dev-settings")] #[test] fn proto_setting_to_stored_rejects_type_mismatch() { let value = SettingValue { value: Some(setting_value::Value::StringValue("true".to_string())), }; - let err = proto_setting_to_stored("dummy_bool", &value).unwrap_err(); + let err = proto_setting_to_stored("ocsf_json_enabled", &value).unwrap_err(); assert_eq!(err.code(), Code::InvalidArgument); assert!(err.message().contains("expects bool value")); } - #[cfg(feature = "dev-settings")] #[test] fn proto_setting_to_stored_accepts_bool_for_registered_bool_key() { let value = SettingValue { value: Some(setting_value::Value::BoolValue(true)), }; - let stored = proto_setting_to_stored("dummy_bool", &value).unwrap(); + let stored = proto_setting_to_stored("ocsf_json_enabled", &value).unwrap(); assert_eq!(stored, StoredSettingValue::Bool(true)); } @@ -8988,17 +8986,19 @@ mod tests { ); } - #[cfg(feature = "dev-settings")] #[test] fn merge_effective_settings_global_overrides_sandbox_key() { let global = StoredSettings { revision: 2, settings: [ ( - "log_level".to_string(), - StoredSettingValue::String("warn".to_string()), + settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(false), + ), + ( + settings::AGENT_POLICY_PROPOSALS_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(false), ), - ("dummy_int".to_string(), StoredSettingValue::Int(7)), ] .into_iter() .collect(), @@ -9008,10 +9008,13 @@ mod tests { revision: 1, settings: [ ( - "log_level".to_string(), - StoredSettingValue::String("debug".to_string()), + settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + ), + ( + "ocsf_json_enabled".to_string(), + StoredSettingValue::Bool(true), ), - ("dummy_bool".to_string(), StoredSettingValue::Bool(true)), ] .into_iter() .collect(), @@ -9019,39 +9022,45 @@ mod tests { }; let merged = merge_effective_settings(&global, &sandbox).unwrap(); - let log_level = merged.get("log_level").expect("log_level present"); - assert_eq!(log_level.scope, SettingScope::Global as i32); + let providers_v2 = merged + .get(settings::PROVIDERS_V2_ENABLED_KEY) + .expect("providers_v2_enabled present"); + assert_eq!(providers_v2.scope, SettingScope::Global as i32); assert_eq!( - log_level.value.as_ref().and_then(|v| v.value.as_ref()), - Some(&setting_value::Value::StringValue("warn".to_string())) + providers_v2.value.as_ref().and_then(|v| v.value.as_ref()), + Some(&setting_value::Value::BoolValue(false)) ); - let dummy_bool = merged.get("dummy_bool").expect("dummy_bool present"); - assert_eq!(dummy_bool.scope, SettingScope::Sandbox as i32); + let ocsf_json = merged + .get("ocsf_json_enabled") + .expect("ocsf_json_enabled present"); + assert_eq!(ocsf_json.scope, SettingScope::Sandbox as i32); - let dummy_int = merged.get("dummy_int").expect("dummy_int present"); - assert_eq!(dummy_int.scope, SettingScope::Global as i32); + let proposals = merged + .get(settings::AGENT_POLICY_PROPOSALS_ENABLED_KEY) + .expect("agent_policy_proposals_enabled present"); + assert_eq!(proposals.scope, SettingScope::Global as i32); } - #[cfg(feature = "dev-settings")] #[test] fn merge_effective_settings_sandbox_scoped_value_has_sandbox_scope() { let global = StoredSettings::default(); let sandbox = StoredSettings { revision: 1, - settings: [( - "log_level".to_string(), - StoredSettingValue::String("debug".to_string()), - )] - .into_iter() + settings: std::iter::once(( + "ocsf_json_enabled".to_string(), + StoredSettingValue::Bool(true), + )) .collect(), ..Default::default() }; let merged = merge_effective_settings(&global, &sandbox).unwrap(); - let log_level = merged.get("log_level").expect("log_level present"); - assert_eq!(log_level.scope, SettingScope::Sandbox as i32); - assert!(log_level.value.is_some()); + let ocsf_json = merged + .get("ocsf_json_enabled") + .expect("ocsf_json_enabled present"); + assert_eq!(ocsf_json.scope, SettingScope::Sandbox as i32); + assert!(ocsf_json.value.is_some()); } #[test] @@ -9274,9 +9283,10 @@ mod tests { "log_level".to_string(), StoredSettingValue::String("error".to_string()), ); - settings - .settings - .insert("dummy_bool".to_string(), StoredSettingValue::Bool(true)); + settings.settings.insert( + "ocsf_json_enabled".to_string(), + StoredSettingValue::Bool(true), + ); settings.revision = 5; save_global_settings(&store, &settings).await.unwrap(); @@ -9287,7 +9297,7 @@ mod tests { Some(&StoredSettingValue::String("error".to_string())) ); assert_eq!( - loaded.settings.get("dummy_bool"), + loaded.settings.get("ocsf_json_enabled"), Some(&StoredSettingValue::Bool(true)) ); } @@ -9298,9 +9308,10 @@ mod tests { let sandbox_name = "my-sandbox"; let mut settings = StoredSettings::default(); - settings - .settings - .insert("dummy_int".to_string(), StoredSettingValue::Int(99)); + settings.settings.insert( + settings::PROPOSAL_APPROVAL_MODE_KEY.to_string(), + StoredSettingValue::String("auto".to_string()), + ); settings.revision = 3; save_sandbox_settings(&store, sandbox_name, &settings) .await @@ -9309,8 +9320,8 @@ mod tests { let loaded = load_sandbox_settings(&store, sandbox_name).await.unwrap(); assert_eq!(loaded.revision, 3); assert_eq!( - loaded.settings.get("dummy_int"), - Some(&StoredSettingValue::Int(99)) + loaded.settings.get(settings::PROPOSAL_APPROVAL_MODE_KEY), + Some(&StoredSettingValue::String("auto".to_string())) ); } @@ -9434,14 +9445,15 @@ mod tests { let store = test_store().await; let mut global = StoredSettings::default(); - global - .settings - .insert("dummy_int".to_string(), StoredSettingValue::Int(42)); + global.settings.insert( + "ocsf_json_enabled".to_string(), + StoredSettingValue::Bool(true), + ); global.revision = 1; save_global_settings(&store, &global).await.unwrap(); let loaded_global = load_global_settings(&store).await.unwrap(); - assert!(loaded_global.settings.contains_key("dummy_int")); + assert!(loaded_global.settings.contains_key("ocsf_json_enabled")); } #[tokio::test] diff --git a/e2e/rust/e2e-docker.sh b/e2e/rust/e2e-docker.sh index a020f87c83..70e9835bde 100755 --- a/e2e/rust/e2e-docker.sh +++ b/e2e/rust/e2e-docker.sh @@ -12,7 +12,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" E2E_TEST="${OPENSHELL_E2E_DOCKER_TEST:-smoke}" E2E_FEATURES="${OPENSHELL_E2E_DOCKER_FEATURES:-e2e,e2e-docker}" -cargo build -p openshell-cli --features openshell-core/dev-settings +cargo build -p openshell-cli exec "${ROOT}/e2e/with-docker-gateway.sh" \ cargo test --manifest-path "${ROOT}/e2e/rust/Cargo.toml" \ diff --git a/e2e/rust/e2e-kubernetes.sh b/e2e/rust/e2e-kubernetes.sh index 0644a06183..ec0faefed5 100755 --- a/e2e/rust/e2e-kubernetes.sh +++ b/e2e/rust/e2e-kubernetes.sh @@ -21,7 +21,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" E2E_FEATURES="${OPENSHELL_E2E_KUBERNETES_FEATURES:-e2e,e2e-host-gateway,e2e-kubernetes}" -cargo build -p openshell-cli --features openshell-core/dev-settings +cargo build -p openshell-cli test_filter=() if [ -n "${OPENSHELL_E2E_KUBE_TEST:-}" ]; then diff --git a/e2e/rust/e2e-podman.sh b/e2e/rust/e2e-podman.sh index 5f325d0d27..26843e128c 100755 --- a/e2e/rust/e2e-podman.sh +++ b/e2e/rust/e2e-podman.sh @@ -12,7 +12,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" E2E_TEST="${OPENSHELL_E2E_PODMAN_TEST:-}" E2E_FEATURES="${OPENSHELL_E2E_PODMAN_FEATURES:-e2e-podman}" -cargo build -p openshell-cli --features openshell-core/dev-settings +cargo build -p openshell-cli TEST_ARGS=( cargo test --manifest-path "${ROOT}/e2e/rust/Cargo.toml" diff --git a/e2e/rust/e2e-vm.sh b/e2e/rust/e2e-vm.sh index 926821e539..467d419f02 100755 --- a/e2e/rust/e2e-vm.sh +++ b/e2e/rust/e2e-vm.sh @@ -91,8 +91,7 @@ echo "==> Building openshell-gateway, openshell-driver-vm, openshell (CLI)" cargo build \ -p openshell-server \ -p openshell-driver-vm \ - -p openshell-cli \ - --features openshell-core/dev-settings + -p openshell-cli if [ "$(uname -s)" = "Darwin" ]; then echo "==> Codesigning openshell-driver-vm (Hypervisor entitlement)" diff --git a/e2e/rust/tests/settings_management.rs b/e2e/rust/tests/settings_management.rs index 69cb7cf161..e69269d7d8 100644 --- a/e2e/rust/tests/settings_management.rs +++ b/e2e/rust/tests/settings_management.rs @@ -21,7 +21,7 @@ use openshell_e2e::harness::output::strip_ansi; use openshell_e2e::harness::sandbox::SandboxGuard; use tokio::time::{Instant, sleep}; -const TEST_KEY: &str = "dummy_bool"; +const TEST_KEY: &str = "ocsf_json_enabled"; static SETTINGS_E2E_LOCK: Mutex<()> = Mutex::new(()); struct CliResult { @@ -231,7 +231,7 @@ async fn settings_global_override_round_trip() { assert!( set_global .clean_output - .contains("Set global setting dummy_bool=false"), + .contains(&format!("Set global setting {TEST_KEY}=false")), "expected global set output:\n{}", set_global.clean_output ); @@ -289,7 +289,7 @@ async fn settings_global_override_round_trip() { assert!( delete_global .clean_output - .contains("Deleted global setting dummy_bool"), + .contains(&format!("Deleted global setting {TEST_KEY}")), "expected global delete confirmation in output:\n{}", delete_global.clean_output ); diff --git a/e2e/support/gateway-common.sh b/e2e/support/gateway-common.sh index 8da3d07064..a52b4c0770 100644 --- a/e2e/support/gateway-common.sh +++ b/e2e/support/gateway-common.sh @@ -186,13 +186,11 @@ e2e_build_gateway_binaries() { echo "Building openshell-gateway..." cargo build "${jobs[@]}" \ - -p openshell-server --bin openshell-gateway \ - --features openshell-core/dev-settings + -p openshell-server --bin openshell-gateway echo "Building openshell-cli..." cargo build "${jobs[@]}" \ - -p openshell-cli --bin openshell \ - --features openshell-core/dev-settings + -p openshell-cli if [ ! -x "${target_dir}/debug/openshell-gateway" ]; then echo "ERROR: expected openshell-gateway binary at ${target_dir}/debug/openshell-gateway" >&2 diff --git a/tasks/scripts/stage-prebuilt-binaries.sh b/tasks/scripts/stage-prebuilt-binaries.sh index 99cc2cf4b8..8d1334b8c6 100755 --- a/tasks/scripts/stage-prebuilt-binaries.sh +++ b/tasks/scripts/stage-prebuilt-binaries.sh @@ -153,7 +153,7 @@ build_component_for_arch() { resolve_component "$component" target="$(target_triple "$arch" "$target_libc")" stage="${ROOT}/deploy/docker/.build/prebuilt-binaries/${arch}" - features="${EXTRA_CARGO_FEATURES:-openshell-core/dev-settings}" + features="${EXTRA_CARGO_FEATURES:-}" if [[ "$component" == "gateway" && " ${features} " != *" bundled-z3 "* ]]; then features="${features} bundled-z3" fi diff --git a/tasks/test.toml b/tasks/test.toml index 3ee4b6ab5a..444ea15e16 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -60,14 +60,14 @@ hide = true ["e2e:rust"] description = "Run Rust CLI e2e tests against a Docker-backed gateway" run = [ - "cargo build -p openshell-cli --features openshell-core/dev-settings", + "cargo build -p openshell-cli", "e2e/with-docker-gateway.sh cargo test --manifest-path e2e/rust/Cargo.toml --features e2e-docker", ] ["e2e:websocket-conformance"] description = "Run focused WebSocket conformance e2e tests against a Docker-backed gateway" run = [ - "cargo build -p openshell-cli --features openshell-core/dev-settings", + "cargo build -p openshell-cli", "e2e/with-docker-gateway.sh cargo test --manifest-path e2e/rust/Cargo.toml --features e2e-docker --test websocket_conformance", ] @@ -110,7 +110,7 @@ run = "e2e/rust/e2e-docker.sh" ["e2e:mechanistic-smoke"] description = "Run mechanistic L4 smoke against a Docker-backed gateway" run = [ - "cargo build -p openshell-cli --features openshell-core/dev-settings", + "cargo build -p openshell-cli", "e2e/with-docker-gateway.sh bash -lc 'target/debug/openshell settings set --global --key agent_policy_proposals_enabled --value true --yes && OPENSHELL_BIN=$PWD/target/debug/openshell bash e2e/policy-advisor/mechanistic-smoke.sh'", ]