Skip to content

Commit a13e1bf

Browse files
crowlKatscrowlKats
authored
feat(admin): allow deleting package versions (#1011)
this PR adds an endpoint to be able to delete package versions, but only for admins. This is not exposed in the UI for admins either, as the fact that this is for admins only is just a temporary restriction, and the idea is to come up with sensible restrictions that apply to users so they can delete versions (see #899).
1 parent 89e2426 commit a13e1bf

File tree

9 files changed

+457
-16
lines changed

9 files changed

+457
-16
lines changed

‎api/.sqlx/query-1d1876dcffdc0d3d1eead78a1514f3cac87830214e21abdeb5be583c1f6804b1.json‎

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

‎api/migrations/20250310055436_package_version_drop.sql‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
alter table package_version_dependencies
2+
drop constraint package_version_dependencies_package_scope_package_name_fkey;
3+
4+
alter table package_version_dependencies
5+
add foreign key (package_scope, package_name) references packages ON UPDATE CASCADE ON DELETE CASCADE;
6+
7+
8+
alter table package_version_dependencies
9+
drop constraint package_version_dependencies_package_scope_package_name_pa_fkey;
10+
11+
alter table package_version_dependencies
12+
add constraint package_version_dependencies_package_scope_package_name_pa_fkey
13+
foreign key (package_scope, package_name, package_version) references package_versions ON UPDATE CASCADE ON DELETE CASCADE;
14+
15+
alter table package_files
16+
drop constraint package_files_scope_name_version_fkey;
17+
18+
alter table package_files
19+
add foreign key (scope, name, version) references package_versions
20+
ON UPDATE CASCADE ON DELETE CASCADE;
21+
22+
alter table npm_tarballs
23+
drop constraint npm_tarballs_scope_name_version_fkey;
24+
25+
alter table npm_tarballs
26+
add foreign key (scope, name, version) references package_versions
27+
on UPDATE CASCADE ON DELETE CASCADE;

‎api/src/api/errors.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ errors!(
237237
status: BAD_REQUEST,
238238
"The requested package is archived. Unarchive it to modify settings or publish to it.",
239239
},
240+
DeleteVersionHasDependents {
241+
status: BAD_REQUEST,
242+
"The requested package version has dependents. Only a version without dependents can be deleted.",
243+
},
240244
);
241245

242246
pub fn map_unique_violation(err: sqlx::Error, new_err: ApiError) -> ApiError {

‎api/src/api/package.rs‎

Lines changed: 158 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ pub fn package_router() -> Router<Body, ApiError> {
145145
"/:package/versions/:version",
146146
util::auth(version_update_handler),
147147
)
148+
.delete(
149+
"/:package/versions/:version",
150+
util::auth(version_delete_handler),
151+
)
148152
.post(
149153
"/:package/versions/:version/provenance",
150154
util::auth(version_provenance_statements_handler),
@@ -942,8 +946,98 @@ pub async fn version_update_handler(
942946
gzip_encoded: false,
943947
},
944948
)
945-
.await
946-
.unwrap();
949+
.await?;
950+
951+
let npm_version_manifest_path =
952+
crate::gcs_paths::npm_version_manifest_path(&scope, &package);
953+
let npm_version_manifest =
954+
generate_npm_version_manifest(db, npm_url, &scope, &package).await?;
955+
let content = serde_json::to_vec_pretty(&npm_version_manifest)?;
956+
buckets
957+
.npm_bucket
958+
.upload(
959+
npm_version_manifest_path.into(),
960+
UploadTaskBody::Bytes(content.into()),
961+
GcsUploadOptions {
962+
content_type: Some("application/json".into()),
963+
cache_control: Some(CACHE_CONTROL_DO_NOT_CACHE.into()),
964+
gzip_encoded: false,
965+
},
966+
)
967+
.await?;
968+
969+
Ok(
970+
Response::builder()
971+
.status(StatusCode::NO_CONTENT)
972+
.body(Body::empty())
973+
.unwrap(),
974+
)
975+
}
976+
977+
#[instrument(
978+
name = "DELETE /api/scopes/:scope/packages/:package/versions/:version",
979+
skip(req),
980+
err,
981+
fields(scope, package, version)
982+
)]
983+
pub async fn version_delete_handler(
984+
req: Request<Body>,
985+
) -> ApiResult<Response<Body>> {
986+
let scope = req.param_scope()?;
987+
let package = req.param_package()?;
988+
let version = req.param_version()?;
989+
Span::current().record("scope", field::display(&scope));
990+
Span::current().record("package", field::display(&package));
991+
Span::current().record("version", field::display(&version));
992+
993+
let db = req.data::<Database>().unwrap();
994+
let buckets = req.data::<Buckets>().unwrap().clone();
995+
let npm_url = &req.data::<NpmUrl>().unwrap().0;
996+
997+
let iam = req.iam();
998+
iam.check_admin_access()?;
999+
1000+
let count = db
1001+
.count_package_dependents(
1002+
crate::db::DependencyKind::Jsr,
1003+
&format!("@{}/{}", scope, package),
1004+
)
1005+
.await?;
1006+
1007+
if count > 0 {
1008+
return Err(ApiError::DeleteVersionHasDependents);
1009+
}
1010+
1011+
db.delete_package_version(&scope, &package, &version)
1012+
.await?;
1013+
1014+
let path = crate::gcs_paths::docs_v1_path(&scope, &package, &version);
1015+
buckets.docs_bucket.delete_file(path.into()).await?;
1016+
1017+
let path = crate::gcs_paths::version_metadata(&scope, &package, &version);
1018+
buckets.modules_bucket.delete_file(path.into()).await?;
1019+
1020+
let path =
1021+
crate::gcs_paths::file_path_root_directory(&scope, &package, &version);
1022+
buckets.modules_bucket.delete_directory(path.into()).await?;
1023+
1024+
let package_metadata_path =
1025+
crate::gcs_paths::package_metadata(&scope, &package);
1026+
let package_metadata = PackageMetadata::create(db, &scope, &package).await?;
1027+
1028+
let content = serde_json::to_vec_pretty(&package_metadata)?;
1029+
buckets
1030+
.modules_bucket
1031+
.upload(
1032+
package_metadata_path.into(),
1033+
UploadTaskBody::Bytes(content.into()),
1034+
GcsUploadOptions {
1035+
content_type: Some("application/json".into()),
1036+
cache_control: Some(CACHE_CONTROL_DO_NOT_CACHE.into()),
1037+
gzip_encoded: false,
1038+
},
1039+
)
1040+
.await?;
9471041

9481042
let npm_version_manifest_path =
9491043
crate::gcs_paths::npm_version_manifest_path(&scope, &package);
@@ -4093,4 +4187,66 @@ ggHohNAjhbzDaY2iBW/m3NC5dehGUP4T2GBo/cwGhg==
40934187
assert_eq!(tasks.len(), 1);
40944188
assert_eq!(tasks[0].id, task2.id);
40954189
}
4190+
4191+
#[tokio::test]
4192+
async fn delete_version() {
4193+
let mut t = TestSetup::new().await;
4194+
let staff_token = t.staff_user.token.clone();
4195+
4196+
// unpublished package
4197+
let mut resp = t
4198+
.http()
4199+
.get("/api/scopes/scope/packages/foo/versions/0.0.1/dependencies/graph")
4200+
.call()
4201+
.await
4202+
.unwrap();
4203+
resp
4204+
.expect_err_code(StatusCode::NOT_FOUND, "packageVersionNotFound")
4205+
.await;
4206+
4207+
let task = process_tarball_setup(&t, create_mock_tarball("ok")).await;
4208+
assert_eq!(task.status, PublishingTaskStatus::Success, "{:?}", task);
4209+
4210+
// Now publish a package that has a few deps
4211+
let package_name = PackageName::try_from("bar").unwrap();
4212+
let version = Version::try_from("1.2.3").unwrap();
4213+
let task = crate::publish::tests::process_tarball_setup2(
4214+
&t,
4215+
create_mock_tarball("depends_on_ok"),
4216+
&package_name,
4217+
&version,
4218+
false,
4219+
)
4220+
.await;
4221+
assert_eq!(task.status, PublishingTaskStatus::Success, "{:?}", task);
4222+
4223+
let mut resp = t
4224+
.http()
4225+
.delete("/api/scopes/scope/packages/foo/versions/0.0.1")
4226+
.token(Some(&staff_token))
4227+
.call()
4228+
.await
4229+
.unwrap();
4230+
resp
4231+
.expect_err_code(StatusCode::BAD_REQUEST, "deleteVersionHasDependents")
4232+
.await;
4233+
4234+
let mut resp = t
4235+
.http()
4236+
.delete("/api/scopes/scope/packages/bar/versions/1.2.3")
4237+
.token(Some(&staff_token))
4238+
.call()
4239+
.await
4240+
.unwrap();
4241+
resp.expect_ok_no_content().await;
4242+
4243+
let mut resp = t
4244+
.http()
4245+
.delete("/api/scopes/scope/packages/foo/versions/0.0.1")
4246+
.token(Some(&staff_token))
4247+
.call()
4248+
.await
4249+
.unwrap();
4250+
resp.expect_ok_no_content().await;
4251+
}
40964252
}

