From 6c295d1c4580066c59618eebcf38f64db5c61c4a Mon Sep 17 00:00:00 2001 From: Daniel Harvey Date: Thu, 22 Aug 2024 16:59:27 +0100 Subject: [PATCH] Revert "Optional CompatibilityConfig for `v3-engine`" (#1006) Reverts hasura/v3-engine#998 We are going to use `opendds` Flags for this instead. V3_GIT_ORIGIN_REV_ID: 155c7a6b17be95a6370b08fdc425791e3eb8b70a --- v3/Cargo.lock | 3 - v3/changelog.md | 3 - v3/crates/compatibility/Cargo.toml | 2 - v3/crates/compatibility/src/config.rs | 67 ++++++++++--------- v3/crates/compatibility/src/error.rs | 16 ----- v3/crates/compatibility/src/lib.rs | 8 +-- v3/crates/compatibility/src/types.rs | 40 ----------- v3/crates/engine/Cargo.toml | 1 - v3/crates/engine/bin/engine/main.rs | 36 ++-------- v3/crates/engine/tests/common.rs | 1 - .../src/types/configuration.rs | 7 -- .../tests/metadata_golden_tests.rs | 1 - v3/justfile | 3 - .../compatibility/compatibility_config.json | 6 -- 14 files changed, 43 insertions(+), 151 deletions(-) delete mode 100644 v3/crates/compatibility/src/error.rs delete mode 100644 v3/crates/compatibility/src/types.rs delete mode 100644 v3/static/compatibility/compatibility_config.json diff --git a/v3/Cargo.lock b/v3/Cargo.lock index 36e1d06c45a..d30a7925d5d 100644 --- a/v3/Cargo.lock +++ b/v3/Cargo.lock @@ -939,9 +939,7 @@ dependencies = [ name = "compatibility" version = "3.0.0" dependencies = [ - "anyhow", "chrono", - "metadata-resolve", "open-dds", "opendds-derive", "schemars", @@ -1769,7 +1767,6 @@ dependencies = [ "base64 0.22.1", "build-data", "clap", - "compatibility", "criterion", "execute", "goldenfile", diff --git a/v3/changelog.md b/v3/changelog.md index c82bdebc722..b8d99a780a6 100644 --- a/v3/changelog.md +++ b/v3/changelog.md @@ -4,9 +4,6 @@ ### Added -- Allow passing an optional `CompatibiityConfig` file that allows opting in to - new breaking metadata changes. - ### Fixed ### Changed diff --git a/v3/crates/compatibility/Cargo.toml b/v3/crates/compatibility/Cargo.toml index be2f2b05c80..6d19b01a68e 100644 --- a/v3/crates/compatibility/Cargo.toml +++ b/v3/crates/compatibility/Cargo.toml @@ -8,11 +8,9 @@ license.workspace = true bench = false [dependencies] -metadata-resolve = { path = "../metadata-resolve" } open-dds = { path = "../open-dds" } opendds-derive = { path = "../utils/opendds-derive" } -anyhow = { workspace = true } chrono = { workspace = true } schemars = { workspace = true } serde = { workspace = true } diff --git a/v3/crates/compatibility/src/config.rs b/v3/crates/compatibility/src/config.rs index 191813f5703..925062c3ee3 100644 --- a/v3/crates/compatibility/src/config.rs +++ b/v3/crates/compatibility/src/config.rs @@ -1,7 +1,27 @@ use super::compatibility_date::CompatibilityDate; -use super::types::CompatibilityConfig; use chrono::NaiveDate; -use metadata_resolve::configuration::WarningsToRaise; + +#[derive(Clone, Debug, PartialEq, serde::Serialize, opendds_derive::OpenDd)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +#[opendd(json_schema(id = "v1/CompatibilityConfig"))] +/// The compatibility configuration of the Hasura metadata. +pub struct CompatibilityConfigV1 { + /// Any backwards incompatible changes made to Hasura DDN after this date won't impact the metadata. + pub date: CompatibilityDate, + // TODO: add flags. +} + +#[derive(Clone, Debug, PartialEq, serde::Serialize, opendds_derive::OpenDd)] +#[serde(rename_all = "camelCase")] +#[serde(deny_unknown_fields)] +#[opendd(json_schema(id = "v2/CompatibilityConfig"))] +/// The compatibility configuration of the Hasura metadata. +pub struct CompatibilityConfigV2 { + /// Any backwards incompatible changes made to Hasura DDN after this date won't impact the metadata. + pub date: CompatibilityDate, + // TODO: add flags. +} pub const fn new_compatibility_date(year: i32, month: u32, day: u32) -> CompatibilityDate { CompatibilityDate(match NaiveDate::from_ymd_opt(year, month, day) { @@ -11,34 +31,17 @@ pub const fn new_compatibility_date(year: i32, month: u32, day: u32) -> Compatib }) } -/// Resolve `CompatibilityConfig` which is not part of metadata. Hence we resolve/build -/// it separately. -pub fn resolve_compatibility_config( - raw_compatibility_config: &str, -) -> Result { - Ok(open_dds::traits::OpenDd::deserialize( - serde_json::from_str(raw_compatibility_config)?, - )?) -} - -// given compatibility config, work out which warnings becomes errors using the date -pub fn config_to_metadata_resolve(compat_config: &Option) -> WarningsToRaise { - match compat_config { - Some(CompatibilityConfig::V1(compat_config_v1)) => { - warnings_to_raise_from_date(&compat_config_v1.date) - } - Some(CompatibilityConfig::V2(compat_config_v2)) => { - warnings_to_raise_from_date(&compat_config_v2.date) - } - - None => WarningsToRaise::default(), - } -} - -// note we have no warnings to raise yet, so this is a no-op whilst we get the plumbing sorted -fn warnings_to_raise_from_date(_date: &CompatibilityDate) -> WarningsToRaise { - WarningsToRaise { - // some_boolean_option: date >= &new_compatibility_date(2024, 1, 1) - - } +#[derive(Debug, PartialEq, thiserror::Error)] +pub enum CompatibilityError { + #[error("no compatibility config found")] + NoCompatibilityConfigFound, + #[error("duplicate compatibility config found")] + DuplicateCompatibilityConfig, + #[error("compatibility date {specified} is too old, oldest supported date is {oldest}")] + DateTooOld { + specified: CompatibilityDate, + oldest: CompatibilityDate, + }, + #[error("compatibility date {0} is in the future")] + DateInTheFuture(CompatibilityDate), } diff --git a/v3/crates/compatibility/src/error.rs b/v3/crates/compatibility/src/error.rs deleted file mode 100644 index 28fa5031932..00000000000 --- a/v3/crates/compatibility/src/error.rs +++ /dev/null @@ -1,16 +0,0 @@ -use super::compatibility_date::CompatibilityDate; - -#[derive(Debug, PartialEq, thiserror::Error)] -pub enum CompatibilityError { - #[error("no compatibility config found")] - NoCompatibilityConfigFound, - #[error("duplicate compatibility config found")] - DuplicateCompatibilityConfig, - #[error("compatibility date {specified} is too old, oldest supported date is {oldest}")] - DateTooOld { - specified: CompatibilityDate, - oldest: CompatibilityDate, - }, - #[error("compatibility date {0} is in the future")] - DateInTheFuture(CompatibilityDate), -} diff --git a/v3/crates/compatibility/src/lib.rs b/v3/crates/compatibility/src/lib.rs index 00b796fa821..e994b0b79bf 100644 --- a/v3/crates/compatibility/src/lib.rs +++ b/v3/crates/compatibility/src/lib.rs @@ -1,13 +1,7 @@ mod compatibility_date; pub use compatibility_date::CompatibilityDate; -mod error; -pub use error::CompatibilityError; - -mod types; -pub use types::{CompatibilityConfigV1, CompatibilityConfigV2}; - mod config; pub use config::{ - config_to_metadata_resolve, new_compatibility_date, resolve_compatibility_config, + new_compatibility_date, CompatibilityConfigV1, CompatibilityConfigV2, CompatibilityError, }; diff --git a/v3/crates/compatibility/src/types.rs b/v3/crates/compatibility/src/types.rs deleted file mode 100644 index a42f6f530f6..00000000000 --- a/v3/crates/compatibility/src/types.rs +++ /dev/null @@ -1,40 +0,0 @@ -use super::compatibility_date::CompatibilityDate; -use serde::{Deserialize, Serialize}; - -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, opendds_derive::OpenDd)] -#[serde(rename_all = "camelCase")] -#[serde(deny_unknown_fields)] -#[opendd(json_schema(id = "v1/CompatibilityConfig"))] -/// The compatibility configuration of the Hasura metadata. -pub struct CompatibilityConfigV1 { - /// Any backwards incompatible changes made to Hasura DDN after this date won't impact the metadata. - pub date: CompatibilityDate, - // TODO: add flags. -} - -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, opendds_derive::OpenDd)] -#[serde(rename_all = "camelCase")] -#[serde(deny_unknown_fields)] -#[opendd(json_schema(id = "v2/CompatibilityConfig"))] -/// The compatibility configuration of the Hasura metadata. -pub struct CompatibilityConfigV2 { - /// Any backwards incompatible changes made to Hasura DDN after this date won't impact the metadata. - pub date: CompatibilityDate, - // TODO: add flags. -} - -#[derive(Serialize, Debug, Clone, PartialEq, opendds_derive::OpenDd, Deserialize)] -#[serde(tag = "version", content = "definition")] -#[serde(rename_all = "camelCase")] -#[serde(deny_unknown_fields)] -#[opendd(as_versioned_with_definition)] -#[opendd(json_schema(title = "CompatibilityConfig"))] -/// Definition of the authentication configuration used by the API server. -pub enum CompatibilityConfig { - /// Definition of the authentication configuration v1, used by the API server. - #[opendd(json_schema(title = "CompatibilityConfigV1"))] - V1(CompatibilityConfigV1), - /// Definition of the authentication configuration v2, used by the API server. - #[opendd(json_schema(title = "CompatibilityConfigV2"))] - V2(CompatibilityConfigV2), -} diff --git a/v3/crates/engine/Cargo.toml b/v3/crates/engine/Cargo.toml index fed5392643f..1392c25f6ec 100644 --- a/v3/crates/engine/Cargo.toml +++ b/v3/crates/engine/Cargo.toml @@ -21,7 +21,6 @@ name = "execute" harness = false [dependencies] -compatibility = { path = "../compatibility" } execute = { path = "../execute" } hasura-authn-core = { path = "../auth/hasura-authn-core" } hasura-authn-jwt = { path = "../auth/hasura-authn-jwt" } diff --git a/v3/crates/engine/bin/engine/main.rs b/v3/crates/engine/bin/engine/main.rs index 65dc49defe1..32867b09316 100644 --- a/v3/crates/engine/bin/engine/main.rs +++ b/v3/crates/engine/bin/engine/main.rs @@ -66,9 +66,6 @@ struct ServerOptions { /// The configuration file used for authentication. #[arg(long, value_name = "PATH", env = "AUTHN_CONFIG_PATH")] authn_config_path: PathBuf, - /// The configuration file used for compatibility. - #[arg(long, value_name = "PATH", env = "COMPATIBILITY_CONFIG_PATH")] - compatibility_config_path: Option, /// The host IP on which the server listens, defaulting to all IPv4 and IPv6 addresses. #[arg(long, value_name = "HOST", env = "HOST", default_value_t = net::IpAddr::V6(net::Ipv6Addr::UNSPECIFIED))] host: net::IpAddr, @@ -198,8 +195,6 @@ async fn shutdown_signal() { enum StartupError { #[error("could not read the auth config - {0}")] ReadAuth(anyhow::Error), - #[error("could not read the compatibility config - {0}")] - ReadCompatibility(anyhow::Error), #[error("failed to build engine state - {0}")] ReadSchema(anyhow::Error), } @@ -353,6 +348,11 @@ impl EngineRouter { #[allow(clippy::print_stdout)] async fn start_engine(server: &ServerOptions) -> Result<(), StartupError> { + let metadata_resolve_configuration = metadata_resolve::configuration::Configuration { + allow_unknown_subgraphs: server.partial_supergraph, + unstable_features: resolve_unstable_features(&server.unstable_features), + }; + let expose_internal_errors = if server.expose_internal_errors { execute::ExposeInternalErrors::Expose } else { @@ -362,10 +362,8 @@ async fn start_engine(server: &ServerOptions) -> Result<(), StartupError> { let state = build_state( expose_internal_errors, &server.authn_config_path, - &server.compatibility_config_path, &server.metadata_path, - server.partial_supergraph, - resolve_unstable_features(&server.unstable_features), + metadata_resolve_configuration, ) .map_err(StartupError::ReadSchema)?; @@ -750,34 +748,14 @@ fn print_warnings(warnings: Vec) { fn build_state( expose_internal_errors: execute::ExposeInternalErrors, authn_config_path: &PathBuf, - compatibility_config_path: &Option, metadata_path: &PathBuf, - allow_unknown_subgraphs: bool, - unstable_features: metadata_resolve::configuration::UnstableFeatures, + metadata_resolve_configuration: metadata_resolve::configuration::Configuration, ) -> Result, anyhow::Error> { // Auth Config let raw_auth_config = std::fs::read_to_string(authn_config_path)?; let (auth_config, auth_warnings) = resolve_auth_config(&raw_auth_config).map_err(StartupError::ReadAuth)?; - // Compatibility Config - let compatibility_config = match compatibility_config_path { - Some(path) => { - let raw_config = std::fs::read_to_string(path)?; - let compatibility_config = compatibility::resolve_compatibility_config(&raw_config) - .map_err(StartupError::ReadCompatibility)?; - Some(compatibility_config) - } - None => None, - }; - - // derive metadata resolve configuration using compatibility configuration - let metadata_resolve_configuration = metadata_resolve::configuration::Configuration { - allow_unknown_subgraphs, - unstable_features, - warnings_to_raise: compatibility::config_to_metadata_resolve(&compatibility_config), - }; - // Metadata let raw_metadata = std::fs::read_to_string(metadata_path)?; let metadata = open_dds::Metadata::from_json_str(&raw_metadata)?; diff --git a/v3/crates/engine/tests/common.rs b/v3/crates/engine/tests/common.rs index 7087940026a..b34f624892e 100644 --- a/v3/crates/engine/tests/common.rs +++ b/v3/crates/engine/tests/common.rs @@ -548,7 +548,6 @@ pub(crate) fn test_metadata_resolve_configuration() -> metadata_resolve::configu enable_order_by_expressions: false, enable_ndc_v02_support: true, }, - warnings_to_raise: metadata_resolve::configuration::WarningsToRaise {}, } } diff --git a/v3/crates/metadata-resolve/src/types/configuration.rs b/v3/crates/metadata-resolve/src/types/configuration.rs index d68ba98d77e..363e3ec7a1f 100644 --- a/v3/crates/metadata-resolve/src/types/configuration.rs +++ b/v3/crates/metadata-resolve/src/types/configuration.rs @@ -6,7 +6,6 @@ pub struct Configuration { pub allow_unknown_subgraphs: bool, pub unstable_features: UnstableFeatures, - pub warnings_to_raise: WarningsToRaise, } /// internal feature flags used in metadata resolve steps @@ -18,9 +17,3 @@ pub struct UnstableFeatures { pub enable_order_by_expressions: bool, pub enable_ndc_v02_support: bool, } - -/// struct of warnings that we'd like to raise to errors, based on CompatibilityConfig -/// -/// Deserialization is only intended to be used for testing and is not reliable. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, serde::Deserialize)] -pub struct WarningsToRaise {} diff --git a/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs b/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs index eb34bc7b822..a0892959c3e 100644 --- a/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs +++ b/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs @@ -97,7 +97,6 @@ fn read_test_configuration( Ok(configuration::Configuration { allow_unknown_subgraphs: false, unstable_features, - warnings_to_raise: configuration::WarningsToRaise {}, }) } } diff --git a/v3/justfile b/v3/justfile index 84de3fe538a..2c1ce08c904 100644 --- a/v3/justfile +++ b/v3/justfile @@ -37,7 +37,6 @@ run-local-with-shell: RUST_LOG=DEBUG cargo run --bin engine -- \ --otlp-endpoint http://localhost:4317 \ --authn-config-path static/auth/auth_config.json \ - --compatibility-config-path static/compatibility/compatibility_config.json \ --metadata-path crates/engine/tests/schema.json \ --expose-internal-errors | ts "engine: " & wait @@ -83,7 +82,6 @@ watch: start-docker-test-deps start-docker-run-deps -x 'run --bin engine -- \ --otlp-endpoint http://localhost:4317 \ --authn-config-path static/auth/auth_config.json \ - --compatibility-config-path static/compatibility/compatibility_config.json \ --metadata-path crates/engine/tests/schema.json \ --expose-internal-errors' @@ -140,7 +138,6 @@ run: start-docker-test-deps start-docker-run-deps RUST_LOG=DEBUG cargo run --bin engine -- \ --otlp-endpoint http://localhost:4317 \ --authn-config-path static/auth/auth_config.json \ - --compatibility-config-path static/compatibility/compatibility_config.json \ --metadata-path crates/engine/tests/schema.json \ --expose-internal-errors diff --git a/v3/static/compatibility/compatibility_config.json b/v3/static/compatibility/compatibility_config.json deleted file mode 100644 index c019a6cbe82..00000000000 --- a/v3/static/compatibility/compatibility_config.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "version": "v1", - "definition": { - "date": "2023-10-09" - } -}