Skip to content

Commit b1aa608

Browse files
authored
move s3 cred resolution later (#2954)
<!-- CURSOR_SUMMARY --> > [!NOTE] > Move S3 credential handling to runtime with `resolve_s3_credentials_from_env`, add `resolve_credentials` to `load_from_user_code`, and set context-specific env/flags across CLI, planning, and TS/Py libs. > > - **Core (InfrastructureMap)**: > - Add `resolve_s3_credentials_from_env()` to resolve S3/S3Queue creds from env at runtime and recalc `engine_params_hash`. > - Change `load_from_user_code(project, resolve_credentials)` and optionally resolve creds; propagate throughout callers. > - Planning (`plan.rs`): load prebuilt JSON (no creds) in prod when present, then ALWAYS resolve creds at runtime; when building from code, pass `true`. > - **Partial Map (DMV2)**: > - Keep S3/S3Queue env markers in engine configs; remove build-time secret resolution helper. > - **CLI commands**: > - Pass resolve flag appropriately: > - Runtime ops use `true`: `remote_plan`, `remote_gen_migration`, `remote_refresh`, `migrate`, workflows, seed data. > - Structure-only ops use `false`: `check` (and JSON write), `db pull`, `ls`, `kafka pull`. > - **Executors**: > - Set `IS_LOADING_INFRA_MAP=true` when running TS `dmv2-serializer` and Python serializer to emit markers during infra loading. > - **Language libs (secrets)**: > - TS/Py `mooseRuntimeEnv.get()`/`get()` now return markers when `IS_LOADING_INFRA_MAP=true`, otherwise fetch real env vars (error if unset); tests updated. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 67dc4cc. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 53bc4ba commit b1aa608

File tree

17 files changed

+252
-82
lines changed

17 files changed

+252
-82
lines changed

apps/framework-cli/src/cli.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ pub async fn top_command_handler(
482482

483483
let infra_map = if project_arc.features.data_model_v2 {
484484
debug!("Loading InfrastructureMap from user code (DMV2)");
485-
InfrastructureMap::load_from_user_code(&project_arc)
485+
// Don't resolve credentials for moose check - avoids baking into Docker
486+
InfrastructureMap::load_from_user_code(&project_arc, false)
486487
.await
487488
.map_err(|e| {
488489
RoutineFailure::error(Message {

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -888,7 +888,7 @@ async fn legacy_remote_plan_logic(
888888
// Build the inframap from the local project
889889
let local_infra_map = if project.features.data_model_v2 {
890890
debug!("Loading InfrastructureMap from user code (DMV2)");
891-
InfrastructureMap::load_from_user_code(project).await?
891+
InfrastructureMap::load_from_user_code(project, true).await?
892892
} else {
893893
debug!("Loading InfrastructureMap from primitives");
894894
let primitive_map = PrimitiveMap::load(project).await?;
@@ -994,7 +994,7 @@ pub async fn remote_plan(
994994
// Build the inframap from the local project
995995
let local_infra_map = if project.features.data_model_v2 {
996996
debug!("Loading InfrastructureMap from user code (DMV2)");
997-
InfrastructureMap::load_from_user_code(project).await?
997+
InfrastructureMap::load_from_user_code(project, true).await?
998998
} else {
999999
debug!("Loading InfrastructureMap from primitives");
10001000
let primitive_map = PrimitiveMap::load(project).await?;
@@ -1114,9 +1114,10 @@ pub async fn remote_gen_migration(
11141114
use anyhow::Context;
11151115

11161116
// Build the inframap from the local project
1117+
// Resolve credentials for generating migration DDL with S3 tables
11171118
let local_infra_map = if project.features.data_model_v2 {
11181119
debug!("Loading InfrastructureMap from user code (DMV2)");
1119-
InfrastructureMap::load_from_user_code(project).await?
1120+
InfrastructureMap::load_from_user_code(project, true).await?
11201121
} else {
11211122
debug!("Loading InfrastructureMap from primitives");
11221123
let primitive_map = PrimitiveMap::load(project).await?;
@@ -1235,7 +1236,7 @@ pub async fn remote_refresh(
12351236
// Build the inframap from the local project
12361237
let local_infra_map = if project.features.data_model_v2 {
12371238
debug!("Loading InfrastructureMap from user code (DMV2)");
1238-
InfrastructureMap::load_from_user_code(project).await?
1239+
InfrastructureMap::load_from_user_code(project, true).await?
12391240
} else {
12401241
debug!("Loading InfrastructureMap from primitives");
12411242
let primitive_map = PrimitiveMap::load(project).await?;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,8 @@ pub async fn db_pull(
493493
let (client, db) = create_client_and_db(remote_url).await?;
494494

495495
debug!("Loading InfrastructureMap from user code (DMV2)");
496-
let infra_map = InfrastructureMap::load_from_user_code(project)
496+
// Don't resolve credentials for code generation - only needs structure
497+
let infra_map = InfrastructureMap::load_from_user_code(project, false)
497498
.await
498499
.map_err(|e| {
499500
RoutineFailure::error(Message::new(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ pub async fn write_external_topics(
6363
.filter(|n| inc.is_match(n) && !exc.is_match(n))
6464
.collect();
6565

66-
let infra_map = InfrastructureMap::load_from_user_code(project)
66+
// Don't resolve credentials - only checking which topics are managed
67+
let infra_map = InfrastructureMap::load_from_user_code(project, false)
6768
.await
6869
.map_err(|e| {
6970
RoutineFailure::error(Message::new(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ pub async fn ls_dmv2(
293293
name: Option<&str>,
294294
json: bool,
295295
) -> Result<RoutineSuccess, RoutineFailure> {
296-
let infra_map = InfrastructureMap::load_from_user_code(project)
296+
// Don't resolve credentials for ls command - only inspects structure
297+
let infra_map = InfrastructureMap::load_from_user_code(project, false)
297298
.await
298299
.map_err(|e| {
299300
RoutineFailure::new(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -498,8 +498,8 @@ pub async fn execute_migration(
498498

499499
let current_tables = &current_infra_map.tables;
500500

501-
// Load target state from current code
502-
let target_infra_map = InfrastructureMap::load_from_user_code(project)
501+
// Load target state from current code with credentials resolved for migration DDL
502+
let target_infra_map = InfrastructureMap::load_from_user_code(project, true)
503503
.await
504504
.map_err(|e| {
505505
RoutineFailure::new(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ pub async fn run_workflow_and_get_run_ids(
125125
name: &str,
126126
input: Option<String>,
127127
) -> Result<WorkflowStartInfo, RoutineFailure> {
128-
let infra_map = InfrastructureMap::load_from_user_code(project)
128+
// Resolve credentials for workflows that may interact with S3 tables
129+
let infra_map = InfrastructureMap::load_from_user_code(project, true)
129130
.await
130131
.map_err(|e| {
131132
RoutineFailure::new(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ fn build_count_query(
106106
/// Loads the infrastructure map based on project configuration
107107
async fn load_infrastructure_map(project: &Project) -> Result<InfrastructureMap, RoutineFailure> {
108108
if project.features.data_model_v2 {
109-
InfrastructureMap::load_from_user_code(project)
109+
// Resolve credentials for seeding data into S3-backed tables
110+
InfrastructureMap::load_from_user_code(project, true)
110111
.await
111112
.map_err(|e| {
112113
RoutineFailure::error(Message {

apps/framework-cli/src/framework/core/infrastructure_map.rs

Lines changed: 117 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,6 +2222,108 @@ impl InfrastructureMap {
22222222
Ok(infra_map)
22232223
}
22242224

2225+
/// Resolves S3 credentials from environment variables at runtime.
2226+
///
2227+
/// This method iterates through all tables in the infrastructure map and resolves
2228+
/// any environment variable markers (like `MOOSE_ENV::AWS_ACCESS_KEY_ID`) in S3 and S3Queue
2229+
/// engine configurations to their actual values from the environment.
2230+
///
2231+
/// This MUST be called at runtime (dev/prod mode start) rather than at build time
2232+
/// to avoid baking credentials into Docker images.
2233+
///
2234+
/// # Returns
2235+
/// A Result indicating success or containing an error message if credential resolution fails
2236+
pub fn resolve_s3_credentials_from_env(&mut self) -> Result<(), String> {
2237+
use crate::infrastructure::olap::clickhouse::queries::ClickhouseEngine;
2238+
use crate::utilities::secrets::resolve_optional_runtime_env;
2239+
2240+
for table in self.tables.values_mut() {
2241+
let mut should_recalc_hash = false;
2242+
2243+
if let Some(engine) = &mut table.engine {
2244+
match engine {
2245+
ClickhouseEngine::S3Queue {
2246+
aws_access_key_id,
2247+
aws_secret_access_key,
2248+
..
2249+
} => {
2250+
// Resolve environment variable markers for AWS credentials
2251+
let resolved_access_key = resolve_optional_runtime_env(aws_access_key_id)
2252+
.map_err(|e| {
2253+
format!(
2254+
"Failed to resolve runtime environment variable for table '{}' field 'awsAccessKeyId': {}",
2255+
table.name, e
2256+
)
2257+
})?;
2258+
2259+
let resolved_secret_key =
2260+
resolve_optional_runtime_env(aws_secret_access_key).map_err(|e| {
2261+
format!(
2262+
"Failed to resolve runtime environment variable for table '{}' field 'awsSecretAccessKey': {}",
2263+
table.name, e
2264+
)
2265+
})?;
2266+
2267+
*aws_access_key_id = resolved_access_key;
2268+
*aws_secret_access_key = resolved_secret_key;
2269+
should_recalc_hash = true;
2270+
2271+
log::debug!(
2272+
"Resolved S3Queue credentials for table '{}' at runtime",
2273+
table.name
2274+
);
2275+
}
2276+
ClickhouseEngine::S3 {
2277+
aws_access_key_id,
2278+
aws_secret_access_key,
2279+
..
2280+
} => {
2281+
// Resolve environment variable markers for AWS credentials
2282+
let resolved_access_key = resolve_optional_runtime_env(aws_access_key_id)
2283+
.map_err(|e| {
2284+
format!(
2285+
"Failed to resolve runtime environment variable for table '{}' field 'awsAccessKeyId': {}",
2286+
table.name, e
2287+
)
2288+
})?;
2289+
2290+
let resolved_secret_key =
2291+
resolve_optional_runtime_env(aws_secret_access_key).map_err(|e| {
2292+
format!(
2293+
"Failed to resolve runtime environment variable for table '{}' field 'awsSecretAccessKey': {}",
2294+
table.name, e
2295+
)
2296+
})?;
2297+
2298+
*aws_access_key_id = resolved_access_key;
2299+
*aws_secret_access_key = resolved_secret_key;
2300+
should_recalc_hash = true;
2301+
2302+
log::debug!(
2303+
"Resolved S3 credentials for table '{}' at runtime",
2304+
table.name
2305+
);
2306+
}
2307+
_ => {
2308+
// No credentials to resolve for other engine types
2309+
}
2310+
}
2311+
}
2312+
2313+
// Recalculate engine_params_hash after resolving credentials
2314+
if should_recalc_hash {
2315+
table.engine_params_hash =
2316+
table.engine.as_ref().map(|e| e.non_alterable_params_hash());
2317+
log::debug!(
2318+
"Recalculated engine_params_hash for table '{}' after credential resolution",
2319+
table.name
2320+
);
2321+
}
2322+
}
2323+
2324+
Ok(())
2325+
}
2326+
22252327
/// Stores the infrastructure map in Redis for persistence and sharing
22262328
///
22272329
/// Serializes the map to protocol buffers and stores it in Redis using
@@ -2472,10 +2574,16 @@ impl InfrastructureMap {
24722574
///
24732575
/// # Arguments
24742576
/// * `project` - The project to load the infrastructure map from
2577+
/// * `resolve_credentials` - Whether to resolve S3 credentials from environment variables.
2578+
/// Set to `false` for build-time operations like `moose check` to avoid baking credentials
2579+
/// into Docker images. Set to `true` for runtime operations that need to interact with infrastructure.
24752580
///
24762581
/// # Returns
24772582
/// A Result containing the infrastructure map or an error
2478-
pub async fn load_from_user_code(project: &Project) -> anyhow::Result<Self> {
2583+
pub async fn load_from_user_code(
2584+
project: &Project,
2585+
resolve_credentials: bool,
2586+
) -> anyhow::Result<Self> {
24792587
let partial = if project.language == SupportedLanguages::Typescript {
24802588
let process = crate::framework::typescript::export_collectors::collect_from_index(
24812589
project,
@@ -2486,12 +2594,19 @@ impl InfrastructureMap {
24862594
} else {
24872595
load_main_py(project, &project.project_location).await?
24882596
};
2489-
let infra_map = partial.into_infra_map(
2597+
let mut infra_map = partial.into_infra_map(
24902598
project.language,
24912599
&project.main_file(),
24922600
&project.clickhouse_config.db_name,
24932601
)?;
24942602

2603+
// Resolve S3 credentials at runtime if requested
2604+
if resolve_credentials {
2605+
infra_map
2606+
.resolve_s3_credentials_from_env()
2607+
.map_err(|e| anyhow::anyhow!("Failed to resolve S3 credentials: {}", e))?;
2608+
}
2609+
24952610
// Provide explicit feedback when streams are defined but streaming engine is disabled
24962611
if !project.features.streaming_engine && infra_map.uses_streaming() {
24972612
show_message_wrapper(

apps/framework-cli/src/framework/core/partial_infrastructure_map.rs

Lines changed: 7 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ use crate::{
6565
scripts::Workflow, versions::Version,
6666
},
6767
infrastructure::olap::clickhouse::queries::ClickhouseEngine,
68-
utilities::{constants, secrets::resolve_optional_runtime_env},
68+
utilities::constants,
6969
};
7070

7171
/// Defines how Moose manages the lifecycle of database resources when code changes.
@@ -455,34 +455,6 @@ pub struct PartialInfrastructureMap {
455455
web_apps: HashMap<String, PartialWebApp>,
456456
}
457457

458-
/// Helper function to resolve S3 credentials from environment variables
459-
/// This ensures consistent credential handling between S3 and S3Queue engines
460-
fn resolve_s3_credentials(
461-
aws_access_key_id: &Option<String>,
462-
aws_secret_access_key: &Option<String>,
463-
table_name: &str,
464-
) -> Result<(Option<String>, Option<String>), DmV2LoadingError> {
465-
// Resolve environment variable markers for AWS credentials at runtime
466-
// This must happen before the infrastructure diff to support credential rotation
467-
let resolved_access_key = resolve_optional_runtime_env(aws_access_key_id).map_err(|e| {
468-
DmV2LoadingError::RuntimeEnvResolution {
469-
table_name: table_name.to_string(),
470-
field: "awsAccessKeyId".to_string(),
471-
error: e.to_string(),
472-
}
473-
})?;
474-
475-
let resolved_secret_key = resolve_optional_runtime_env(aws_secret_access_key).map_err(|e| {
476-
DmV2LoadingError::RuntimeEnvResolution {
477-
table_name: table_name.to_string(),
478-
field: "awsSecretAccessKey".to_string(),
479-
error: e.to_string(),
480-
}
481-
})?;
482-
483-
Ok((resolved_access_key, resolved_secret_key))
484-
}
485-
486458
impl PartialInfrastructureMap {
487459
/// Creates a new [`PartialInfrastructureMap`] by executing and reading from a subprocess.
488460
///
@@ -802,37 +774,25 @@ impl PartialInfrastructureMap {
802774
})),
803775

804776
Some(EngineConfig::S3Queue(config)) => {
805-
// Resolve S3 credentials using shared helper
806-
let (resolved_access_key, resolved_secret_key) = resolve_s3_credentials(
807-
&config.aws_access_key_id,
808-
&config.aws_secret_access_key,
809-
&partial_table.name,
810-
)?;
811-
777+
// Keep environment variable markers as-is - credentials will be resolved at runtime
812778
// S3Queue settings are handled in table_settings, not in the engine
813779
Ok(Some(ClickhouseEngine::S3Queue {
814780
s3_path: config.s3_path.clone(),
815781
format: config.format.clone(),
816782
compression: config.compression.clone(),
817783
headers: config.headers.clone(),
818-
aws_access_key_id: resolved_access_key,
819-
aws_secret_access_key: resolved_secret_key,
784+
aws_access_key_id: config.aws_access_key_id.clone(),
785+
aws_secret_access_key: config.aws_secret_access_key.clone(),
820786
}))
821787
}
822788

823789
Some(EngineConfig::S3(config)) => {
824-
// Resolve S3 credentials using shared helper
825-
let (resolved_access_key, resolved_secret_key) = resolve_s3_credentials(
826-
&config.aws_access_key_id,
827-
&config.aws_secret_access_key,
828-
&partial_table.name,
829-
)?;
830-
790+
// Keep environment variable markers as-is - credentials will be resolved at runtime
831791
Ok(Some(ClickhouseEngine::S3 {
832792
path: config.path.clone(),
833793
format: config.format.clone(),
834-
aws_access_key_id: resolved_access_key,
835-
aws_secret_access_key: resolved_secret_key,
794+
aws_access_key_id: config.aws_access_key_id.clone(),
795+
aws_secret_access_key: config.aws_secret_access_key.clone(),
836796
compression: config.compression.clone(),
837797
partition_strategy: config.partition_strategy.clone(),
838798
partition_columns_in_data_file: config.partition_columns_in_data_file.clone(),

0 commit comments

Comments
 (0)