Read the connection URI from the environment. (#325)

### What

The connection URI can now be provided from the environment, using
`"uri": {"variable": "..."}`.

This means that we can use the same configuration in development and
production, and enables the CLI or Cloud build infrastructure to fill it
in with e.g. a region-specific URL.

### How

We replace the `ResolvedSecret` from the NDC SDK (which has also been
removed from the SDK) with our own `Secret` type, which can be `Plain`
or `FromEnvironment`. We then handle the case appropriately, whereas
before, we would panic if we ever received a non-resolved secret.

I have introduced the `Environment` trait to supply environment
variables. During normal execution, this is a wrapper around
`sys::env::var`. In tests, however, we can use `EmptyEnvironment`, or a
`HashMap` containing test data.

This also simplified a couple of pieces of test infrastructure.

* We no longer need to template the Aurora configuration (we just
provide the appropriate value as an environment variable).
* Generating configuration becomes a single call to `ndc-postgres-cli
update`.
* The benchmarks no longer need a generated configuration file.

Author: SamirTalwar

.github/workflows/benchmarks.yaml

@@ -34,16 +34,6 @@ jobs:
           cd benchmarks/component
           docker compose up --detach --wait postgres grafana
 
-      - name: Generate the NDC metadata configuration 🚧
-        run: |
-          set -e -u -o pipefail
-          cd benchmarks/component
-          mkdir -p generated/ndc-metadata
-          jq --arg uri 'postgresql://postgres:password@postgres' \
-            '.connectionUri.uri.value = $uri' \
-            ../../static/postgres/v3-chinook-ndc-metadata/configuration.json \
-            > ./generated/ndc-metadata/configuration.json
-
       - name: Run benchmarks 🏃
         run: |
           cd benchmarks/component

.github/workflows/cargo-test.yaml

@@ -122,15 +122,6 @@ jobs:
           version: "25.x"
           repo-token: ${{ secrets.GITHUB_TOKEN }}
 
-      - name: setup NDC metadata
-        env:
-          AURORA_CONNECTION_STRING: ${{ secrets.AURORA_CONNECTION_STRING }}
-        run: |
-          jq \
-            '.connectionUri = {"uri": {"value": (env | .AURORA_CONNECTION_STRING)}}' \
-            static/aurora/v3-chinook-ndc-metadata/template.json \
-            > static/aurora/v3-chinook-ndc-metadata/configuration.json
-
       - name: install tools
         run: |
           rustup show

Cargo.lock

@@ -54,12 +54,12 @@ services:
     ports:
       - 8080:8080
     environment:
-      CONFIGURATION_FILE: "/ndc-metadata.json"
+      CONNECTION_URI: "postgresql://postgres:password@postgres"
       # we don't care about traces right now, and the benchmarks flood the batch buffer
       OTEL_TRACES_SAMPLER: "always_off"
     volumes:
       - type: bind
-        source: ./generated/ndc-metadata
+        source: ../../static/postgres/v3-chinook-ndc-metadata
         target: /etc/connector
         read_only: true
     healthcheck:

benchmarks/component/docker-compose.yaml

@@ -26,23 +26,17 @@ info 'Starting the dependencies'
 docker compose up --wait postgres grafana
 POSTGRESQL_SOCKET="$(docker compose port postgres 5432)"
 
-info 'Generating the NDC metadata configuration'
-mkdir -p generated/ndc-metadata
-jq --arg uri "postgresql://postgres:password@${POSTGRESQL_SOCKET}" \
-  '.connectionUri.uri.value = $uri' \
-  ../../static/postgres/v3-chinook-ndc-metadata/configuration.json \
-  > ./generated/ndc-metadata/configuration.json
-
 info 'Starting the agent'
 if nc -z localhost 8080; then
   echo >&2 'ERROR: There is already an agent running on port 8080.'
   exit 1
 fi
 
+CONNECTION_URI="postgresql://postgres:password@${POSTGRESQL_SOCKET}" \
 OTEL_EXPORTER_OTLP_TRACES_ENDPOINT='http://localhost:4317' \
-  OTEL_SERVICE_NAME='ndc-postgres' \
+OTEL_SERVICE_NAME='ndc-postgres' \
   cargo run -p ndc-postgres --quiet --release -- \
-    serve --configuration=./generated/ndc-metadata \
+    serve --configuration='../../static/postgres/v3-chinook-ndc-metadata' \
   >& agent.log &
 AGENT_PID=$!
 echo "$AGENT_PID" > ./agent.pid

benchmarks/component/start.sh

@@ -27,10 +27,14 @@ pub enum Error {
 }
 
 /// Run a command in a given directory.
-pub async fn run(command: Command, context_path: &Path) -> anyhow::Result<()> {
+pub async fn run(
+    command: Command,
+    context_path: &Path,
+    environment: impl configuration::environment::Environment,
+) -> anyhow::Result<()> {
     match command {
         Command::Initialize => initialize(context_path)?,
-        Command::Update => update(context_path).await?,
+        Command::Update => update(context_path, environment).await?,
     };
     Ok(())
 }
@@ -58,13 +62,16 @@ fn initialize(context_path: &Path) -> anyhow::Result<()> {
 /// Update the configuration in the current directory by introspecting the database.
 ///
 /// This expects a configuration with a valid connection URI.
-async fn update(context_path: &Path) -> anyhow::Result<()> {
+async fn update(
+    context_path: &Path,
+    environment: impl configuration::environment::Environment,
+) -> anyhow::Result<()> {
     let configuration_file_path = context_path.join(configuration::CONFIGURATION_FILENAME);
     let input: configuration::RawConfiguration = {
         let reader = fs::File::open(&configuration_file_path)?;
         serde_json::from_reader(reader)?
     };
-    let output = configuration::introspect(input).await?;
+    let output = configuration::introspect(input, environment).await?;
     let writer = fs::File::create(&configuration_file_path)?;
     serde_json::to_writer_pretty(writer, &output)?;
     Ok(())

crates/cli/src/lib.rs

@@ -9,6 +9,7 @@ use std::path::PathBuf;
 use clap::Parser;
 
 use ndc_postgres_cli::*;
+use ndc_postgres_configuration as configuration;
 
 /// The command-line arguments.
 #[derive(Debug, Parser)]
@@ -31,6 +32,11 @@ pub async fn main() -> anyhow::Result<()> {
         Some(path) => path,
         None => env::current_dir()?,
     };
-    run(args.subcommand, &context_path).await?;
+    run(
+        args.subcommand,
+        &context_path,
+        configuration::environment::ProcessEnvironment,
+    )
+    .await?;
     Ok(())
 }

crates/cli/src/main.rs

@@ -1,13 +1,19 @@
 use std::fs;
 
 use ndc_postgres_cli::*;
+use ndc_postgres_configuration as configuration;
 use ndc_postgres_configuration::RawConfiguration;
 
 #[tokio::test]
 async fn test_initialize_directory() -> anyhow::Result<()> {
     let dir = tempfile::tempdir()?;
 
-    run(Command::Initialize, dir.path()).await?;
+    run(
+        Command::Initialize,
+        dir.path(),
+        configuration::environment::EmptyEnvironment,
+    )
+    .await?;
 
     let configuration_file_path = dir.path().join("configuration.json");
     assert!(configuration_file_path.exists());
@@ -25,7 +31,13 @@ async fn test_do_not_initialize_when_files_already_exist() -> anyhow::Result<()>
         "this directory is no longer empty",
     )?;
 
-    match run(Command::Initialize, dir.path()).await {
+    match run(
+        Command::Initialize,
+        dir.path(),
+        configuration::environment::EmptyEnvironment,
+    )
+    .await
+    {
         Ok(()) => panic!("Expected the command to fail."),
         Err(error) => match error.downcast::<Error>() {
             Err(input) => panic!("Expected a CLI error, but got {input}"),

crates/cli/tests/initialize_tests.rs

@@ -1,38 +1,44 @@
+use std::collections::HashMap;
 use std::fs;
 
 use ndc_postgres_cli::*;
 use ndc_postgres_configuration as configuration;
 use ndc_postgres_configuration::RawConfiguration;
 
-const CONNECTION_STRING: &str = "postgresql://postgres:password@localhost:64002";
+const CONNECTION_URI: &str = "postgresql://postgres:password@localhost:64002";
 
 #[tokio::test]
 async fn test_update_configuration() -> anyhow::Result<()> {
     let dir = tempfile::tempdir()?;
+
+    let connection_uri = configuration::ConnectionUri(configuration::Secret::FromEnvironment {
+        variable: "CONNECTION_URI".into(),
+    });
+
     {
         let configuration_file_path = dir.path().join("configuration.json");
         let input = RawConfiguration::Version3(configuration::version3::RawConfiguration {
-            connection_uri: CONNECTION_STRING.into(),
+            connection_uri: connection_uri.clone(),
             ..configuration::version3::RawConfiguration::empty()
         });
         let writer = fs::File::create(configuration_file_path)?;
         serde_json::to_writer(writer, &input)?;
     }
 
-    run(Command::Update, dir.path()).await?;
+    let environment = HashMap::from([("CONNECTION_URI".into(), CONNECTION_URI.to_string())]);
+    run(Command::Update, dir.path(), environment).await?;
 
     let configuration_file_path = dir.path().join("configuration.json");
     assert!(configuration_file_path.exists());
     let contents = fs::read_to_string(configuration_file_path)?;
     let output: RawConfiguration = serde_json::from_str(&contents)?;
     match output {
         RawConfiguration::Version3(configuration::version3::RawConfiguration {
-            connection_uri:
-                configuration::ConnectionUri::Uri(configuration::ResolvedSecret(connection_uri)),
+            connection_uri: updated_connection_uri,
             metadata,
             ..
         }) => {
-            assert_eq!(connection_uri, CONNECTION_STRING.to_string());
+            assert_eq!(updated_connection_uri, connection_uri);
             let some_table_metadata = metadata.tables.0.get("Artist");
             assert!(some_table_metadata.is_some());
         }

crates/cli/tests/update_tests.rs

@@ -9,11 +9,12 @@ workspace = true
 [dependencies]
 query-engine-metadata = { path = "../query-engine/metadata" }
 
-ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "09edbd5" }
+ndc-sdk = { git = "https://github.com/hasura/ndc-hub.git", rev = "393213d" }
 
+anyhow = "1.0.80"
 schemars = { version = "0.8.16", features = ["smol_str", "preserve_order"] }
 serde = "1.0.197"
 serde_json = { version = "1.0.114", features = ["raw_value"] }
 sqlx = { version = "0.7.3", features = [ "json", "postgres", "runtime-tokio-rustls" ] }
+thiserror = "1.0.57"
 tracing = "0.1.40"
-anyhow = "1.0.80"

crates/configuration/Cargo.toml

@@ -10,7 +10,8 @@ use ndc_sdk::connector;
 
 use query_engine_metadata::metadata;
 
-use crate::values::{ConnectionUri, IsolationLevel, PoolSettings, ResolvedSecret};
+use crate::environment::Environment;
+use crate::values::{ConnectionUri, IsolationLevel, PoolSettings, Secret};
 use crate::version3;
 
 pub const CONFIGURATION_FILENAME: &str = "configuration.json";
@@ -43,34 +44,50 @@ pub struct Configuration {
     pub mutations_version: Option<metadata::mutations::MutationsVersion>,
 }
 
-pub async fn introspect(input: RawConfiguration) -> anyhow::Result<RawConfiguration> {
+pub async fn introspect(
+    input: RawConfiguration,
+    environment: impl Environment,
+) -> anyhow::Result<RawConfiguration> {
     match input {
         RawConfiguration::Version3(config) => Ok(RawConfiguration::Version3(
-            version3::introspect(config).await?,
+            version3::introspect(config, environment).await?,
         )),
     }
 }
 
 pub async fn parse_configuration(
     configuration_dir: impl AsRef<Path>,
+    environment: impl Environment,
 ) -> Result<Configuration, connector::ParseError> {
     let configuration_file = configuration_dir.as_ref().join(CONFIGURATION_FILENAME);
     let configuration_reader = fs::File::open(&configuration_file)?;
     let configuration: version3::RawConfiguration = serde_json::from_reader(configuration_reader)
         .map_err(|error| {
         connector::ParseError::ParseError(connector::LocatedError {
-            file_path: configuration_file,
+            file_path: configuration_file.clone(),
             line: error.line(),
             column: error.column(),
             message: error.to_string(),
         })
     })?;
+    let connection_uri = match configuration.connection_uri {
+        ConnectionUri(Secret::Plain(uri)) => Ok(uri),
+        ConnectionUri(Secret::FromEnvironment { variable }) => {
+            environment.read(&variable).map_err(|error| {
+                connector::ParseError::ValidateError(connector::InvalidNodes(vec![
+                    connector::InvalidNode {
+                        file_path: configuration_file,
+                        node_path: vec![connector::KeyOrIndex::Key("connectionUri".to_string())],
+                        message: error.to_string(),
+                    },
+                ]))
+            })
+        }
+    }?;
     Ok(Configuration {
         metadata: configuration.metadata,
         pool_settings: configuration.pool_settings,
-        connection_uri: match configuration.connection_uri {
-            ConnectionUri::Uri(ResolvedSecret(uri)) => uri,
-        },
+        connection_uri,
         isolation_level: configuration.isolation_level,
         mutations_version: configuration.configure_options.mutations_version,
     })