Skip to content

chore(api): Stop calling project-release endpoints #2991

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
wants to merge 1 commit into
base: szokeasaurusrex/remove-brotli
Choose a base branch
from
+85 −181
Open

chore(api): Stop calling project-release endpoints #2991

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
144 changes: 39 additions & 105 deletions src/api/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use chrono::Duration;
use chrono::{DateTime, FixedOffset, Utc};
use clap::ArgMatches;
use flate2::write::GzEncoder;
use if_chain::if_chain;
use lazy_static::lazy_static;
use log::{debug, info, warn};
use parking_lot::Mutex;
Expand Down Expand Up @@ -460,21 +459,9 @@ impl AuthenticatedApi<'_> {

/// Creates a new release.
pub fn new_release(&self, org: &str, release: &NewRelease) -> ApiResult<ReleaseInfo> {
// for single project releases use the legacy endpoint that is project bound.
// This means we can support both old and new servers.
if release.projects.len() == 1 {
let path = format!(
"/projects/{}/{}/releases/",
PathArg(org),
PathArg(&release.projects[0])
);
self.post(&path, release)?
.convert_rnf(ApiErrorKind::ProjectNotFound)
} else {
let path = format!("/organizations/{}/releases/", PathArg(org));
self.post(&path, release)?
.convert_rnf(ApiErrorKind::OrganizationNotFound)
}
let path = format!("/organizations/{}/releases/", PathArg(org));
self.post(&path, release)?
.convert_rnf(ApiErrorKind::OrganizationNotFound)
}

/// Updates a release.
Expand All @@ -484,29 +471,20 @@ impl AuthenticatedApi<'_> {
version: &str,
release: &UpdatedRelease,
) -> ApiResult<ReleaseInfo> {
if_chain! {
if let Some(ref projects) = release.projects;
if projects.len() == 1;
then {
let path = format!("/projects/{}/{}/releases/{}/",
PathArg(org),
PathArg(&projects[0]),
PathArg(version)
);
self.put(&path, release)?.convert_rnf(ApiErrorKind::ReleaseNotFound)
} else {
if release.version.is_some() {
let path = format!("/organizations/{}/releases/",
PathArg(org));
return self.post(&path, release)?.convert_rnf(ApiErrorKind::ReleaseNotFound)
}

let path = format!("/organizations/{}/releases/{}/",
PathArg(org),
PathArg(version));
self.put(&path, release)?.convert_rnf(ApiErrorKind::ReleaseNotFound)
}
if release.version.is_some() {
let path = format!("/organizations/{}/releases/", PathArg(org));
return self
.post(&path, release)?
.convert_rnf(ApiErrorKind::ReleaseNotFound);
}

let path = format!(
"/organizations/{}/releases/{}/",
PathArg(org),
PathArg(version)
);
self.put(&path, release)?
.convert_rnf(ApiErrorKind::ReleaseNotFound)
}

/// Sets release commits
Expand All @@ -530,28 +508,14 @@ impl AuthenticatedApi<'_> {
}

