From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 2/2] api/config: use http_bail for 'not found' errors
Date: Fri, 4 Mar 2022 14:47:51 +0100 [thread overview]
Message-ID: <20220304134751.704763-2-d.csapak@proxmox.com> (raw)
In-Reply-To: <20220304134751.704763-1-d.csapak@proxmox.com>
the api should return a 404 error for entries that do not exist
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
not sure if this is a breaking change, since we did return a '400' before
so i'm sending it for api2/config only, and would send more if we
decide this is the way we want to go
we're already doing this for some api endpoints (e.g. acme)
src/api2/config/access/openid.rs | 6 +++---
src/api2/config/changer.rs | 6 +++---
src/api2/config/datastore.rs | 6 +++---
src/api2/config/drive.rs | 6 +++---
src/api2/config/media_pool.rs | 6 +++---
src/api2/config/remote.rs | 4 ++--
src/api2/config/sync.rs | 4 ++--
src/api2/config/tape_backup_job.rs | 6 +++---
src/api2/config/tape_encryption_keys.rs | 8 ++++----
src/api2/config/traffic_control.rs | 6 +++---
src/api2/config/verify.rs | 6 +++---
11 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/src/api2/config/access/openid.rs b/src/api2/config/access/openid.rs
index 1e5b5519..83112ce7 100644
--- a/src/api2/config/access/openid.rs
+++ b/src/api2/config/access/openid.rs
@@ -1,11 +1,11 @@
/// Configure OpenId realms
-use anyhow::{bail, Error};
+use anyhow::Error;
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -112,7 +112,7 @@ pub fn delete_openid_realm(
}
if domains.sections.remove(&realm).is_none() {
- bail!("realm '{}' does not exist.", realm);
+ http_bail!(NOT_FOUND, "realm '{}' does not exist.", realm);
}
domains::save_config(&domains)?;
diff --git a/src/api2/config/changer.rs b/src/api2/config/changer.rs
index f3513011..4fc654a9 100644
--- a/src/api2/config/changer.rs
+++ b/src/api2/config/changer.rs
@@ -1,9 +1,9 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
use ::serde::{Deserialize, Serialize};
use serde_json::Value;
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -256,7 +256,7 @@ pub fn delete_changer(name: String, _param: Value) -> Result<(), Error> {
}
config.sections.remove(&name);
},
- None => bail!("Delete changer '{}' failed - no such entry", name),
+ None => http_bail!(NOT_FOUND, "Delete changer '{}' failed - no such entry", name),
}
let drive_list: Vec<LtoTapeDrive> = config.convert_to_typed_array("lto")?;
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 0ec84423..992092a9 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -1,11 +1,11 @@
use std::path::PathBuf;
-use anyhow::{bail, Error};
+use anyhow::Error;
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, RpcEnvironmentType, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, RpcEnvironmentType, Permission};
use proxmox_schema::{api, param_bail, ApiType};
use proxmox_section_config::SectionConfigData;
use proxmox_sys::WorkerTaskContext;
@@ -359,7 +359,7 @@ pub async fn delete_datastore(
match config.sections.get(&name) {
Some(_) => { config.sections.remove(&name); },
- None => bail!("datastore '{}' does not exist.", name),
+ None => http_bail!(NOT_FOUND, "datastore '{}' does not exist.", name),
}
if !keep_job_configs {
diff --git a/src/api2/config/drive.rs b/src/api2/config/drive.rs
index 68dc8d06..370c5a94 100644
--- a/src/api2/config/drive.rs
+++ b/src/api2/config/drive.rs
@@ -1,9 +1,9 @@
-use anyhow::{bail, format_err, Error};
+use anyhow::{format_err, Error};
use ::serde::{Deserialize, Serialize};
use serde_json::Value;
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -258,7 +258,7 @@ pub fn delete_drive(name: String, _param: Value) -> Result<(), Error> {
}
config.sections.remove(&name);
},
- None => bail!("Delete drive '{}' failed - no such drive", name),
+ None => http_bail!(NOT_FOUND, "Delete drive '{}' failed - no such drive", name),
}
pbs_config::drive::save_config(&config)?;
diff --git a/src/api2/config/media_pool.rs b/src/api2/config/media_pool.rs
index be4ff053..f350eaea 100644
--- a/src/api2/config/media_pool.rs
+++ b/src/api2/config/media_pool.rs
@@ -1,7 +1,7 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
use ::serde::{Deserialize, Serialize};
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -217,7 +217,7 @@ pub fn delete_pool(name: String) -> Result<(), Error> {
match config.sections.get(&name) {
Some(_) => { config.sections.remove(&name); },
- None => bail!("delete pool '{}' failed - no such pool", name),
+ None => http_bail!(NOT_FOUND, "delete pool '{}' failed - no such pool", name),
}
pbs_config::media_pool::save_config(&config)?;
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index 89564e8f..12e35ba4 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -6,7 +6,7 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{http_err, ApiMethod, Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, http_err, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_client::{HttpClient, HttpClientOptions};
@@ -272,7 +272,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
match config.sections.get(&name) {
Some(_) => { config.sections.remove(&name); },
- None => bail!("remote '{}' does not exist.", name),
+ None => http_bail!(NOT_FOUND, "remote '{}' does not exist.", name),
}
pbs_config::remote::save_config(&config)?;
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 93584ecb..ab4acf22 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -3,7 +3,7 @@ use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -367,7 +367,7 @@ pub fn delete_sync_job(
}
config.sections.remove(&id);
},
- Err(_) => { bail!("job '{}' does not exist.", id) },
+ Err(_) => { http_bail!(NOT_FOUND, "job '{}' does not exist.", id) },
};
sync::save_config(&config)?;
diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
index 4d452039..770488be 100644
--- a/src/api2/config/tape_backup_job.rs
+++ b/src/api2/config/tape_backup_job.rs
@@ -1,9 +1,9 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -268,7 +268,7 @@ pub fn delete_tape_backup_job(
Ok(_job) => {
config.sections.remove(&id);
},
- Err(_) => { bail!("job '{}' does not exist.", id) },
+ Err(_) => { http_bail!(NOT_FOUND, "job '{}' does not exist.", id) },
};
pbs_config::tape_job::save_config(&config)?;
diff --git a/src/api2/config/tape_encryption_keys.rs b/src/api2/config/tape_encryption_keys.rs
index 910eb0ab..3e9a60d1 100644
--- a/src/api2/config/tape_encryption_keys.rs
+++ b/src/api2/config/tape_encryption_keys.rs
@@ -2,7 +2,7 @@ use anyhow::{format_err, bail, Error};
use serde_json::Value;
use hex::FromHex;
-use proxmox_router::{ApiMethod, Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -126,7 +126,7 @@ pub fn change_passphrase(
let key_config = match config_map.get(&fingerprint) {
Some(key_config) => key_config,
- None => bail!("tape encryption key configuration '{}' does not exist.", fingerprint),
+ None => http_bail!(NOT_FOUND, "tape encryption key configuration '{}' does not exist.", fingerprint),
};
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -234,7 +234,7 @@ pub fn read_key(
let key_config = match config_map.get(&fingerprint) {
Some(key_config) => key_config,
- None => bail!("tape encryption key '{}' does not exist.", fingerprint),
+ None => http_bail!(NOT_FOUND, "tape encryption key '{}' does not exist.", fingerprint),
};
if key_config.kdf.is_none() {
@@ -281,7 +281,7 @@ pub fn delete_key(
match config_map.get(&fingerprint) {
Some(_) => { config_map.remove(&fingerprint); },
- None => bail!("tape encryption key '{}' does not exist.", fingerprint),
+ None => http_bail!(NOT_FOUND, "tape encryption key '{}' does not exist.", fingerprint),
}
save_key_configs(config_map)?;
diff --git a/src/api2/config/traffic_control.rs b/src/api2/config/traffic_control.rs
index f992157d..bd14138d 100644
--- a/src/api2/config/traffic_control.rs
+++ b/src/api2/config/traffic_control.rs
@@ -1,9 +1,9 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{ApiMethod, Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -245,7 +245,7 @@ pub fn delete_traffic_control(name: String, digest: Option<String>) -> Result<()
match config.sections.get(&name) {
Some(_) => { config.sections.remove(&name); },
- None => bail!("traffic control rule '{}' does not exist.", name),
+ None => http_bail!(NOT_FOUND, "traffic control rule '{}' does not exist.", name),
}
pbs_config::traffic_control::save_config(&config)?;
diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
index 2d7e9485..c0a7820f 100644
--- a/src/api2/config/verify.rs
+++ b/src/api2/config/verify.rs
@@ -1,9 +1,9 @@
-use anyhow::{bail, Error};
+use anyhow::Error;
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
use hex::FromHex;
-use proxmox_router::{Router, RpcEnvironment, Permission};
+use proxmox_router::{http_bail, Router, RpcEnvironment, Permission};
use proxmox_schema::{api, param_bail};
use pbs_api_types::{
@@ -287,7 +287,7 @@ pub fn delete_verification_job(
match config.sections.get(&id) {
Some(_) => { config.sections.remove(&id); },
- None => bail!("job '{}' does not exist.", id),
+ None => http_bail!(NOT_FOUND, "job '{}' does not exist.", id),
}
verify::save_config(&config)?;
--
2.30.2
next prev parent reply other threads:[~2022-03-04 13:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-04 13:47 [pbs-devel] [PATCH proxmox-backup 1/2] api/config: use param_bail for parameter errors Dominik Csapak
2022-03-04 13:47 ` Dominik Csapak [this message]
2022-03-08 8:12 ` [pbs-devel] applied-series: " Wolfgang Bumiller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220304134751.704763-2-d.csapak@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox