Skip to content

Commit

Permalink
Use new SecretValue from ndc-sdk (#92)
Browse files Browse the repository at this point in the history 
### 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.
  • Loading branch information
@plcplc
plcplc authored Oct 20, 2023
1 parent 8cf9b72 commit 14bbf1c
Show file tree
Hide file tree
Showing 15 changed files with 166 additions and 62 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cargo-test.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions crates/connectors/ndc-citus/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
10 changes: 8 additions & 2 deletions crates/connectors/ndc-citus/tests/ndc_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ async fn test_connector() -> Result<(), Vec<ndc_test::FailedTest>> {
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 {
Expand Down
6 changes: 3 additions & 3 deletions crates/connectors/ndc-cockroach/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
10 changes: 8 additions & 2 deletions crates/connectors/ndc-cockroach/tests/ndc_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ async fn test_connector() -> Result<(), Vec<ndc_test::FailedTest>> {
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 {
Expand Down
6 changes: 3 additions & 3 deletions crates/connectors/ndc-postgres/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand All @@ -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"
Expand Down
66 changes: 29 additions & 37 deletions crates/connectors/ndc-postgres/src/configuration/version1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ResolvedSecretIntermediate> for ResolvedSecret {
fn from(value: ResolvedSecretIntermediate) -> Self {
match value {
ResolvedSecretIntermediate::Unwrapped(inner) => ResolvedSecret(inner),
ResolvedSecretIntermediate::Wrapped { value: inner } => ResolvedSecret(inner),
impl TryFrom<SecretValue> for ResolvedSecret {
fn try_from(value: SecretValue) -> Result<Self, Self::Error> {
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<ResolvedSecret> for ResolvedSecretIntermediate {
fn from(ResolvedSecret(value): ResolvedSecret) -> Self {
Self::Wrapped { value }
impl From<ResolvedSecret> 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 {
Expand Down
10 changes: 8 additions & 2 deletions crates/connectors/ndc-postgres/tests/ndc_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@ async fn test_connector() -> Result<(), Vec<ndc_test::FailedTest>> {
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 {
Expand Down
33 changes: 32 additions & 1 deletion ...onnectors/ndc-postgres/tests/snapshots/configuration_tests__get_configuration_schema.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 32 additions & 1 deletion ...ectors/ndc-postgres/tests/snapshots/configuration_tests__get_rawconfiguration_schema.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 32 additions & 1 deletion crates/documentation/openapi/src/snapshots/openapi_generator__tests__main.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/query-engine/translation/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
2 changes: 1 addition & 1 deletion crates/tests/other-db-tests/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion crates/tests/tests-common/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

1 comment on commit 14bbf1c

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Component benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 14bbf1c Previous: 8cf9b72 Ratio
select - request time - (query + acquisition) 19.958129630793493 ms 7.829051171410114 ms 2.55

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.