/// Deletes an already existing release. Returns `true` if it was deleted
/// or `false` if not. The project is needed to support the old deletion
/// API.
pub fn delete_release(
&self,
org: &str,
project: Option<&str>,
version: &str,
) -> ApiResult<bool> {
let resp = if let Some(project) = project {
self.delete(&format!(
"/projects/{}/{}/releases/{}/",
PathArg(org),
PathArg(project),
PathArg(version)
))?
} else {
self.delete(&format!(
"/organizations/{}/releases/{}/",
PathArg(org),
PathArg(version)
))?
};
/// or `false` if not.
pub fn delete_release(&self, org: &str, version: &str) -> ApiResult<bool> {
let resp = self.delete(&format!(
"/organizations/{}/releases/{}/",
PathArg(org),
PathArg(version)
))?;

if resp.status() == 404 {
Ok(false)
} else {
Expand All @@ -561,26 +525,12 @@ impl AuthenticatedApi<'_> {

/// Looks up a release and returns it. If it does not exist `None`
/// will be returned.
pub fn get_release(
&self,
org: &str,
project: Option<&str>,
version: &str,
) -> ApiResult<Option<ReleaseInfo>> {
let path = if let Some(project) = project {
format!(
"/projects/{}/{}/releases/{}/",
PathArg(org),
PathArg(project),
PathArg(version)
)
} else {
format!(
"/organizations/{}/releases/{}/",
PathArg(org),
PathArg(version)
)
};
pub fn get_release(&self, org: &str, version: &str) -> ApiResult<Option<ReleaseInfo>> {
let path = format!(
"/organizations/{}/releases/{}/",
PathArg(org),
PathArg(version)
);
let resp = self.get(&path)?;
if resp.status() == 404 {
Ok(None)
Expand All @@ -591,40 +541,24 @@ impl AuthenticatedApi<'_> {

/// Returns a list of releases for a given project. This is currently a
/// capped list by what the server deems an acceptable default limit.
pub fn list_releases(&self, org: &str, project: Option<&str>) -> ApiResult<Vec<ReleaseInfo>> {
if let Some(project) = project {
let path = format!("/projects/{}/{}/releases/", PathArg(org), PathArg(project));
self.get(&path)?
.convert_rnf::<Vec<ReleaseInfo>>(ApiErrorKind::ProjectNotFound)
} else {
let path = format!("/organizations/{}/releases/", PathArg(org));
self.get(&path)?
.convert_rnf::<Vec<ReleaseInfo>>(ApiErrorKind::OrganizationNotFound)
}
pub fn list_releases(&self, org: &str) -> ApiResult<Vec<ReleaseInfo>> {
let path = format!("/organizations/{}/releases/", PathArg(org));
self.get(&path)?
.convert_rnf::<Vec<ReleaseInfo>>(ApiErrorKind::OrganizationNotFound)
}

/// Looks up a release commits and returns it. If it does not exist `None`
Comment on lines 541 to 550
Copy link

@sentry sentry bot Nov 26, 2025

Choose a reason for hiding this comment

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

Bug: The update_release function incorrectly POSTs to create a new release when release.version is set, instead of PUTting to update an existing one.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The update_release function incorrectly attempts to create a new release via a POST request when release.version is Some. This occurs because archive.rs and restore.rs callers explicitly set version: Some(version.into()) in the UpdatedRelease struct. The intended behavior for these commands is to update an existing release using a PUT request, which would happen if release.version were None. Consequently, sentry-cli releases archive and sentry-cli releases restore commands will fail or create unintended duplicate releases instead of modifying the status of an existing release.

💡 Suggested Fix

Remove version: Some(version.into()) from the UpdatedRelease struct instantiation in archive.rs and restore.rs to ensure the update_release function performs a PUT request.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/api/mod.rs#L541-L550

Potential issue: The `update_release` function incorrectly attempts to create a new
release via a POST request when `release.version` is `Some`. This occurs because
`archive.rs` and `restore.rs` callers explicitly set `version: Some(version.into())` in
the `UpdatedRelease` struct. The intended behavior for these commands is to update an
existing release using a PUT request, which would happen if `release.version` were
`None`. Consequently, `sentry-cli releases archive` and `sentry-cli releases restore`
commands will fail or create unintended duplicate releases instead of modifying the
status of an existing release.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3859262

/// will be returned.
pub fn get_release_commits(
&self,
org: &str,
project: Option<&str>,
version: &str,
) -> ApiResult<Option<Vec<ReleaseCommit>>> {
let path = if let Some(project) = project {
format!(
"/projects/{}/{}/releases/{}/commits/",
PathArg(org),
PathArg(project),
PathArg(version)
)
} else {
format!(
"/organizations/{}/releases/{}/commits/",
PathArg(org),
PathArg(version)
)
};
let path = format!(
"/organizations/{}/releases/{}/commits/",
PathArg(org),
PathArg(version)
);
let resp = self.get(&path)?;
if resp.status() == 404 {
Ok(None)
Expand Down
10 changes: 4 additions & 6 deletions src/commands/releases/delete.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,11 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
let api = Api::current();
#[expect(clippy::unwrap_used, reason = "legacy code")]
let version = matches.get_one::<String>("version").unwrap();
let project = config.get_project(matches).ok();

if api.authenticated()?.delete_release(
&config.get_org(matches)?,
project.as_deref(),
version,
)? {
if api
.authenticated()?
.delete_release(&config.get_org(matches)?, version)?
{
println!("Deleted release {version}!");
} else {
println!("Did nothing. Release with this version ({version}) does not exist.");
Expand Down
7 changes: 2 additions & 5 deletions src/commands/releases/info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
let version = matches.get_one::<String>("version").unwrap();
let config = Config::current();
let org = config.get_org(matches)?;
let project = config.get_project(matches).ok();
let release = authenticated_api.get_release(&org, project.as_deref(), version)?;
let release = authenticated_api.get_release(&org, version)?;

if is_quiet_mode() {
if release.is_none() {
Expand Down Expand Up @@ -85,9 +84,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
}

if matches.get_flag("show_commits") {
if let Ok(Some(commits)) =
authenticated_api.get_release_commits(&org, project.as_deref(), version)
{
if let Ok(Some(commits)) = authenticated_api.get_release_commits(&org, version) {
if !commits.is_empty() {
data_row.add(
commits
Expand Down
3 changes: 1 addition & 2 deletions src/commands/releases/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ pub fn make_command(command: Command) -> Command {
pub fn execute(matches: &ArgMatches) -> Result<()> {
let config = Config::current();
let api = Api::current();
let project = config.get_project(matches).ok();
let releases = api
.authenticated()?
.list_releases(&config.get_org(matches)?, project.as_deref())?;
.list_releases(&config.get_org(matches)?)?;

if matches.get_flag("raw") {
let versions = releases
Expand Down
13 changes: 5 additions & 8 deletions tests/integration/releases/delete.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ fn successfully_deletes() {
.mock_endpoint(
MockEndpointBuilder::new(
"DELETE",
"/api/0/projects/wat-org/wat-project/releases/wat-release/",
"/api/0/organizations/wat-org/releases/wat-release/",
)
.with_status(204),
)
Expand All @@ -20,7 +20,7 @@ fn allows_for_release_to_start_with_hyphen() {
.mock_endpoint(
MockEndpointBuilder::new(
"DELETE",
"/api/0/projects/wat-org/wat-project/releases/-hyphenated-release/",
"/api/0/organizations/wat-org/releases/-hyphenated-release/",
)
.with_status(204),
)
Expand All @@ -32,11 +32,8 @@ fn allows_for_release_to_start_with_hyphen() {
fn informs_about_nonexisting_releases() {
TestManager::new()
.mock_endpoint(
MockEndpointBuilder::new(
"DELETE",
"/api/0/projects/wat-org/wat-project/releases/whoops/",
)
.with_status(404),
MockEndpointBuilder::new("DELETE", "/api/0/organizations/wat-org/releases/whoops/")
.with_status(404),
)
.register_trycmd_test("releases/releases-delete-nonexisting.trycmd")
.with_default_token();
Expand All @@ -48,7 +45,7 @@ fn doesnt_allow_to_delete_active_releases() {
.mock_endpoint(
MockEndpointBuilder::new(
"DELETE",
"/api/0/projects/wat-org/wat-project/releases/wat-release/",
"/api/0/organizations/wat-org/releases/wat-release/",
)
.with_status(400)
.with_response_file("releases/delete-active-release.json"),
Expand Down
24 changes: 9 additions & 15 deletions tests/integration/releases/finalize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@ use serde_json::json;
fn successfully_creates_a_release() {
TestManager::new()
.mock_endpoint(
MockEndpointBuilder::new(
"PUT",
"/api/0/projects/wat-org/wat-project/releases/wat-release/",
)
.with_response_file("releases/get-release.json"),
MockEndpointBuilder::new("PUT", "/api/0/organizations/wat-org/releases/wat-release/")
.with_response_file("releases/get-release.json"),
)
.register_trycmd_test("releases/releases-finalize.trycmd")
.with_default_token();
Expand All @@ -22,7 +19,7 @@ fn allows_for_release_to_start_with_hyphen() {
.mock_endpoint(
MockEndpointBuilder::new(
"PUT",
"/api/0/projects/wat-org/wat-project/releases/-hyphenated-release/",
"/api/0/organizations/wat-org/releases/-hyphenated-release/",
)
.with_response_file("releases/get-release.json"),
)
Expand All @@ -34,15 +31,12 @@ fn allows_for_release_to_start_with_hyphen() {
fn release_with_custom_dates() {
TestManager::new()
.mock_endpoint(
MockEndpointBuilder::new(
"PUT",
"/api/0/projects/wat-org/wat-project/releases/wat-release/",
)
.with_response_file("releases/get-release.json")
.with_matcher(Matcher::PartialJson(json!({
"projects": ["wat-project"],
"dateReleased": "2015-05-15T00:00:00Z"
}))),
MockEndpointBuilder::new("PUT", "/api/0/organizations/wat-org/releases/wat-release/")
.with_response_file("releases/get-release.json")
.with_matcher(Matcher::PartialJson(json!({
"projects": ["wat-project"],
"dateReleased": "2015-05-15T00:00:00Z"
}))),
)
.register_trycmd_test("releases/releases-finalize-dates.trycmd")
.with_default_token();
Expand Down
Loading