‎api/src/buckets.rs‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use futures::Future;
88
use futures::FutureExt;
99
use futures::Stream;
1010
use futures::StreamExt;
11+
use futures::TryStreamExt;
1112
use tokio::sync::mpsc;
1213
use tokio_stream::wrappers::UnboundedReceiverStream;
1314
use tracing::instrument;
@@ -24,6 +25,8 @@ pub struct BucketWithQueue {
2425
pub bucket: gcp::Bucket,
2526
upload_queue: DynamicBackgroundTaskQueue<UploadTask>,
2627
download_queue: DynamicBackgroundTaskQueue<DownloadTask>,
28+
delete_queue: DynamicBackgroundTaskQueue<DeleteFileTask>,
29+
list_queue: DynamicBackgroundTaskQueue<ListDirectoryTask>,
2730
}
2831

2932
impl BucketWithQueue {
@@ -32,6 +35,8 @@ impl BucketWithQueue {
3235
bucket,
3336
upload_queue: DynamicBackgroundTaskQueue::default(),
3437
download_queue: DynamicBackgroundTaskQueue::default(),
38+
delete_queue: DynamicBackgroundTaskQueue::default(),
39+
list_queue: DynamicBackgroundTaskQueue::default(),
3540
}
3641
}
3742

@@ -72,6 +77,40 @@ impl BucketWithQueue {
7277
.await
7378
.unwrap()
7479
}
80+
81+
#[instrument(name = "BucketWithQueue::delete_file", skip(self), err)]
82+
pub async fn delete_file(&self, path: Arc<str>) -> Result<bool, GcsError> {
83+
self
84+
.delete_queue
85+
.run(DeleteFileTask {
86+
bucket: self.bucket.clone(),
87+
path,
88+
})
89+
.await
90+
.unwrap()
91+
}
92+
93+
#[instrument(name = "BucketWithQueue::delete_directory", skip(self), err)]
94+
pub async fn delete_directory(&self, path: Arc<str>) -> Result<(), GcsError> {
95+
let list = self
96+
.list_queue
97+
.run(ListDirectoryTask {
98+
bucket: self.bucket.clone(),
99+
path,
100+
})
101+
.await
102+
.unwrap()?;
103+
104+
if let Some(list) = list {
105+
let stream = futures::stream::iter(list.items)
106+
.map(|item| self.delete_file(item.name.into()))
107+
.buffer_unordered(64);
108+
109+
let _ = stream.try_collect::<Vec<_>>().await?;
110+
}
111+
112+
Ok(())
113+
}
75114
}
76115

77116
#[derive(Clone)]
@@ -191,3 +230,61 @@ impl RestartableTask for DownloadTask {
191230
.boxed()
192231
}
193232
}
233+
234+
struct DeleteFileTask {
235+
bucket: gcp::Bucket,
236+
path: Arc<str>,
237+
}
238+
239+
impl RestartableTask for DeleteFileTask {
240+
type Ok = bool;
241+
type Err = gcp::GcsError;
242+
type Fut =
243+
Pin<Box<dyn Future<Output = RestartableTaskResult<Self>> + Send + 'static>>;
244+
245+
fn run(self) -> Self::Fut {
246+
async move {
247+
let res = self.bucket.delete_file(&self.path).await;
248+
match res {
249+
Ok(data) => RestartableTaskResult::Ok(data),
250+
Err(e) if e.is_retryable() => {
251+
RestartableTaskResult::Backoff(DeleteFileTask {
252+
bucket: self.bucket,
253+
path: self.path,
254+
})
255+
}
256+
Err(e) => RestartableTaskResult::Error(e),
257+
}
258+
}
259+
.boxed()
260+
}
261+
}
262+
263+
struct ListDirectoryTask {
264+
bucket: gcp::Bucket,
265+
path: Arc<str>,
266+
}
267+
268+
impl RestartableTask for ListDirectoryTask {
269+
type Ok = Option<gcp::List>;
270+
type Err = gcp::GcsError;
271+
type Fut =
272+
Pin<Box<dyn Future<Output = RestartableTaskResult<Self>> + Send + 'static>>;
273+
274+
fn run(self) -> Self::Fut {
275+
async move {
276+
let res = self.bucket.list(&self.path).await;
277+
match res {
278+
Ok(data) => RestartableTaskResult::Ok(data),
279+
Err(e) if e.is_retryable() => {
280+
RestartableTaskResult::Backoff(ListDirectoryTask {
281+
bucket: self.bucket,
282+
path: self.path,
283+
})
284+
}
285+
Err(e) => RestartableTaskResult::Error(e),
286+
}
287+
}
288+
.boxed()
289+
}
290+
}

‎api/src/db/database.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,6 +1649,26 @@ impl Database {
16491649
.await
16501650
}
16511651

1652+
#[instrument(name = "Database::delete_package_version", skip(self), err)]
1653+
pub async fn delete_package_version(
1654+
&self,
1655+
scope: &ScopeName,
1656+
name: &PackageName,
1657+
version: &Version,
1658+
) -> Result<()> {
1659+
sqlx::query_as!(
1660+
PackageVersion,
1661+
r#"DELETE FROM package_versions WHERE scope = $1 AND name = $2 AND version = $3"#,
1662+
scope as _,
1663+
name as _,
1664+
version as _
1665+
)
1666+
.execute(&self.pool)
1667+
.await?;
1668+
1669+
Ok(())
1670+
}
1671+
16521672
#[instrument(name = "Database::get_package_file", skip(self), err)]
16531673
pub async fn get_package_file(
16541674
&self,

0 commit comments

Comments
 (0)