Skip to content

fixup: give required presigned headers directly to the client #334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cole-h wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fixup: give required presigned headers directly to the client #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/flakehub_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::HashMap;

use color_eyre::eyre::{eyre, Context, Result};
use http::StatusCode;
use reqwest::header::HeaderMap;
Expand All @@ -20,6 +22,7 @@ pub struct Tarball {
#[derive(serde::Deserialize)]
pub(crate) struct StageResult {
pub(crate) s3_upload_url: String,
pub(crate) s3_upload_headers: HashMap<String, String>,

@coderabbitai coderabbitai Bot Jul 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file and nearby definitions.
wc -l src/flakehub_client.rs
cat -n src/flakehub_client.rs | sed -n '1,220p'

# Find all serde-related usage for the type and response path.
rg -n "s3_upload_headers|Decoding release metadata POST response|serde(default)|Deserialize|response\.json|flakehub_client" src

Repository: DeterminateSystems/flakehub-push

Length of output: 6044

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the response handling and the release stage flow.
cat -n src/main.rs | sed -n '130,220p'

# Check whether StageResult is used anywhere else or transformed before deserialization.
rg -n "StageResult|release_stage\(|release_publish\(|Decoding release metadata POST response|s3_upload_headers" src

Repository: DeterminateSystems/flakehub-push

Length of output: 4656

Add a default for StageResult::s3_upload_headers src/flakehub_client.rs/src/main.rs

If a server response ever omits s3_upload_headers, response.json() will fail with a missing-field error and abort the release. Add #[serde(default)] so older responses can still decode cleanly.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/flakehub_client.rs` at line 25, Add the serde default attribute to the
StageResult::s3_upload_headers field so deserialization supplies an empty
HashMap when the server omits it, preserving compatibility with older responses.

pub(crate) uuid: Uuid,
}

Expand Down
12 changes: 8 additions & 4 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{fmt::Display, io::IsTerminal, process::ExitCode};

use clap::Parser;
use color_eyre::eyre::{eyre, Result};
use color_eyre::eyre::{eyre, Context as _, Result};
use error::Error;
use http::StatusCode;
use reqwest::Response;
Expand Down Expand Up @@ -148,7 +148,7 @@ async fn execute() -> Result<std::process::ExitCode> {
let stage_result: StageResult = response
.json()
.await
.map_err(|_| eyre!("Decoding release metadata POST response"))?;
.context("Decoding release metadata POST response")?;

stage_result
}
Expand Down Expand Up @@ -192,8 +192,12 @@ async fn execute() -> Result<std::process::ExitCode> {
}
};

// upload tarball to s3
s3::upload_release_to_s3(stage_result.s3_upload_url, ctx.tarball).await?;
s3::upload_release_to_s3(
stage_result.s3_upload_url,
stage_result.s3_upload_headers,
ctx.tarball,
)
.await?;

// "publish.rs" - publish the release after upload
fhclient.release_publish(stage_result.uuid).await?;
Expand Down
64 changes: 46 additions & 18 deletions src/s3.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
use std::collections::HashMap;

use color_eyre::eyre::{eyre, Result, WrapErr};
use http::header::{CONTENT_LENGTH, CONTENT_TYPE};
use http::{HeaderName, HeaderValue};
use reqwest::header::HeaderMap;

use crate::flakehub_client::Tarball;

pub async fn upload_release_to_s3(presigned_s3_url: String, tarball: Tarball) -> Result<()> {
pub async fn upload_release_to_s3(
presigned_s3_url: String,
s3_headers: HashMap<String, String>,
tarball: Tarball,
) -> Result<()> {
let overrides: HashMap<&str, String> = HashMap::from_iter([
(CONTENT_LENGTH.as_str(), tarball.bytes.len().to_string()),
(CONTENT_TYPE.as_str(), "application/gzip".to_string()),
("x-amz-checksum-sha256", tarball.hash_base64),
]);

let headers = build_headers(&s3_headers, &overrides)?;

let client = reqwest::Client::new();
let tarball_put_response = client
.put(presigned_s3_url)
.headers({
let mut header_map = HeaderMap::new();
header_map.insert(
reqwest::header::CONTENT_LENGTH,
reqwest::header::HeaderValue::from_str(&format!("{}", tarball.bytes.len()))
.unwrap(),
);
header_map.insert(
reqwest::header::HeaderName::from_static("x-amz-checksum-sha256"),
reqwest::header::HeaderValue::from_str(&tarball.hash_base64).unwrap(),
);
header_map.insert(
reqwest::header::CONTENT_TYPE,
reqwest::header::HeaderValue::from_str("application/gzip").unwrap(),
);
header_map
})
.headers(headers)
.body(tarball.bytes)
.send()
.await
Expand All @@ -42,3 +42,31 @@ pub async fn upload_release_to_s3(presigned_s3_url: String, tarball: Tarball) ->

Ok(())
}

fn build_headers(
caller_headers: &HashMap<String, String>,
overrides: &HashMap<&str, String>,
) -> Result<HeaderMap> {
let mut header_map = HeaderMap::with_capacity(caller_headers.len() + overrides.len());

for (name, value) in caller_headers {
if overrides.keys().any(|k| k.eq_ignore_ascii_case(name)) {
continue;
}
let header_name = HeaderName::from_bytes(name.as_bytes())
.wrap_err_with(|| format!("Invalid header name `{name}`"))?;
let header_value = HeaderValue::from_str(value)
.wrap_err_with(|| format!("Invalid header value for `{name}`"))?;
header_map.insert(header_name, header_value);
}

for (name, value) in overrides {
let header_name = HeaderName::from_bytes(name.as_bytes())
.wrap_err_with(|| format!("Invalid header name `{name}`"))?;
let header_value = HeaderValue::from_str(value)
.wrap_err_with(|| format!("Invalid header value for `{name}`"))?;
header_map.insert(header_name, header_value);
}

Ok(header_map)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading