From ba214709b94f9467738e66b016331e97ac7d5d10 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 3 Apr 2026 14:17:27 -0400 Subject: [PATCH] fix: add secretKeyRef support for jwt_secret and rsa_keys (#8698) * feat: add secretKeyRef support for jwt_secret and extra fields (rsa_keys) Co-Authored-By: Claude Opus 4.5 * chore: update ee-repo-ref.txt Co-Authored-By: Claude Opus 4.5 * chore: update ee-repo-ref to 2c24cf597fdf8c4dccd483f1f1e5c49eb42ef3a3 This commit updates the EE repository reference after PR #508 was merged in windmill-ee-private. Previous ee-repo-ref: ade3bb76f8e0a6e658313b54c7180577fc9efc37 New ee-repo-ref: 2c24cf597fdf8c4dccd483f1f1e5c49eb42ef3a3 Automated by sync-ee-ref workflow. * test: replace unit tests with integration tests for jwt_secret and rsa_keys Co-Authored-By: Claude Opus 4.5 * chore: update ee-repo-ref.txt Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: Claude Opus 4.5 Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- backend/tests/instance_config.rs | 171 ++++++++++++++++-- backend/tests/operator_db_sync.rs | 85 +++++++++ .../windmill-common/src/instance_config.rs | 2 +- 4 files changed, 245 insertions(+), 15 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 0ff64f2b50..af52d4bf1d 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -9422b189762ae27edfc346541ae668a4ad728325 +8620654a01c5997137772572822690252143985d diff --git a/backend/tests/instance_config.rs b/backend/tests/instance_config.rs index cc0b8bae2c..afd414b0c3 100644 --- a/backend/tests/instance_config.rs +++ b/backend/tests/instance_config.rs @@ -240,7 +240,10 @@ async fn test_from_db_worker_config_prefix_stripping(db: Pool) { config.worker_configs.contains_key("my_group_name"), "worker__ prefix should be stripped" ); - assert_eq!(config.worker_configs["my_group_name"].extra["cache_clear"], serde_json::json!(5)); + assert_eq!( + config.worker_configs["my_group_name"].extra["cache_clear"], + serde_json::json!(5) + ); } #[sqlx::test(fixtures("base"))] @@ -400,12 +403,11 @@ async fn test_apply_settings_diff_complex_json(db: Pool) { #[sqlx::test(fixtures("base"))] async fn test_apply_settings_diff_delete_nonexistent_is_noop(db: Pool) { - let diff = - SettingsDiff { - upserts: BTreeMap::new(), - deletes: vec!["does_not_exist".to_string()], - ..Default::default() - }; + let diff = SettingsDiff { + upserts: BTreeMap::new(), + deletes: vec!["does_not_exist".to_string()], + ..Default::default() + }; // Should not error apply_settings_diff(&db, &diff).await.unwrap(); @@ -657,11 +659,8 @@ async fn test_roundtrip_to_settings_map_from_db_consistency(db: Pool) }; let map = original.to_settings_map(); - let diff = SettingsDiff { - upserts: map.into_iter().collect(), - deletes: vec![], - ..Default::default() - }; + let diff = + SettingsDiff { upserts: map.into_iter().collect(), deletes: vec![], ..Default::default() }; apply_settings_diff(&db, &diff).await.unwrap(); @@ -866,7 +865,10 @@ async fn test_full_config_roundtrip(db: Pool) { assert_eq!(otel.tracing_enabled, Some(true)); assert_eq!(config.worker_configs.len(), 2); - assert_eq!(config.worker_configs["default"].extra["cache_clear"], serde_json::json!(7)); + assert_eq!( + config.worker_configs["default"].extra["cache_clear"], + serde_json::json!(7) + ); let gpu_auto = config.worker_configs["gpu"].autoscaling.as_ref().unwrap(); assert!(gpu_auto.enabled); assert_eq!(gpu_auto.min_workers, Some(0)); @@ -1006,3 +1008,146 @@ async fn test_replace_mode_protects_settings_in_integration(db: Pool) Some(serde_json::json!("yes")) ); } + +// ======================================================================== +// jwt_secret and rsa_keys declarative roundtrip +// ======================================================================== + +#[sqlx::test(fixtures("base"))] +async fn test_jwt_secret_roundtrip_as_string_or_secret_ref(db: Pool) { + clear_settings_and_configs(&db).await; + + // Simulate what the operator does: parse a GlobalSettings with jwt_secret + // as a resolved StringOrSecretRef::Literal, write to DB, read back. + let settings = windmill_common::instance_config::GlobalSettings { + jwt_secret: Some( + windmill_common::instance_config::StringOrSecretRef::Literal( + "my-jwt-secret-from-k8s".to_string(), + ), + ), + ..Default::default() + }; + + let map = settings.to_settings_map(); + + // The serialized form should be a plain JSON string (not an object) + assert_eq!( + map["jwt_secret"], + serde_json::json!("my-jwt-secret-from-k8s") + ); + + let diff = + SettingsDiff { upserts: map.into_iter().collect(), deletes: vec![], ..Default::default() }; + apply_settings_diff(&db, &diff).await.unwrap(); + + // Read back from DB — jwt_secret should survive the roundtrip + let config = InstanceConfig::from_db(&db).await.unwrap(); + assert_eq!( + config + .global_settings + .jwt_secret + .as_ref() + .and_then(|v| v.as_literal()), + Some("my-jwt-secret-from-k8s") + ); + + // Verify the raw DB value is a plain string (not wrapped in an object) + let raw = get_global_setting(&db, "jwt_secret").await.unwrap(); + assert!( + raw.is_string(), + "jwt_secret in DB should be a plain JSON string" + ); + assert_eq!(raw.as_str().unwrap(), "my-jwt-secret-from-k8s"); +} + +#[sqlx::test(fixtures("base"))] +async fn test_rsa_keys_roundtrip_via_extra(db: Pool) { + clear_settings_and_configs(&db).await; + + // rsa_keys is not a typed field — it flows through GlobalSettings.extra. + // Simulate a resolved secretKeyRef: the operator resolves the ref and + // writes the plain value to extra before syncing to DB. + let json_str = r#"{ + "rsa_keys": { + "private_key": "-----BEGIN RSA PRIVATE KEY-----\ntest-key-data\n-----END RSA PRIVATE KEY-----" + } + }"#; + let settings: windmill_common::instance_config::GlobalSettings = + serde_json::from_str(json_str).unwrap(); + + // rsa_keys should land in extra + assert!(settings.extra.contains_key("rsa_keys")); + + let map = settings.to_settings_map(); + let diff = + SettingsDiff { upserts: map.into_iter().collect(), deletes: vec![], ..Default::default() }; + apply_settings_diff(&db, &diff).await.unwrap(); + + // Read back from DB + let config = InstanceConfig::from_db(&db).await.unwrap(); + assert_eq!( + config.global_settings.extra["rsa_keys"]["private_key"], + "-----BEGIN RSA PRIVATE KEY-----\ntest-key-data\n-----END RSA PRIVATE KEY-----" + ); + + // Verify the raw DB value is a JSON object with private_key + let raw = get_global_setting(&db, "rsa_keys").await.unwrap(); + assert!(raw.is_object()); + assert_eq!( + raw["private_key"].as_str().unwrap(), + "-----BEGIN RSA PRIVATE KEY-----\ntest-key-data\n-----END RSA PRIVATE KEY-----" + ); +} + +#[sqlx::test(fixtures("base"))] +async fn test_replace_mode_protects_jwt_secret_and_rsa_keys(db: Pool) { + clear_settings_and_configs(&db).await; + + // Seed jwt_secret and rsa_keys (both are PROTECTED_SETTINGS) + insert_global_setting(&db, "jwt_secret", serde_json::json!("existing-secret")).await; + insert_global_setting( + &db, + "rsa_keys", + serde_json::json!({"private_key": "existing-rsa-key"}), + ) + .await; + insert_global_setting(&db, "normal_setting", serde_json::json!("will-go")).await; + + let config = InstanceConfig::from_db(&db).await.unwrap(); + let current_map = config.global_settings.to_settings_map(); + + // Desired state: only base_url — jwt_secret and rsa_keys should survive + let mut desired_map = BTreeMap::new(); + desired_map.insert( + "base_url".to_string(), + serde_json::json!("https://example.com"), + ); + + let diff = diff_global_settings(¤t_map, &desired_map, ApplyMode::Replace); + + assert!( + !diff.deletes.contains(&"jwt_secret".to_string()), + "jwt_secret is protected from deletion" + ); + assert!( + !diff.deletes.contains(&"rsa_keys".to_string()), + "rsa_keys is protected from deletion" + ); + assert!( + diff.deletes.contains(&"normal_setting".to_string()), + "normal_setting should be deleted" + ); + + apply_settings_diff(&db, &diff).await.unwrap(); + + // Both protected settings survive + assert_eq!( + get_global_setting(&db, "jwt_secret").await, + Some(serde_json::json!("existing-secret")) + ); + assert_eq!( + get_global_setting(&db, "rsa_keys").await, + Some(serde_json::json!({"private_key": "existing-rsa-key"})) + ); + assert!(get_global_setting(&db, "normal_setting").await.is_none()); +} diff --git a/backend/tests/operator_db_sync.rs b/backend/tests/operator_db_sync.rs index fb7a9e1e39..5460039b57 100644 --- a/backend/tests/operator_db_sync.rs +++ b/backend/tests/operator_db_sync.rs @@ -445,4 +445,89 @@ mod tests { Some(serde_json::json!("value")) ); } + + // ======================================================================== + // jwt_secret and rsa_keys via sync_global_settings + // ======================================================================== + + #[sqlx::test(fixtures("base"))] + async fn test_sync_jwt_secret_via_settings_map(db: Pool) { + // Simulate the full operator pipeline: parse GlobalSettings with a + // resolved jwt_secret (StringOrSecretRef::Literal), convert to + // settings map, and sync to DB. + let settings = windmill_common::instance_config::GlobalSettings { + jwt_secret: Some( + windmill_common::instance_config::StringOrSecretRef::Literal( + "resolved-jwt-value".to_string(), + ), + ), + ..Default::default() + }; + + let map = settings.to_settings_map(); + windmill_operator::db_sync::sync_global_settings(&db, &map) + .await + .expect("sync should succeed"); + + let stored = get_global_setting(&db, "jwt_secret") + .await + .expect("jwt_secret should exist"); + assert_eq!(stored, serde_json::json!("resolved-jwt-value")); + } + + #[sqlx::test(fixtures("base"))] + async fn test_sync_rsa_keys_via_settings_map(db: Pool) { + // rsa_keys flows through GlobalSettings.extra as opaque JSON. + // After operator resolves secretKeyRef, the value is a plain JSON + // object that gets synced to DB. + let json_str = r#"{ + "rsa_keys": { + "private_key": "-----BEGIN RSA PRIVATE KEY-----\ntest\n-----END RSA PRIVATE KEY-----" + } + }"#; + let settings: windmill_common::instance_config::GlobalSettings = + serde_json::from_str(json_str).unwrap(); + + let map = settings.to_settings_map(); + windmill_operator::db_sync::sync_global_settings(&db, &map) + .await + .expect("sync should succeed"); + + let stored = get_global_setting(&db, "rsa_keys") + .await + .expect("rsa_keys should exist"); + assert_eq!( + stored["private_key"], + "-----BEGIN RSA PRIVATE KEY-----\ntest\n-----END RSA PRIVATE KEY-----" + ); + } + + #[sqlx::test(fixtures("base"))] + async fn test_sync_protects_jwt_secret_and_rsa_keys(db: Pool) { + // Pre-populate both protected settings + insert_global_setting(&db, "jwt_secret", serde_json::json!("original-jwt")).await; + insert_global_setting( + &db, + "rsa_keys", + serde_json::json!({"private_key": "original-rsa"}), + ) + .await; + + // Sync with empty desired — protected keys should survive + let desired = BTreeMap::new(); + windmill_operator::db_sync::sync_global_settings(&db, &desired) + .await + .expect("sync should succeed"); + + assert_eq!( + get_global_setting(&db, "jwt_secret").await, + Some(serde_json::json!("original-jwt")), + "jwt_secret is protected and should survive empty sync" + ); + assert_eq!( + get_global_setting(&db, "rsa_keys").await, + Some(serde_json::json!({"private_key": "original-rsa"})), + "rsa_keys is protected and should survive empty sync" + ); + } } diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index ac1fe24a4e..3933724b8a 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -259,7 +259,7 @@ pub struct GlobalSettings { #[serde(skip_serializing_if = "Option::is_none")] pub hub_api_secret: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub jwt_secret: Option, + pub jwt_secret: Option, #[serde(skip_serializing_if = "Option::is_none")] pub scim_token: Option, #[serde(skip_serializing_if = "Option::is_none")]