Skip to content

Commit cb63c58

Browse files
Benoit Aubuchonclaude
andauthored
Support mooseRuntimeEnv.get() for dictionary external source credentials (#3954)
## Summary - Extends `resolve_runtime_credentials_from_env()` in `InfrastructureMap` to resolve `__MOOSE_RUNTIME_ENV__` markers in dictionary external source credential fields - Covers all credentialed sources: ClickHouse, MySQL, PostgreSQL, Redis, MongoDB, and S3 - Uses the same `resolve_runtime_env` / `resolve_optional_runtime_env` infrastructure already used for S3Queue/S3/IcebergS3 table engines ## Motivation ClickHouse Cloud does not support named collections, so dictionary sources require inline `user`/`password` credentials in the DDL. Without this change, users had no way to avoid hardcoding secrets. With this change, they can write: ```typescript OlapDictionary("my_dict", { source: { clickhouse: { host: "ch.cloud.example.com", user: mooseRuntimeEnv.get("CH_DICT_USER"), password: mooseRuntimeEnv.get("CH_DICT_PASSWORD"), ... } } }) ``` ## Test plan - [x] 8 unit tests added covering all 6 credentialed source types plus error and passthrough cases - [x] `cargo clippy --all-targets -- -D warnings` clean - [x] All existing dictionary tests continue to pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches credential-resolution logic for ClickHouse external dictionaries (including error reporting) and adjusts masking/comparison semantics; mistakes could break deployments or leak/ignore credential changes. Changes are well-covered by new regression/unit tests, reducing risk. > > **Overview** > Adds runtime resolution of `__MOOSE_RUNTIME_ENV__` markers for **external-source ClickHouse dictionaries** (ClickHouse/MySQL/Postgres/Redis/MongoDB/S3) in `InfrastructureMap::resolve_runtime_credentials_from_env`, enabling `mooseRuntimeEnv.get()` for dictionary credentials. > > Clarifies and enforces credential-masking semantics: **secrets are masked but usernames remain plaintext** so username changes trigger dictionary rebuilds/diffs; expands drift detection normalization to ignore dictionary `metadata` and password-only differences to avoid false drift. > > Adds extensive regression/unit tests covering dictionary env resolution, masking behavior, diff detection for username changes, and drift detection ignoring metadata-only differences. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9689a57. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 23b7ef0 commit cb63c58

2 files changed

Lines changed: 906 additions & 9 deletions

File tree

apps/framework-cli/src/cli/routines/migrate.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2232,6 +2232,45 @@ mod tests {
22322232
dict
22332233
}
22342234

2235+
// Regression: when expected dict has a source file in its metadata but current has
2236+
// none, detect_drift must return NoDrift — not DriftDetected — because dict metadata
2237+
// must be stripped before comparison, just as table metadata is.
2238+
#[test]
2239+
fn test_detect_drift_no_false_drift_with_dict_metadata_difference() {
2240+
use crate::framework::core::infrastructure::table::{Metadata, SourceLocation};
2241+
2242+
let tables: HashMap<String, Table> = HashMap::new();
2243+
2244+
let mut expected_dict = create_test_dict("my_dict");
2245+
expected_dict.metadata = Some(Metadata {
2246+
description: None,
2247+
source: Some(SourceLocation {
2248+
file: "/old/path/my_dict.ts".to_string(),
2249+
}),
2250+
});
2251+
let mut expected_dicts = HashMap::new();
2252+
expected_dicts.insert("my_dict".to_string(), expected_dict);
2253+
2254+
// current and target have no metadata (metadata stripped after introspection / runtime)
2255+
let mut current_dicts = HashMap::new();
2256+
current_dicts.insert("my_dict".to_string(), create_test_dict("my_dict"));
2257+
let target_dicts = current_dicts.clone();
2258+
2259+
let result = detect_drift(
2260+
&tables,
2261+
&tables,
2262+
&tables,
2263+
&current_dicts,
2264+
&expected_dicts,
2265+
&target_dicts,
2266+
&[],
2267+
);
2268+
assert!(
2269+
matches!(result, DriftStatus::NoDrift),
2270+
"Metadata-only differences in dicts must not cause false drift"
2271+
);
2272+
}
2273+
22352274
// Regression: when state_before was saved with masked credentials (CREDENTIAL_PLACEHOLDER)
22362275
// but the live infra map has real credentials, detect_drift must return NoDrift — not
22372276
// DriftDetected — because credentials should be normalized out before comparison.

0 commit comments

Comments
 (0)