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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal