Skip to content

Commit 62e0845

Browse files
derekwaynecarrderekwaynecarr
committed
refactor(proto)!: use well-known time types
Signed-off-by: Derek Carr <[email protected]>
1 parent 4eaa105 commit 62e0845

110 files changed

Lines changed: 3966 additions & 2299 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎architecture/gateway.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,14 @@ This keeps the gateway data model portable across storage backends and leaves
336336
room for future stores that can provide the same object, label, version, and
337337
scope semantics.
338338

339+
Public protobuf APIs represent absolute times with `google.protobuf.Timestamp`
340+
and elapsed time with `google.protobuf.Duration`. The integer
341+
`created_at_ms` and `updated_at_ms` database columns are intentionally internal
342+
bookkeeping values, not part of that public convention. On startup, both
343+
storage backends transactionally rewrite legacy scalar time fields inside
344+
protobuf payloads before serving requests. A malformed affected payload aborts
345+
and rolls back startup migration rather than silently dropping a value.
346+
339347
The SQLite adapter tightens the on-disk database file to mode `0o600` on every
340348
connect so that provider API keys, SSH session tokens, and sandbox metadata are
341349
not readable by other local users on shared hosts. The same restriction is

‎crates/openshell-cli/src/run.rs‎

Lines changed: 169 additions & 90 deletions
Large diffs are not rendered by default.

