From d6f98af5cc78819667b280252f7e78dd5c83965a Mon Sep 17 00:00:00 2001 From: Daniel Harvey Date: Thu, 22 Aug 2024 13:08:18 +0100 Subject: [PATCH] Optional CompatibilityConfig for `v3-engine` (#998) ### What We want to use `CompatibilityConfig` to configure turning warnings into errors after a certain date, and to make sure we don't break old builds. This allows passing a path to a optional compatibility config file to engine and scaffolds how we'll map this config to metadata resolve options. ### How Add a flag to `v3-engine`, parse the file if flag is set. Test it by adding static test file and using it with `just watch` and `just run`. V3_GIT_ORIGIN_REV_ID: 972c67ae29905b9ce1bb57e150f4cfcfd6a069ef --- 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, 151 insertions(+), 43 deletions(-) create mode 100644 v3/crates/compatibility/src/error.rs create mode 100644 v3/crates/compatibility/src/types.rs create mode 100644 v3/static/compatibility/compatibility_config.json diff --git a/v3/Cargo.lock b/v3/Cargo.lock index d30a7925d5d..36e1d06c45a 100644 --- a/v3/Cargo.lock +++ b/v3/Cargo.lock @@ -939,7 +939,9 @@ dependencies = [ name = "compatibility" version = "3.0.0" dependencies = [ + "anyhow", "chrono", + "metadata-resolve", "open-dds", "opendds-derive", "schemars", @@ -1767,6 +1769,7 @@ dependencies = [ "base64 0.22.1", "build-data", "clap", + "compatibility", "criterion", "execute", "goldenfile", diff --git a/v3/changelog.md b/v3/changelog.md index b8d99a780a6..c82bdebc722 100644 --- a/v3/changelog.md +++ b/v3/changelog.md @@ -4,6 +4,9 @@ ### 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 6d19b01a68e..be2f2b05c80 100644 --- a/v3/crates/compatibility/Cargo.toml +++ b/v3/crates/compatibility/Cargo.toml @@ -8,9 +8,11 @@ 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 925062c3ee3..191813f5703 100644 --- a/v3/crates/compatibility/src/config.rs +++ b/v3/crates/compatibility/src/config.rs @@ -1,27 +1,7 @@ use super::compatibility_date::CompatibilityDate; +use super::types::CompatibilityConfig; use chrono::NaiveDate; - -#[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. -} +use metadata_resolve::configuration::WarningsToRaise; pub const fn new_compatibility_date(year: i32, month: u32, day: u32) -> CompatibilityDate { CompatibilityDate(match NaiveDate::from_ymd_opt(year, month, day) { @@ -31,17 +11,34 @@ pub const fn new_compatibility_date(year: i32, month: u32, day: u32) -> Compatib }) } -#[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), +/// 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) + + } } diff --git a/v3/crates/compatibility/src/error.rs b/v3/crates/compatibility/src/error.rs new file mode 100644 index 00000000000..28fa5031932 --- /dev/null +++ b/v3/crates/compatibility/src/error.rs @@ -0,0 +1,16 @@ +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 e994b0b79bf..00b796fa821 100644 --- a/v3/crates/compatibility/src/lib.rs +++ b/v3/crates/compatibility/src/lib.rs @@ -1,7 +1,13 @@ 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::{ - new_compatibility_date, CompatibilityConfigV1, CompatibilityConfigV2, CompatibilityError, + config_to_metadata_resolve, new_compatibility_date, resolve_compatibility_config, }; diff --git a/v3/crates/compatibility/src/types.rs b/v3/crates/compatibility/src/types.rs new file mode 100644 index 00000000000..a42f6f530f6 --- /dev/null +++ b/v3/crates/compatibility/src/types.rs @@ -0,0 +1,40 @@ +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 1392c25f6ec..fed5392643f 100644 --- a/v3/crates/engine/Cargo.toml +++ b/v3/crates/engine/Cargo.toml @@ -21,6 +21,7 @@ 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 32867b09316..65dc49defe1 100644 --- a/v3/crates/engine/bin/engine/main.rs +++ b/v3/crates/engine/bin/engine/main.rs @@ -66,6 +66,9 @@ 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, @@ -195,6 +198,8 @@ 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), } @@ -348,11 +353,6 @@ 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,8 +362,10 @@ 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, - metadata_resolve_configuration, + server.partial_supergraph, + resolve_unstable_features(&server.unstable_features), ) .map_err(StartupError::ReadSchema)?; @@ -748,14 +750,34 @@ fn print_warnings(warnings: Vec) { fn build_state( expose_internal_errors: execute::ExposeInternalErrors, authn_config_path: &PathBuf, + compatibility_config_path: &Option, metadata_path: &PathBuf, - metadata_resolve_configuration: metadata_resolve::configuration::Configuration, + allow_unknown_subgraphs: bool, + unstable_features: metadata_resolve::configuration::UnstableFeatures, ) -> 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 b34f624892e..7087940026a 100644 --- a/v3/crates/engine/tests/common.rs +++ b/v3/crates/engine/tests/common.rs @@ -548,6 +548,7 @@ 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 363e3ec7a1f..d68ba98d77e 100644 --- a/v3/crates/metadata-resolve/src/types/configuration.rs +++ b/v3/crates/metadata-resolve/src/types/configuration.rs @@ -6,6 +6,7 @@ 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 @@ -17,3 +18,9 @@ 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 a0892959c3e..eb34bc7b822 100644 --- a/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs +++ b/v3/crates/metadata-resolve/tests/metadata_golden_tests.rs @@ -97,6 +97,7 @@ 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 2c1ce08c904..84de3fe538a 100644 --- a/v3/justfile +++ b/v3/justfile @@ -37,6 +37,7 @@ 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 @@ -82,6 +83,7 @@ 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' @@ -138,6 +140,7 @@ 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 new file mode 100644 index 00000000000..c019a6cbe82 --- /dev/null +++ b/v3/static/compatibility/compatibility_config.json @@ -0,0 +1,6 @@ +{ + "version": "v1", + "definition": { + "date": "2023-10-09" + } +}