Skip to content

Commit

Permalink
Use real identity for finish_push (#29479)
Browse files Browse the repository at this point in the history
This fixes a bug where deployment audit log events for pushes with components don't include the member.

GitOrigin-RevId: fb37b284376a2a19910e24b8f38563a33dc1c0ca
  • Loading branch information
emmaling27 authored and Convex, Inc. committed Sep 3, 2024
1 parent 11043de commit 930ecab
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 5 deletions.
4 changes: 2 additions & 2 deletions crates/application/src/deploy_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ impl<RT: Runtime> Application<RT> {
#[minitrace::trace]
pub async fn finish_push(
&self,
identity: Identity,
start_push: StartPushResponse,
dry_run: bool,
) -> anyhow::Result<FinishPushDiff> {
Expand All @@ -492,8 +493,7 @@ impl<RT: Runtime> Application<RT> {
downloaded_source_packages.insert(definition_path.clone(), package);
}

// TODO: We require system identity for creating system tables.
let mut tx = self.begin(Identity::system()).await?;
let mut tx = self.begin(identity.clone()).await?;

// Validate that environment variables haven't changed since `start_push`.
let environment_variables = EnvironmentVariablesModel::new(&mut tx).get_all().await?;
Expand Down
3 changes: 2 additions & 1 deletion crates/application/src/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ impl<RT: Runtime> ApplicationTestExt<RT> for Application<RT> {
_ => anyhow::bail!("Unexpected schema status: {schema_status:?}"),
}
}
self.finish_push(start_push, false).await?;
self.finish_push(Identity::system(), start_push, false)
.await?;
Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions crates/local_backend/src/deploy_config2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ pub async fn finish_push(
State(st): State<LocalAppState>,
Json(req): Json<FinishPushRequest>,
) -> Result<impl IntoResponse, HttpResponseError> {
let _identity = must_be_admin_from_key_with_write_access(
let identity = must_be_admin_from_key_with_write_access(
st.application.app_auth(),
st.instance_name.clone(),
req.admin_key.clone(),
Expand All @@ -246,7 +246,7 @@ pub async fn finish_push(
let start_push = StartPushResponse::try_from(req.start_push)?;
let resp = st
.application
.finish_push(start_push, dry_run)
.finish_push(identity, start_push, dry_run)
.await
.map_err(|e| e.wrap_error_message(|msg| format!("Hit an error while pushing:\n{msg}")))?;
Ok(Json(SerializedFinishPushDiff::try_from(resp)?))
Expand Down

0 comments on commit 930ecab

Please sign in to comment.