From 14bbf1c46569841a8cd132d43cb6b8f61c9a2e7e Mon Sep 17 00:00:00 2001 From: Philip Lykke Carlsen Date: Fri, 20 Oct 2023 13:55:22 +0200 Subject: [PATCH] Use new `SecretValue` from `ndc-sdk` (#92) ### What For consistency with the rest of the services we use the `SecretValue` type from ndc-sdk for values that can be derived from secrets. --- .github/workflows/cargo-test.yaml | 2 +- Cargo.lock | 7 +- crates/connectors/ndc-citus/Cargo.toml | 6 +- .../connectors/ndc-citus/tests/ndc_tests.rs | 10 ++- crates/connectors/ndc-cockroach/Cargo.toml | 6 +- .../ndc-cockroach/tests/ndc_tests.rs | 10 ++- crates/connectors/ndc-postgres/Cargo.toml | 6 +- .../src/configuration/version1.rs | 66 ++++++++----------- .../ndc-postgres/tests/ndc_tests.rs | 10 ++- ...ation_tests__get_configuration_schema.snap | 33 +++++++++- ...on_tests__get_rawconfiguration_schema.snap | 33 +++++++++- .../openapi_generator__tests__main.snap | 33 +++++++++- crates/query-engine/translation/Cargo.toml | 2 +- crates/tests/other-db-tests/Cargo.toml | 2 +- crates/tests/tests-common/Cargo.toml | 2 +- 15 files changed, 166 insertions(+), 62 deletions(-) diff --git a/.github/workflows/cargo-test.yaml b/.github/workflows/cargo-test.yaml index d02b593da..03f34bfac 100644 --- a/.github/workflows/cargo-test.yaml +++ b/.github/workflows/cargo-test.yaml @@ -124,7 +124,7 @@ jobs: run: | # take connection string from env, create deployment file with it cat static/aurora/chinook-deployment-template.json \ - | jq '.connectionUri={"uri":(env | .AURORA_CONNECTION_STRING)}' \ + | jq '.connectionUri={"uri":{"value":(env | .AURORA_CONNECTION_STRING)}}' \ > static/aurora/chinook-deployment.json - name: install tools diff --git a/Cargo.lock b/Cargo.lock index 555cfdb1d..24167d124 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1434,7 +1434,7 @@ dependencies = [ [[package]] name = "ndc-client" version = "0.1.0" -source = "git+https://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.7#1edc777373579299fac06149e48e9744865fda49" +source = "git+https://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.8#2c2def8a01af2f95100b4274599030c930525bee" dependencies = [ "async-trait", "indexmap 1.9.3", @@ -1504,7 +1504,7 @@ dependencies = [ [[package]] name = "ndc-sdk" version = "0.1.0" -source = "git+https://github.com/hasura/ndc-hub.git?rev=41ae7fc#41ae7fc086f9123f792cd010344b59fcfbfa6c78" +source = "git+https://github.com/hasura/ndc-hub.git?rev=098b1c2#098b1c23e37df25102804d3222fb4313e0d8bbb4" dependencies = [ "async-trait", "axum", @@ -1542,7 +1542,7 @@ dependencies = [ [[package]] name = "ndc-test" version = "0.1.0" -source = "git+https://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.7#1edc777373579299fac06149e48e9744865fda49" +source = "git+https://github.com/hasura/ndc-spec.git?tag=v0.1.0-rc.8#2c2def8a01af2f95100b4274599030c930525bee" dependencies = [ "async-trait", "clap", @@ -1552,6 +1552,7 @@ dependencies = [ "proptest", "reqwest", "semver", + "serde", "serde_json", "thiserror", "tokio", diff --git a/crates/connectors/ndc-citus/Cargo.toml b/crates/connectors/ndc-citus/Cargo.toml index eb4cae475..9d33bfd68 100644 --- a/crates/connectors/ndc-citus/Cargo.toml +++ b/crates/connectors/ndc-citus/Cargo.toml @@ -15,7 +15,7 @@ name = "ndc-citus" path = "bin/main.rs" [dependencies] -ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "41ae7fc", package = "ndc-sdk" } +ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "098b1c2", package = "ndc-sdk" } ndc-postgres = { path = "../ndc-postgres" } async-trait = "0.1.74" @@ -25,8 +25,8 @@ tokio = { version = "1.33.0", features = ["full"] } tracing = "0.1.39" [dev-dependencies] -ndc-client = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.7" } -ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.7" } +ndc-client = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.8" } +ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.8" } tests-common = { path = "../../tests/tests-common" } axum = "0.6.19" diff --git a/crates/connectors/ndc-citus/tests/ndc_tests.rs b/crates/connectors/ndc-citus/tests/ndc_tests.rs index e107ab82a..9af7009e7 100644 --- a/crates/connectors/ndc-citus/tests/ndc_tests.rs +++ b/crates/connectors/ndc-citus/tests/ndc_tests.rs @@ -32,8 +32,14 @@ async fn test_connector() -> Result<(), Vec> { api_key: None, }; - let test_results = - ndc_test::test_connector(&ndc_test::TestConfiguration { seed: None }, &configuration).await; + let test_results = ndc_test::test_connector( + &ndc_test::TestConfiguration { + seed: None, + snapshots_dir: None, + }, + &configuration, + ) + .await; if test_results.failures.is_empty() { Ok(()) } else { diff --git a/crates/connectors/ndc-cockroach/Cargo.toml b/crates/connectors/ndc-cockroach/Cargo.toml index 8293247df..b28cd4de4 100644 --- a/crates/connectors/ndc-cockroach/Cargo.toml +++ b/crates/connectors/ndc-cockroach/Cargo.toml @@ -15,7 +15,7 @@ name = "ndc-cockroach" path = "bin/main.rs" [dependencies] -ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "41ae7fc", package = "ndc-sdk" } +ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "098b1c2", package = "ndc-sdk" } ndc-postgres = { path = "../ndc-postgres" } async-trait = "0.1.74" @@ -25,8 +25,8 @@ tokio = { version = "1.33.0", features = ["full"] } tracing = "0.1.39" [dev-dependencies] -ndc-client = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.7" } -ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.7" } +ndc-client = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.8" } +ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.8" } tests-common = { path = "../../tests/tests-common" } axum = "0.6.19" diff --git a/crates/connectors/ndc-cockroach/tests/ndc_tests.rs b/crates/connectors/ndc-cockroach/tests/ndc_tests.rs index e107ab82a..9af7009e7 100644 --- a/crates/connectors/ndc-cockroach/tests/ndc_tests.rs +++ b/crates/connectors/ndc-cockroach/tests/ndc_tests.rs @@ -32,8 +32,14 @@ async fn test_connector() -> Result<(), Vec> { api_key: None, }; - let test_results = - ndc_test::test_connector(&ndc_test::TestConfiguration { seed: None }, &configuration).await; + let test_results = ndc_test::test_connector( + &ndc_test::TestConfiguration { + seed: None, + snapshots_dir: None, + }, + &configuration, + ) + .await; if test_results.failures.is_empty() { Ok(()) } else { diff --git a/crates/connectors/ndc-postgres/Cargo.toml b/crates/connectors/ndc-postgres/Cargo.toml index 8f77c2947..079adcb10 100644 --- a/crates/connectors/ndc-postgres/Cargo.toml +++ b/crates/connectors/ndc-postgres/Cargo.toml @@ -16,7 +16,7 @@ path = "bin/main.rs" [dependencies] -ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "41ae7fc", package = "ndc-sdk" } +ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "098b1c2", package = "ndc-sdk" } query-engine-execution = { path = "../../query-engine/execution" } query-engine-metadata = { path = "../../query-engine/metadata" } query-engine-sql = { path = "../../query-engine/sql" } @@ -33,8 +33,8 @@ tokio = { version = "1.33.0", features = ["full"] } tracing = "0.1.39" [dev-dependencies] -ndc-client = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.7" } -ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.7" } +ndc-client = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.8" } +ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.0-rc.8" } tests-common = { path = "../../tests/tests-common" } axum = "0.6.19" diff --git a/crates/connectors/ndc-postgres/src/configuration/version1.rs b/crates/connectors/ndc-postgres/src/configuration/version1.rs index 8ba073f2c..6a5706556 100644 --- a/crates/connectors/ndc-postgres/src/configuration/version1.rs +++ b/crates/connectors/ndc-postgres/src/configuration/version1.rs @@ -2,7 +2,7 @@ use tracing::{info_span, Instrument}; use ndc_sdk::connector; -use ndc_sdk::models::secretable_value_reference; +use ndc_sdk::secret::{SecretValue, SecretValueImpl}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use sqlx::postgres::PgConnection; @@ -169,53 +169,45 @@ pub struct Configuration { pub config: RawConfiguration, } -/// A wrapper around a value that may have come directly from user-specified -/// configuration, or may have been resolved from a secret provided externally. -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] -#[serde( - from = "ResolvedSecretIntermediate", - into = "ResolvedSecretIntermediate" -)] +// Configuration type for values that can come from secrets. That format includes both literal +// values as well as symbolic references to secrets. +// At this point we should only ever see resolved secrets, which this type captures. +// +// In order to correctly mark fields that can have secret values in the json schema, this type +// intentionally does not implement the JsonSchema trait. Instead, you should +// attach the attribute: +// +// #[schemars(with = "SecretValue")] +// +// to fields of type 'ResolvedSecret'. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(try_from = "SecretValue")] +#[serde(into = "SecretValue")] pub struct ResolvedSecret(pub String); -/// The intermediate type representing the two formats in which we can parse -/// `ResolvedSecret`: -/// -/// 1. `"postgresql://..."` -/// 2. `{"value": "postgresql://..."}` -/// -/// We do not store this type, it is only used during deserialization. -#[derive(Debug, Deserialize, Serialize)] -#[serde(untagged)] -enum ResolvedSecretIntermediate { - Unwrapped(String), - Wrapped { value: String }, -} - -impl From for ResolvedSecret { - fn from(value: ResolvedSecretIntermediate) -> Self { - match value { - ResolvedSecretIntermediate::Unwrapped(inner) => ResolvedSecret(inner), - ResolvedSecretIntermediate::Wrapped { value: inner } => ResolvedSecret(inner), +impl TryFrom for ResolvedSecret { + fn try_from(value: SecretValue) -> Result { + match value.0 { + SecretValueImpl::Value(v) => Ok(ResolvedSecret(v)), + SecretValueImpl::StringValueFromSecret(secret) => { + Err(format!("Unresolved secret: {}", secret)) + } } } + + type Error = String; } -// The wrapped form is the canonical form, so we always serialize to that. -impl From for ResolvedSecretIntermediate { - fn from(ResolvedSecret(value): ResolvedSecret) -> Self { - Self::Wrapped { value } +impl From for SecretValue { + fn from(value: ResolvedSecret) -> SecretValue { + SecretValue(SecretValueImpl::Value(value.0)) } } -// In the user facing configuration, the connection string can either be a literal or a reference -// to a secret, so we advertize either in the JSON schema. However, when building the configuration, -// we expect the metadata build service to have resolved the secret reference so we deserialize -// only to a String. -#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub enum ConnectionUri { - Uri(#[schemars(schema_with = "secretable_value_reference")] ResolvedSecret), + Uri(#[schemars(with = "SecretValue")] ResolvedSecret), } impl RawConfiguration { diff --git a/crates/connectors/ndc-postgres/tests/ndc_tests.rs b/crates/connectors/ndc-postgres/tests/ndc_tests.rs index e107ab82a..9af7009e7 100644 --- a/crates/connectors/ndc-postgres/tests/ndc_tests.rs +++ b/crates/connectors/ndc-postgres/tests/ndc_tests.rs @@ -32,8 +32,14 @@ async fn test_connector() -> Result<(), Vec> { api_key: None, }; - let test_results = - ndc_test::test_connector(&ndc_test::TestConfiguration { seed: None }, &configuration).await; + let test_results = ndc_test::test_connector( + &ndc_test::TestConfiguration { + seed: None, + snapshots_dir: None, + }, + &configuration, + ) + .await; if test_results.failures.is_empty() { Ok(()) } else { diff --git a/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_configuration_schema.snap b/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_configuration_schema.snap index a06f0e8e8..1215b4346 100644 --- a/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_configuration_schema.snap +++ b/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_configuration_schema.snap @@ -70,7 +70,38 @@ expression: schema ], "properties": { "uri": { - "$ref": "https://raw.githubusercontent.com/hasura/ndc-spec/main/ndc-client/tests/json_schema/secretable_value.jsonschema" + "$ref": "#/definitions/SecretValueImpl" + } + }, + "additionalProperties": false + } + ] + }, + "SecretValueImpl": { + "$id": "https://hasura.io/jsonschemas/SecretValue", + "title": "SecretValue", + "description": "Either a literal string or a reference to a Hasura secret", + "oneOf": [ + { + "type": "object", + "required": [ + "value" + ], + "properties": { + "value": { + "type": "string" + } + }, + "additionalProperties": false + }, + { + "type": "object", + "required": [ + "stringValueFromSecret" + ], + "properties": { + "stringValueFromSecret": { + "type": "string" } }, "additionalProperties": false diff --git a/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_rawconfiguration_schema.snap b/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_rawconfiguration_schema.snap index a3a0bb3e2..5d4d78ae2 100644 --- a/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_rawconfiguration_schema.snap +++ b/crates/connectors/ndc-postgres/tests/snapshots/configuration_tests__get_rawconfiguration_schema.snap @@ -58,7 +58,38 @@ expression: schema ], "properties": { "uri": { - "$ref": "https://raw.githubusercontent.com/hasura/ndc-spec/main/ndc-client/tests/json_schema/secretable_value.jsonschema" + "$ref": "#/definitions/SecretValueImpl" + } + }, + "additionalProperties": false + } + ] + }, + "SecretValueImpl": { + "$id": "https://hasura.io/jsonschemas/SecretValue", + "title": "SecretValue", + "description": "Either a literal string or a reference to a Hasura secret", + "oneOf": [ + { + "type": "object", + "required": [ + "value" + ], + "properties": { + "value": { + "type": "string" + } + }, + "additionalProperties": false + }, + { + "type": "object", + "required": [ + "stringValueFromSecret" + ], + "properties": { + "stringValueFromSecret": { + "type": "string" } }, "additionalProperties": false diff --git a/crates/documentation/openapi/src/snapshots/openapi_generator__tests__main.snap b/crates/documentation/openapi/src/snapshots/openapi_generator__tests__main.snap index 559ada97b..0c932f4f3 100644 --- a/crates/documentation/openapi/src/snapshots/openapi_generator__tests__main.snap +++ b/crates/documentation/openapi/src/snapshots/openapi_generator__tests__main.snap @@ -58,7 +58,38 @@ expression: generated_schema ], "properties": { "uri": { - "$ref": "https://raw.githubusercontent.com/hasura/ndc-spec/main/ndc-client/tests/json_schema/secretable_value.jsonschema" + "$ref": "#/components/schemas/SecretValueImpl" + } + }, + "additionalProperties": false + } + ] + }, + "SecretValueImpl": { + "$id": "https://hasura.io/jsonschemas/SecretValue", + "title": "SecretValue", + "description": "Either a literal string or a reference to a Hasura secret", + "oneOf": [ + { + "type": "object", + "required": [ + "value" + ], + "properties": { + "value": { + "type": "string" + } + }, + "additionalProperties": false + }, + { + "type": "object", + "required": [ + "stringValueFromSecret" + ], + "properties": { + "stringValueFromSecret": { + "type": "string" } }, "additionalProperties": false diff --git a/crates/query-engine/translation/Cargo.toml b/crates/query-engine/translation/Cargo.toml index 8e65b6921..e61edc6a9 100644 --- a/crates/query-engine/translation/Cargo.toml +++ b/crates/query-engine/translation/Cargo.toml @@ -6,7 +6,7 @@ license.workspace = true [dependencies] -ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "41ae7fc", package = "ndc-sdk" } +ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "098b1c2", package = "ndc-sdk" } query-engine-metadata = { path = "../metadata" } query-engine-sql = { path = "../sql" } diff --git a/crates/tests/other-db-tests/Cargo.toml b/crates/tests/other-db-tests/Cargo.toml index 703be5a51..e616ad62f 100644 --- a/crates/tests/other-db-tests/Cargo.toml +++ b/crates/tests/other-db-tests/Cargo.toml @@ -16,7 +16,7 @@ yugabyte = [] [dependencies] tests-common = { path = "../tests-common" } -ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "41ae7fc", package = "ndc-sdk" } +ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "098b1c2", package = "ndc-sdk" } ndc-postgres = { path = "../../connectors/ndc-postgres" } axum = "0.6.19" diff --git a/crates/tests/tests-common/Cargo.toml b/crates/tests/tests-common/Cargo.toml index 6ae88fbfe..f9fc4f942 100644 --- a/crates/tests/tests-common/Cargo.toml +++ b/crates/tests/tests-common/Cargo.toml @@ -9,7 +9,7 @@ name = "tests_common" path = "src/lib.rs" [dependencies] -ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "41ae7fc", package = "ndc-sdk" } +ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "098b1c2", package = "ndc-sdk" } axum = "0.6.19" axum-test-helper = "0.3.0"