Skip to content

Commit 79eae79

Browse files
johntmyersjohntmyers
committed
fix(providers): resolve refresh rebase fallout
1 parent 74b146f commit 79eae79

3 files changed

Lines changed: 47 additions & 2 deletions

File tree

‎crates/openshell-server/src/grpc/provider.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,7 @@ pub(super) async fn handle_configure_provider_refresh(
13241324
name: provider_name.to_string(),
13251325
created_at_ms: 0,
13261326
labels: std::collections::HashMap::new(),
1327+
resource_version: 0,
13271328
}),
13281329
r#type: String::new(),
13291330
credentials: std::collections::HashMap::new(),
@@ -1418,6 +1419,7 @@ pub(super) async fn handle_delete_provider_refresh(
14181419
name: provider_name.to_string(),
14191420
created_at_ms: 0,
14201421
labels: std::collections::HashMap::new(),
1422+
resource_version: 0,
14211423
}),
14221424
r#type: String::new(),
14231425
credentials: std::collections::HashMap::new(),
@@ -2110,6 +2112,7 @@ mod tests {
21102112
name: "msgraph".to_string(),
21112113
created_at_ms: 0,
21122114
labels: HashMap::new(),
2115+
resource_version: 0,
21132116
}),
21142117
r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(),
21152118
credentials: std::iter::once((
@@ -2222,6 +2225,7 @@ mod tests {
22222225
name: "msgraph".to_string(),
22232226
created_at_ms: 0,
22242227
labels: HashMap::new(),
2228+
resource_version: 0,
22252229
}),
22262230
r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(),
22272231
credentials: std::iter::once((
@@ -2264,6 +2268,7 @@ mod tests {
22642268
name: "msgraph".to_string(),
22652269
created_at_ms: 0,
22662270
labels: HashMap::new(),
2271+
resource_version: 0,
22672272
}),
22682273
r#type: String::new(),
22692274
credentials: HashMap::new(),
@@ -2315,6 +2320,7 @@ mod tests {
23152320
name: "existing-graph".to_string(),
23162321
created_at_ms: 0,
23172322
labels: HashMap::new(),
2323+
resource_version: 0,
23182324
}),
23192325
r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(),
23202326
credentials: std::iter::once((
@@ -2336,6 +2342,7 @@ mod tests {
23362342
name: "refreshing-graph".to_string(),
23372343
created_at_ms: 0,
23382344
labels: HashMap::new(),
2345+
resource_version: 0,
23392346
}),
23402347
r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(),
23412348
credentials: std::iter::once(("OTHER_TOKEN".to_string(), "other".to_string()))
@@ -2354,6 +2361,7 @@ mod tests {
23542361
name: "collision".to_string(),
23552362
created_at_ms: 0,
23562363
labels: HashMap::new(),
2364+
resource_version: 0,
23572365
}),
23582366
spec: Some(SandboxSpec {
23592367
providers: vec!["existing-graph".to_string(), "refreshing-graph".to_string()],
@@ -2404,6 +2412,7 @@ mod tests {
24042412
name: name.to_string(),
24052413
created_at_ms: 0,
24062414
labels: HashMap::new(),
2415+
resource_version: 0,
24072416
}),
24082417
r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(),
24092418
credentials: HashMap::new(),
@@ -2422,6 +2431,7 @@ mod tests {
24222431
name: "refresh-collision".to_string(),
24232432
created_at_ms: 0,
24242433
labels: HashMap::new(),
2434+
resource_version: 0,
24252435
}),
24262436
spec: Some(SandboxSpec {
24272437
providers: vec!["first-graph".to_string(), "second-graph".to_string()],
@@ -2487,6 +2497,7 @@ mod tests {
24872497
name: "msgraph".to_string(),
24882498
created_at_ms: 0,
24892499
labels: HashMap::new(),
2500+
resource_version: 0,
24902501
}),
24912502
r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(),
24922503
credentials: std::iter::once((
@@ -2553,6 +2564,7 @@ mod tests {
25532564
name: "msgraph".to_string(),
25542565
created_at_ms: 0,
25552566
labels: HashMap::new(),
2567+
resource_version: 0,
25562568
}),
25572569
r#type: "outlook".to_string(),
25582570
credentials: std::iter::once((
@@ -2849,6 +2861,7 @@ mod tests {
28492861
))
28502862
.collect(),
28512863
config: HashMap::new(),
2864+
credential_expires_at_ms: HashMap::new(),
28522865
},
28532866
)
28542867
.await
@@ -2877,6 +2890,7 @@ mod tests {
28772890
))
28782891
.collect(),
28792892
config: HashMap::new(),
2893+
credential_expires_at_ms: HashMap::new(),
28802894
},
28812895
)
28822896
.await
@@ -2921,6 +2935,7 @@ mod tests {
29212935
name: "gitlab-no-creds".to_string(),
29222936
created_at_ms: 1_000_000,
29232937
labels: HashMap::new(),
2938+
resource_version: 0,
29242939
}),
29252940
r#type: "gitlab".to_string(),
29262941
credentials: HashMap::new(),
@@ -2990,6 +3005,7 @@ mod tests {
29903005
name: "delegated-refresh-no-token-yet".to_string(),
29913006
created_at_ms: 1_000_000,
29923007
labels: HashMap::new(),
3008+
resource_version: 0,
29933009
}),
29943010
r#type: "delegated-refresh-api".to_string(),
29953011
credentials: HashMap::new(),
@@ -3025,6 +3041,7 @@ mod tests {
30253041
name: "mixed-required-no-token-yet".to_string(),
30263042
created_at_ms: 1_000_000,
30273043
labels: HashMap::new(),
3044+
resource_version: 0,
30283045
}),
30293046
r#type: "mixed-required-api".to_string(),
30303047
credentials: HashMap::new(),
@@ -3060,6 +3077,7 @@ mod tests {
30603077
name: "optional-static-no-token-yet".to_string(),
30613078
created_at_ms: 1_000_000,
30623079
labels: HashMap::new(),
3080+
resource_version: 0,
30633081
}),
30643082
r#type: "optional-static-api".to_string(),
30653083
credentials: HashMap::new(),
@@ -3347,6 +3365,7 @@ mod tests {
33473365
name: "expiring-provider".to_string(),
33483366
created_at_ms: 0,
33493367
labels: HashMap::new(),
3368+
resource_version: 0,
33503369
}),
33513370
r#type: "test".to_string(),
33523371
credentials: [
@@ -3541,6 +3560,7 @@ mod tests {
35413560
name: "provider-a".to_string(),
35423561
created_at_ms: 0,
35433562
labels: HashMap::new(),
3563+
resource_version: 0,
35443564
}),
35453565
r#type: "outlook".to_string(),
35463566
credentials: std::iter::once((
@@ -3562,6 +3582,7 @@ mod tests {
35623582
name: "provider-b".to_string(),
35633583
created_at_ms: 0,
35643584
labels: HashMap::new(),
3585+
resource_version: 0,
35653586
}),
35663587
r#type: "google-drive".to_string(),
35673588
credentials: std::iter::once((
@@ -3581,6 +3602,7 @@ mod tests {
35813602
name: "collision".to_string(),
35823603
created_at_ms: 0,
35833604
labels: HashMap::new(),
3605+
resource_version: 0,
35843606
}),
35853607
spec: Some(SandboxSpec {
35863608
providers: vec!["provider-a".to_string(), "provider-b".to_string()],
@@ -3598,6 +3620,7 @@ mod tests {
35983620
name: "provider-b".to_string(),
35993621
created_at_ms: 0,
36003622
labels: HashMap::new(),
3623+
resource_version: 0,
36013624
}),
36023625
r#type: String::new(),
36033626
credentials: std::iter::once((
@@ -3742,6 +3765,7 @@ mod tests {
37423765
r#type: String::new(), // Empty type is ignored in update
37433766
credentials: HashMap::new(),
37443767
config: HashMap::new(),
3768+
credential_expires_at_ms: HashMap::new(),
37453769
};
37463770

37473771
// Attempt to update with an oversized credential key (exceeds MAX_MAP_KEY_LEN)
@@ -3893,6 +3917,7 @@ mod tests {
38933917
&state,
38943918
Request::new(UpdateProviderRequest {
38953919
provider: Some(updated_provider.clone()),
3920+
credential_expires_at_ms: HashMap::new(),
38963921
}),
38973922
)
38983923
.await
@@ -3960,6 +3985,7 @@ mod tests {
39603985
&state,
39613986
Request::new(UpdateProviderRequest {
39623987
provider: Some(stale_provider),
3988+
credential_expires_at_ms: HashMap::new(),
39633989
}),
39643990
)
39653991
.await
@@ -4029,6 +4055,7 @@ mod tests {
40294055
&state_clone,
40304056
Request::new(UpdateProviderRequest {
40314057
provider: Some(updated),
4058+
credential_expires_at_ms: HashMap::new(),
40324059
}),
40334060
)
40344061
.await

‎crates/openshell-server/src/grpc/sandbox.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ use tracing::{debug, info, warn};
3838
use russh::ChannelMsg;
3939
use russh::client::AuthResult;
4040

41-
use super::provider::{get_provider_record, is_valid_env_key};
41+
use super::provider::{
42+
get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique,
43+
};
4244
use super::validation::{
4345
level_matches, source_matches, validate_exec_request_fields, validate_policy_safety,
4446
validate_sandbox_spec,
@@ -80,6 +82,7 @@ pub(super) async fn handle_create_sandbox(
8082
.map_err(|e| Status::internal(format!("fetch provider failed: {e}")))?
8183
.ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?;
8284
}
85+
validate_provider_environment_keys_unique(state.store.as_ref(), &spec.providers).await?;
8386

8487
// Ensure the template always carries the resolved image.
8588
let mut spec = spec;

‎crates/openshell-server/src/provider_refresh.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ pub fn new_refresh_state(
199199
name: refresh_state_name(&provider_id, credential_key),
200200
created_at_ms: now_ms,
201201
labels: HashMap::new(),
202+
resource_version: 0,
202203
}),
203204
provider_id,
204205
provider_name,
@@ -416,8 +417,20 @@ async fn apply_minted_credential(
416417
crate::grpc::provider::validate_provider_update_against_attached_sandboxes(store, &updated)
417418
.await?;
418419
store
419-
.put_message(&updated)
420+
.update_message_cas::<Provider, _>(provider.object_id(), 0, |current| {
421+
current
422+
.credentials
423+
.insert(credential_key.to_string(), minted.access_token.clone());
424+
if minted.expires_at_ms > 0 {
425+
current
426+
.credential_expires_at_ms
427+
.insert(credential_key.to_string(), minted.expires_at_ms);
428+
} else {
429+
current.credential_expires_at_ms.remove(credential_key);
430+
}
431+
})
420432
.await
433+
.map(|_| ())
421434
.map_err(|e| Status::internal(format!("persist refreshed provider credential failed: {e}")))
422435
}
423436

@@ -914,6 +927,7 @@ mod tests {
914927
name: "collision".to_string(),
915928
created_at_ms: 1,
916929
labels: HashMap::new(),
930+
resource_version: 0,
917931
}),
918932
spec: Some(SandboxSpec {
919933
providers: vec!["existing-graph".to_string(), "refreshing-graph".to_string()],
@@ -1167,6 +1181,7 @@ mod tests {
11671181
name: name.to_string(),
11681182
created_at_ms: 1,
11691183
labels: HashMap::new(),
1184+
resource_version: 0,
11701185
}),
11711186
r#type: provider_type.to_string(),
11721187
credentials: HashMap::new(),

0 commit comments

Comments
 (0)