‎crates/openshell-cli/tests/ensure_providers_integration.rs‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,17 @@ impl TestOpenShell {
6161
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
6262
id: format!("id-{name}"),
6363
name: name.to_string(),
64-
created_at_ms: 0,
64+
created_time: None,
6565
labels: HashMap::new(),
6666
resource_version: 0,
6767
annotations: HashMap::new(),
6868
workspace: String::new(),
69-
deletion_timestamp_ms: 0,
69+
deletion_time: None,
7070
}),
7171
r#type: provider_type.to_string(),
7272
credentials: HashMap::new(),
7373
config: HashMap::new(),
74-
credential_expires_at_ms: HashMap::new(),
74+
credential_expiration_times: HashMap::new(),
7575
profile_workspace: "default".to_string(),
7676
credential_handles: HashMap::new(),
7777
},
@@ -382,38 +382,34 @@ impl OpenShell for TestOpenShell {
382382
}
383383
base
384384
};
385-
let merge_expiry = |mut base: HashMap<String, i64>, incoming: HashMap<String, i64>| {
386-
if incoming.is_empty() {
387-
return base;
388-
}
389-
for (k, v) in incoming {
390-
if v <= 0 {
391-
base.remove(&k);
392-
} else {
393-
base.insert(k, v);
385+
let merge_expiry =
386+
|mut base: HashMap<String, prost_types::Timestamp>,
387+
incoming: HashMap<String, prost_types::Timestamp>| {
388+
if incoming.is_empty() {
389+
return base;
394390
}
395-
}
396-
base
397-
};
391+
base.extend(incoming);
392+
base
393+
};
398394
let existing_metadata = existing.metadata.clone().unwrap_or_default();
399395
let provider_metadata = provider.metadata.clone().unwrap_or_default();
400396
let updated = Provider {
401397
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
402398
id: existing_metadata.id,
403399
name: provider_metadata.name,
404-
created_at_ms: existing_metadata.created_at_ms,
400+
created_time: existing_metadata.created_time,
405401
labels: existing_metadata.labels,
406402
resource_version: 0,
407403
annotations: HashMap::new(),
408404
workspace: String::new(),
409-
deletion_timestamp_ms: 0,
405+
deletion_time: None,
410406
}),
411407
r#type: existing.r#type,
412408
credentials: merge(existing.credentials, provider.credentials),
413409
config: merge(existing.config, provider.config),
414-
credential_expires_at_ms: merge_expiry(
415-
existing.credential_expires_at_ms,
416-
provider.credential_expires_at_ms,
410+
credential_expiration_times: merge_expiry(
411+
existing.credential_expiration_times,
412+
provider.credential_expiration_times,
417413
),
418414
profile_workspace: existing.profile_workspace,
419415
credential_handles: if provider.credential_handles.is_empty() {

‎crates/openshell-cli/tests/provider_commands_integration.rs‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,12 @@ impl OpenShell for TestOpenShell {
172172
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
173173
id: format!("sb-{name}"),
174174
name,
175-
created_at_ms: 0,
175+
created_time: None,
176176
labels: HashMap::new(),
177177
resource_version: 1,
178178
annotations: HashMap::new(),
179179
workspace: String::new(),
180-
deletion_timestamp_ms: 0,
180+
deletion_time: None,
181181
}),
182182
spec: None,
183183
status: None,
@@ -651,38 +651,34 @@ impl OpenShell for TestOpenShell {
651651
}
652652
base
653653
};
654-
let merge_expiry = |mut base: HashMap<String, i64>, incoming: HashMap<String, i64>| {
655-
if incoming.is_empty() {
656-
return base;
657-
}
658-
for (k, v) in incoming {
659-
if v <= 0 {
660-
base.remove(&k);
661-
} else {
662-
base.insert(k, v);
654+
let merge_expiry =
655+
|mut base: HashMap<String, prost_types::Timestamp>,
656+
incoming: HashMap<String, prost_types::Timestamp>| {
657+
if incoming.is_empty() {
658+
return base;
663659
}
664-
}
665-
base
666-
};
660+
base.extend(incoming);
661+
base
662+
};
667663
let existing_metadata = existing.metadata.clone().unwrap_or_default();
668664
let provider_metadata = provider.metadata.clone().unwrap_or_default();
669665
let updated = Provider {
670666
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
671667
id: existing_metadata.id,
672668
name: provider_metadata.name,
673-
created_at_ms: existing_metadata.created_at_ms,
669+
created_time: existing_metadata.created_time,
674670
labels: existing_metadata.labels,
675671
resource_version: 0,
676672
annotations: HashMap::new(),
677673
workspace: String::new(),
678-
deletion_timestamp_ms: 0,
674+
deletion_time: None,
679675
}),
680676
r#type: existing.r#type,
681677
credentials: merge(existing.credentials, provider.credentials),
682678
config: merge(existing.config, provider.config),
683-
credential_expires_at_ms: merge_expiry(
684-
existing.credential_expires_at_ms,
685-
provider.credential_expires_at_ms,
679+
credential_expiration_times: merge_expiry(
680+
existing.credential_expiration_times,
681+
provider.credential_expiration_times,
686682
),
687683
profile_workspace: existing.profile_workspace,
688684
credential_handles: if provider.credential_handles.is_empty() {
@@ -743,7 +739,10 @@ impl OpenShell for TestOpenShell {
743739
credential_key: request.credential_key.clone(),
744740
material: request.material.clone(),
745741
secret_material_keys: request.secret_material_keys.clone(),
746-
expires_at_ms: request.expires_at_ms,
742+
expires_at_ms: request
743+
.expiration_time
744+
.as_ref()
745+
.and_then(|value| openshell_core::time::timestamp_to_millis(value).ok()),
747746
});
748747
let configure_failure = self
749748
.state
@@ -764,14 +763,14 @@ impl OpenShell for TestOpenShell {
764763
credential_key: request.credential_key.clone(),
765764
strategy: request.strategy,
766765
status: "configured".to_string(),
767-
expires_at_ms: request.expires_at_ms.unwrap_or_default(),
768-
next_refresh_at_ms: 0,
769-
last_refresh_at_ms: 0,
766+
expiration_time: request.expiration_time,
767+
next_refresh_time: None,
768+
last_refresh_time: None,
770769
last_error: String::new(),
771770
recovery_action: 0,
772771
failure_code: String::new(),
773772
provider_error_subtype: String::new(),
774-
last_error_at_ms: 0,
773+
last_error_time: None,
775774
};
776775
drop(providers);
777776
self.state
@@ -810,9 +809,9 @@ impl OpenShell for TestOpenShell {
810809
.get_mut(&(provider_name.clone(), credential_key.clone()))
811810
.ok_or_else(|| Status::not_found("provider refresh state not found"))?;
812811
status.status = "refreshed".to_string();
813-
status.last_refresh_at_ms = 1;
814-
status.next_refresh_at_ms = 3_600_000;
815-
status.expires_at_ms = 3_600_000;
812+
status.last_refresh_time = openshell_core::time::timestamp_from_millis(1).ok();
813+
status.next_refresh_time = openshell_core::time::timestamp_from_millis(3_600_000).ok();
814+
status.expiration_time = openshell_core::time::timestamp_from_millis(3_600_000).ok();
816815
let status = status.clone();
817816
drop(refresh_statuses);
818817
let mut providers = self.state.providers.lock().await;
@@ -822,9 +821,10 @@ impl OpenShell for TestOpenShell {
822821
provider
823822
.credentials
824823
.insert(credential_key.clone(), format!("minted-{credential_key}"));
825-
provider
826-
.credential_expires_at_ms
827-
.insert(credential_key, 3_600_000);
824+
provider.credential_expiration_times.insert(
825+
credential_key,
826+
openshell_core::time::timestamp_from_millis(3_600_000).unwrap(),
827+
);
828828
Ok(Response::new(RotateProviderCredentialResponse {
829829
status: Some(status),
830830
}))
@@ -2132,7 +2132,7 @@ async fn provider_update_from_existing_uses_profile_discovery_when_v2_enabled()
21322132
r#type: "custom-update-discovery".to_string(),
21332133
credentials: HashMap::new(),
21342134
config: HashMap::new(),
2135-
credential_expires_at_ms: HashMap::new(),
2135+
credential_expiration_times: HashMap::new(),
21362136
profile_workspace: "default".to_string(),
21372137
credential_handles: HashMap::new(),
21382138
},

‎crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,12 @@ impl OpenShell for TestOpenShell {
120120
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
121121
id: format!("id-{sandbox_name}"),
122122
name: sandbox_name,
123-
created_at_ms: 0,
123+
created_time: None,
124124
labels: HashMap::new(),
125125
resource_version: 0,
126126
annotations: HashMap::new(),
127127
workspace: String::new(),
128-
deletion_timestamp_ms: 0,
128+
deletion_time: None,
129129
}),
130130
..Sandbox::default()
131131
};
@@ -158,12 +158,12 @@ impl OpenShell for TestOpenShell {
158158
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
159159
id: format!("id-{name}"),
160160
name,
161-
created_at_ms: 0,
161+
created_time: None,
162162
labels: HashMap::new(),
163163
resource_version: 0,
164164
annotations: HashMap::new(),
165165
workspace: String::new(),
166-
deletion_timestamp_ms: 0,
166+
deletion_time: None,
167167
}),
168168
..Sandbox::default()
169169
};
@@ -449,12 +449,12 @@ impl OpenShell for TestOpenShell {
449449
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
450450
id: sandbox_id.clone(),
451451
name: sandbox_id.trim_start_matches("id-").to_string(),
452-
created_at_ms: 0,
452+
created_time: None,
453453
labels: HashMap::new(),
454454
resource_version: 0,
455455
annotations: HashMap::new(),
456456
workspace: String::new(),
457-
deletion_timestamp_ms: 0,
457+
deletion_time: None,
458458
}),
459459
..Sandbox::default()
460460
};
@@ -467,7 +467,7 @@ impl OpenShell for TestOpenShell {
467467
status: "False".to_string(),
468468
reason: "ProcessExited".to_string(),
469469
message: "VM process exited with status 0".to_string(),
470-
last_transition_time: String::new(),
470+
transition_time: None,
471471
}],
472472
..Default::default()
473473
}),
@@ -517,7 +517,7 @@ impl OpenShell for TestOpenShell {
517517
.send(Ok(SandboxStreamEvent {
518518
payload: Some(sandbox_stream_event::Payload::Log(SandboxLogLine {
519519
sandbox_id: sandbox_id.clone(),
520-
timestamp_ms: 0,
520+
event_time: None,
521521
level: "INFO".to_string(),
522522
target: "test".to_string(),
523523
message: message.to_string(),

‎crates/openshell-cli/tests/sandbox_name_fallback_integration.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,12 @@ impl OpenShell for TestOpenShell {
119119
metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta {
120120
id: "test-id".to_string(),
121121
name,
122-
created_at_ms: 0,
122+
created_time: None,
123123
labels: std::collections::HashMap::new(),
124124
resource_version: 0,
125125
annotations: std::collections::HashMap::new(),
126126
workspace: String::new(),
127-
deletion_timestamp_ms: 0,
127+
deletion_time: None,
128128
}),
129129
..Default::default()
130130
}),
@@ -473,8 +473,8 @@ impl OpenShell for TestOpenShell {
473473
version: 7,
474474
policy_hash: "sha256:test-policy".to_string(),
475475
status: PolicyStatus::Loaded.into(),
476-
created_at_ms: 1_700_000_000_000,
477-
loaded_at_ms: 1_700_000_000_500,
476+
created_time: openshell_core::time::timestamp_from_millis(1_700_000_000_000).ok(),
477+
loaded_time: openshell_core::time::timestamp_from_millis(1_700_000_000_500).ok(),
478478
policy: Some(policy),
479479
..Default::default()
480480
}),

‎crates/openshell-core/src/forward.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,7 @@ mod tests {
10931093
gateway_host: "gateway.example.com".to_string(),
10941094
gateway_port: 443,
10951095
host_key_fingerprint: String::new(),
1096-
expires_at_ms: 0,
1096+
expiration_time: None,
10971097
}
10981098
}
10991099

‎crates/openshell-core/src/grpc_client.rs‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ use crate::proto::{
3131
UpdateConfigRequest, inference_client::InferenceClient, open_shell_client::OpenShellClient,
3232
};
3333
use crate::sandbox_env;
34+
use crate::time::{duration_to_std, timestamp_to_millis};
3435
use miette::{IntoDiagnostic, Result, WrapErr};
3536
use openshell_extension_core::{BearerTokenSlot, ExtensionCredentialStore};
3637
use tonic::Status;
@@ -483,10 +484,11 @@ async fn refresh_extension_credentials_with_client(
483484
"gateway returned an unexpected or duplicate extension credential"
484485
));
485486
}
486-
validated.insert(
487-
credential.service_name,
488-
(credential.token, credential.expires_at_ms),
489-
);
487+
let expiration_time = credential.expiration_time.as_ref().ok_or_else(|| {
488+
miette::miette!("gateway returned an extension credential without an expiration time")
489+
})?;
490+
let expires_at_ms = timestamp_to_millis(expiration_time).into_diagnostic()?;
491+
validated.insert(credential.service_name, (credential.token, expires_at_ms));
490492
}
491493
if validated.len() != expected.len() {
492494
return Err(miette::miette!(
@@ -850,10 +852,19 @@ pub async fn fetch_provider_environment(
850852
.into_diagnostic()?;
851853

852854
let inner = response.into_inner();
855+
let credential_expires_at_ms = inner
856+
.credential_expiration_times
857+
.iter()
858+
.map(|(name, expiration_time)| {
859+
timestamp_to_millis(expiration_time)
860+
.map(|value| (name.clone(), value))
861+
.into_diagnostic()
862+
})
863+
.collect::<Result<HashMap<_, _>>>()?;
853864
Ok(ProviderEnvironmentResult {
854865
environment: inner.environment,
855866
provider_env_revision: inner.provider_env_revision,
856-
credential_expires_at_ms: inner.credential_expires_at_ms,
867+
credential_expires_at_ms,
857868
dynamic_credentials: inner.dynamic_credentials,
858869
static_credential_bindings: inner.static_credential_bindings,
859870
non_secret_environment_keys: inner.non_secret_environment_keys,
@@ -886,9 +897,18 @@ pub async fn exchange_provider_subject_token(
886897
.await
887898
.map_err(provider_subject_token_exchange_status)?;
888899
let inner = response.into_inner();
900+
let expires_in = inner
901+
.expires_after
902+
.as_ref()
903+
.map(duration_to_std)
904+
.transpose()
905+
.into_diagnostic()?
906+
.map_or(0, |value| {
907+
i64::try_from(value.as_secs()).unwrap_or(i64::MAX)
908+
});
889909
Ok(ProviderSubjectTokenExchangeResult {
890910
access_token: inner.access_token,
891-
expires_in: inner.expires_in,
911+
expires_in,
892912
token_type: inner.token_type,
893913
})
894914
}

0 commit comments

Comments
 (0)