From a8a20e9210b7a1b42de365de4e2132a0d50f41cc Mon Sep 17 00:00:00 2001 From: Dietmar Maurer Date: Thu, 12 Aug 2021 09:30:41 +0200 Subject: [PATCH] use new api updater features Signed-off-by: Wolfgang Bumiller --- pbs-api-types/src/userid.rs | 8 +- src/api2/config/access/tfa.rs | 10 ++- src/api2/config/acme.rs | 90 +++++++++++----------- src/api2/config/datastore.rs | 100 ++++++------------------- src/api2/node/config.rs | 63 +++++++++++++--- src/bin/proxmox_backup_manager/acme.rs | 6 +- src/config/acme/plugin.rs | 18 ++--- src/config/datastore.rs | 8 +- src/config/node.rs | 28 +++---- src/config/tfa.rs | 6 +- 10 files changed, 164 insertions(+), 173 deletions(-) diff --git a/pbs-api-types/src/userid.rs b/pbs-api-types/src/userid.rs index e931181e..8418f13e 100644 --- a/pbs-api-types/src/userid.rs +++ b/pbs-api-types/src/userid.rs @@ -30,7 +30,7 @@ use lazy_static::lazy_static; use serde::{Deserialize, Serialize}; use proxmox::api::api; -use proxmox::api::schema::{ApiStringFormat, Schema, StringSchema}; +use proxmox::api::schema::{ApiStringFormat, Schema, StringSchema, Updatable}; use proxmox::const_regex; // we only allow a limited set of characters @@ -403,6 +403,12 @@ pub struct Userid { name_len: usize, } +impl Updatable for Userid { + type Updater = Option; + + const UPDATER_IS_OPTION: bool = true; +} + impl Userid { pub const API_SCHEMA: Schema = StringSchema::new("User ID") .format(&PROXMOX_USER_ID_FORMAT) diff --git a/src/api2/config/access/tfa.rs b/src/api2/config/access/tfa.rs index 6ac9b5de..baa6f403 100644 --- a/src/api2/config/access/tfa.rs +++ b/src/api2/config/access/tfa.rs @@ -5,7 +5,6 @@ use anyhow::Error; use crate::api2::types::PROXMOX_CONFIG_DIGEST_SCHEMA; use proxmox::api::{api, Permission, Router, RpcEnvironment, SubdirMap}; -use proxmox::api::schema::Updatable; use proxmox::list_subdirs_api_method; use crate::config::tfa::{self, WebauthnConfig, WebauthnConfigUpdater}; @@ -74,9 +73,14 @@ pub fn update_webauthn_config( let digest = proxmox::tools::hex_to_digest(digest)?; crate::tools::detect_modified_configuration_file(&digest, &wa.digest()?)?; } - wa.update_from::<&str>(webauthn, &[])?; + if let Some(ref rp) = webauthn.rp { wa.rp = rp.clone(); } + if let Some(ref origin) = webauthn.rp { wa.origin = origin.clone(); } + if let Some(ref id) = webauthn.id { wa.id = id.clone(); } } else { - tfa.webauthn = Some(WebauthnConfig::try_build_from(webauthn)?); + let rp = webauthn.rp.unwrap(); + let origin = webauthn.origin.unwrap(); + let id = webauthn.id.unwrap(); + tfa.webauthn = Some(WebauthnConfig { rp, origin, id }); } tfa::write(&tfa)?; diff --git a/src/api2/config/acme.rs b/src/api2/config/acme.rs index 70c95058..5d8c4481 100644 --- a/src/api2/config/acme.rs +++ b/src/api2/config/acme.rs @@ -9,7 +9,6 @@ use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; use proxmox::api::router::SubdirMap; -use proxmox::api::schema::Updatable; use proxmox::api::{api, Permission, Router, RpcEnvironment}; use proxmox::http_bail; use proxmox::list_subdirs_api_method; @@ -21,7 +20,7 @@ use crate::acme::AcmeClient; use crate::api2::types::{AcmeAccountName, AcmeChallengeSchema, Authid, KnownAcmeDirectory}; use crate::config::acl::PRIV_SYS_MODIFY; use crate::config::acme::plugin::{ - DnsPlugin, DnsPluginCore, DnsPluginCoreUpdater, PLUGIN_ID_SCHEMA, + self, DnsPlugin, DnsPluginCore, DnsPluginCoreUpdater, PLUGIN_ID_SCHEMA, }; use crate::server::WorkerTask; use crate::tools::ControlFlow; @@ -464,7 +463,7 @@ pub struct PluginConfig { /// /// Allows to cope with long TTL of DNS records. #[serde(skip_serializing_if = "Option::is_none", default)] - validation_delay: Option, + alidation_delay: Option, /// Flag to disable the config. #[serde(skip_serializing_if = "Option::is_none", default)] @@ -515,8 +514,6 @@ fn modify_cfg_for_api(id: &str, ty: &str, data: &Value) -> PluginConfig { )] /// List ACME challenge plugins. pub fn list_plugins(mut rpcenv: &mut dyn RpcEnvironment) -> Result, Error> { - use crate::config::acme::plugin; - let (plugins, digest) = plugin::config()?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); Ok(plugins @@ -539,8 +536,6 @@ pub fn list_plugins(mut rpcenv: &mut dyn RpcEnvironment) -> Result Result { - use crate::config::acme::plugin; - let (plugins, digest) = plugin::config()?; rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); @@ -562,7 +557,7 @@ pub fn get_plugin(id: String, mut rpcenv: &mut dyn RpcEnvironment) -> Result Result Result<(), Error> { - use crate::config::acme::plugin; - +pub fn add_plugin(r#type: String, core: DnsPluginCore, data: String) -> Result<(), Error> { // Currently we only support DNS plugins and the standalone plugin is "fixed": if r#type != "dns" { bail!("invalid ACME plugin type: {:?}", r#type); @@ -588,13 +581,8 @@ pub fn add_plugin(r#type: String, core: DnsPluginCoreUpdater, data: String) -> R let data = String::from_utf8(base64::decode(&data)?) .map_err(|_| format_err!("data must be valid UTF-8"))?; - //core.api_fixup()?; - // FIXME: Solve the Updater with non-optional fields thing... - let id = core - .id - .clone() - .ok_or_else(|| format_err!("missing required 'id' parameter"))?; + let id = core.id.clone(); let _lock = plugin::lock()?; @@ -603,10 +591,7 @@ pub fn add_plugin(r#type: String, core: DnsPluginCoreUpdater, data: String) -> R bail!("ACME plugin ID {:?} already exists", id); } - let plugin = serde_json::to_value(DnsPlugin { - core: DnsPluginCore::try_build_from(core)?, - data, - })?; + let plugin = serde_json::to_value(DnsPlugin { core, data })?; plugins.insert(id, r#type, plugin); @@ -628,8 +613,6 @@ pub fn add_plugin(r#type: String, core: DnsPluginCoreUpdater, data: String) -> R )] /// Delete an ACME plugin configuration. pub fn delete_plugin(id: String) -> Result<(), Error> { - use crate::config::acme::plugin; - let _lock = plugin::lock()?; let (mut plugins, _digest) = plugin::config()?; @@ -641,10 +624,23 @@ pub fn delete_plugin(id: String) -> Result<(), Error> { Ok(()) } +#[api()] +#[derive(Serialize, Deserialize)] +#[serde(rename_all="kebab-case")] +#[allow(non_camel_case_types)] +/// Deletable property name +pub enum DeletableProperty { + /// Delete the disable property + disable, + /// Delete the validation-delay property + validation_delay, +} + #[api( input: { properties: { - core_update: { + id: { schema: PLUGIN_ID_SCHEMA }, + update: { type: DnsPluginCoreUpdater, flatten: true, }, @@ -654,14 +650,18 @@ pub fn delete_plugin(id: String) -> Result<(), Error> { // This is different in the API! description: "DNS plugin data (base64 encoded with padding).", }, + delete: { + description: "List of properties to delete.", + type: Array, + optional: true, + items: { + type: DeletableProperty, + } + }, digest: { description: "Digest to protect against concurrent updates", optional: true, }, - delete: { - description: "Options to remove from the configuration", - optional: true, - }, }, }, access: { @@ -671,13 +671,12 @@ pub fn delete_plugin(id: String) -> Result<(), Error> { )] /// Update an ACME plugin configuration. pub fn update_plugin( - core_update: DnsPluginCoreUpdater, + id: String, + update: DnsPluginCoreUpdater, data: Option, - delete: Option, + delete: Option>, digest: Option, ) -> Result<(), Error> { - use crate::config::acme::plugin; - let data = data .as_deref() .map(base64::decode) @@ -685,16 +684,6 @@ pub fn update_plugin( .map(String::from_utf8) .transpose() .map_err(|_| format_err!("data must be valid UTF-8"))?; - //core_update.api_fixup()?; - - // unwrap: the id is matched by this method's API path - let id = core_update.id.clone().unwrap(); - - let delete: Vec<&str> = delete - .as_deref() - .unwrap_or("") - .split(&[' ', ',', ';', '\0'][..]) - .collect(); let _lock = plugin::lock()?; @@ -712,10 +701,21 @@ pub fn update_plugin( } let mut plugin: DnsPlugin = serde_json::from_value(entry.clone())?; - plugin.core.update_from(core_update, &delete)?; - if let Some(data) = data { - plugin.data = data; + + if let Some(delete) = delete { + for delete_prop in delete { + match delete_prop { + DeletableProperty::validation_delay => { plugin.core.validation_delay = None; }, + DeletableProperty::disable => { plugin.core.disable = None; }, + } + } } + if let Some(data) = data { plugin.data = data; } + if let Some(api) = update.api { plugin.core.api = api; } + if update.validation_delay.is_some() { plugin.core.validation_delay = update.validation_delay; } + if update.disable.is_some() { plugin.core.disable = update.disable; } + + *entry = serde_json::to_value(plugin)?; } None => http_bail!(NOT_FOUND, "no such plugin"), diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 7edc43d8..aa754808 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -19,7 +19,7 @@ use crate::api2::admin::{ use crate::api2::types::*; use crate::backup::*; use crate::config::cached_user_info::CachedUserInfo; -use crate::config::datastore::{self, DataStoreConfig}; +use crate::config::datastore::{self, DataStoreConfig, DataStoreConfigUpdater}; use crate::config::acl::{PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY}; use crate::server::{jobstate, WorkerTask}; @@ -183,55 +183,9 @@ pub enum DeletableProperty { name: { schema: DATASTORE_SCHEMA, }, - comment: { - optional: true, - schema: SINGLE_LINE_COMMENT_SCHEMA, - }, - "notify-user": { - optional: true, - type: Userid, - }, - "notify": { - optional: true, - schema: DATASTORE_NOTIFY_STRING_SCHEMA, - }, - "gc-schedule": { - optional: true, - schema: GC_SCHEDULE_SCHEMA, - }, - "prune-schedule": { - optional: true, - schema: PRUNE_SCHEDULE_SCHEMA, - }, - "keep-last": { - optional: true, - schema: PRUNE_SCHEMA_KEEP_LAST, - }, - "keep-hourly": { - optional: true, - schema: PRUNE_SCHEMA_KEEP_HOURLY, - }, - "keep-daily": { - optional: true, - schema: PRUNE_SCHEMA_KEEP_DAILY, - }, - "keep-weekly": { - optional: true, - schema: PRUNE_SCHEMA_KEEP_WEEKLY, - }, - "keep-monthly": { - optional: true, - schema: PRUNE_SCHEMA_KEEP_MONTHLY, - }, - "keep-yearly": { - optional: true, - schema: PRUNE_SCHEMA_KEEP_YEARLY, - }, - "verify-new": { - description: "If enabled, all new backups will be verified right after completion.", - type: bool, - optional: true, - default: false, + update: { + type: DataStoreConfigUpdater, + flatten: true, }, delete: { description: "List of properties to delete.", @@ -252,21 +206,9 @@ pub enum DeletableProperty { }, )] /// Update datastore config. -#[allow(clippy::too_many_arguments)] pub fn update_datastore( + update: DataStoreConfigUpdater, name: String, - comment: Option, - gc_schedule: Option, - prune_schedule: Option, - keep_last: Option, - keep_hourly: Option, - keep_daily: Option, - keep_weekly: Option, - keep_monthly: Option, - keep_yearly: Option, - verify_new: Option, - notify: Option, - notify_user: Option, delete: Option>, digest: Option, ) -> Result<(), Error> { @@ -302,7 +244,7 @@ pub fn update_datastore( } } - if let Some(comment) = comment { + if let Some(comment) = update.comment { let comment = comment.trim().to_string(); if comment.is_empty() { data.comment = None; @@ -312,25 +254,25 @@ pub fn update_datastore( } let mut gc_schedule_changed = false; - if gc_schedule.is_some() { - gc_schedule_changed = data.gc_schedule != gc_schedule; - data.gc_schedule = gc_schedule; + if update.gc_schedule.is_some() { + gc_schedule_changed = data.gc_schedule != update.gc_schedule; + data.gc_schedule = update.gc_schedule; } let mut prune_schedule_changed = false; - if prune_schedule.is_some() { - prune_schedule_changed = data.prune_schedule != prune_schedule; - data.prune_schedule = prune_schedule; + if update.prune_schedule.is_some() { + prune_schedule_changed = data.prune_schedule != update.prune_schedule; + data.prune_schedule = update.prune_schedule; } - if keep_last.is_some() { data.keep_last = keep_last; } - if keep_hourly.is_some() { data.keep_hourly = keep_hourly; } - if keep_daily.is_some() { data.keep_daily = keep_daily; } - if keep_weekly.is_some() { data.keep_weekly = keep_weekly; } - if keep_monthly.is_some() { data.keep_monthly = keep_monthly; } - if keep_yearly.is_some() { data.keep_yearly = keep_yearly; } + if update.keep_last.is_some() { data.keep_last = update.keep_last; } + if update.keep_hourly.is_some() { data.keep_hourly = update.keep_hourly; } + if update.keep_daily.is_some() { data.keep_daily = update.keep_daily; } + if update.keep_weekly.is_some() { data.keep_weekly = update.keep_weekly; } + if update.keep_monthly.is_some() { data.keep_monthly = update.keep_monthly; } + if update.keep_yearly.is_some() { data.keep_yearly = update.keep_yearly; } - if let Some(notify_str) = notify { + if let Some(notify_str) = update.notify { let value = parse_property_string(¬ify_str, &DatastoreNotify::API_SCHEMA)?; let notify: DatastoreNotify = serde_json::from_value(value)?; if let DatastoreNotify { gc: None, verify: None, sync: None } = notify { @@ -339,9 +281,9 @@ pub fn update_datastore( data.notify = Some(notify_str); } } - if verify_new.is_some() { data.verify_new = verify_new; } + if update.verify_new.is_some() { data.verify_new = update.verify_new; } - if notify_user.is_some() { data.notify_user = notify_user; } + if update.notify_user.is_some() { data.notify_user = update.notify_user; } config.set_data(&name, "datastore", &data)?; diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs index d7d96c34..f6133d99 100644 --- a/src/api2/node/config.rs +++ b/src/api2/node/config.rs @@ -1,6 +1,6 @@ use anyhow::Error; +use ::serde::{Deserialize, Serialize}; -use proxmox::api::schema::Updatable; use proxmox::api::{api, Permission, Router, RpcEnvironment}; use crate::api2::types::NODE_SCHEMA; @@ -32,6 +32,28 @@ pub fn get_node_config(mut rpcenv: &mut dyn RpcEnvironment) -> Result Result Result, + // node: String, // not used + update: NodeConfigUpdater, + delete: Option>, digest: Option, ) -> Result<(), Error> { let _lock = crate::config::node::lock()?; @@ -71,13 +98,27 @@ pub fn update_node_config( } } - let delete: Vec<&str> = delete - .as_deref() - .unwrap_or("") - .split(&[' ', ',', ';', '\0'][..]) - .collect(); + if let Some(delete) = delete { + for delete_prop in delete { + match delete_prop { + DeletableProperty::acme => { config.acme = None; }, + DeletableProperty::acmedomain0 => { config.acmedomain0 = None; }, + DeletableProperty::acmedomain1 => { config.acmedomain1 = None; }, + DeletableProperty::acmedomain2 => { config.acmedomain2 = None; }, + DeletableProperty::acmedomain3 => { config.acmedomain3 = None; }, + DeletableProperty::acmedomain4 => { config.acmedomain4 = None; }, + DeletableProperty::http_proxy => { config.http_proxy = None; }, + } + } + } - config.update_from(updater, &delete)?; + if update.acme.is_some() { config.acme = update.acme; } + if update.acmedomain0.is_some() { config.acmedomain0 = update.acmedomain0; } + if update.acmedomain1.is_some() { config.acmedomain1 = update.acmedomain1; } + if update.acmedomain2.is_some() { config.acmedomain2 = update.acmedomain2; } + if update.acmedomain3.is_some() { config.acmedomain3 = update.acmedomain3; } + if update.acmedomain4.is_some() { config.acmedomain4 = update.acmedomain4; } + if update.http_proxy.is_some() { config.http_proxy = update.http_proxy; } crate::config::node::save_config(&config)?; diff --git a/src/bin/proxmox_backup_manager/acme.rs b/src/bin/proxmox_backup_manager/acme.rs index 61ce12b5..b1baeda7 100644 --- a/src/bin/proxmox_backup_manager/acme.rs +++ b/src/bin/proxmox_backup_manager/acme.rs @@ -9,7 +9,7 @@ use proxmox::tools::fs::file_get_contents; use proxmox_backup::acme::AcmeClient; use proxmox_backup::api2; use proxmox_backup::api2::types::AcmeAccountName; -use proxmox_backup::config::acme::plugin::DnsPluginCoreUpdater; +use proxmox_backup::config::acme::plugin::DnsPluginCore; use proxmox_backup::config::acme::KNOWN_ACME_DIRECTORIES; pub fn acme_mgmt_cli() -> CommandLineInterface { @@ -312,7 +312,7 @@ fn get_plugin(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error description: "The ACME challenge plugin type.", }, core: { - type: DnsPluginCoreUpdater, + type: DnsPluginCore, flatten: true, }, data: { @@ -323,7 +323,7 @@ fn get_plugin(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error } )] /// Show acme account information. -fn add_plugin(r#type: String, core: DnsPluginCoreUpdater, data: String) -> Result<(), Error> { +fn add_plugin(r#type: String, core: DnsPluginCore, data: String) -> Result<(), Error> { let data = base64::encode(&file_get_contents(&data)?); api2::config::acme::add_plugin(r#type, core, data)?; Ok(()) diff --git a/src/config/acme/plugin.rs b/src/config/acme/plugin.rs index a4322fdd..17bb8beb 100644 --- a/src/config/acme/plugin.rs +++ b/src/config/acme/plugin.rs @@ -62,20 +62,21 @@ impl Default for StandalonePlugin { #[serde(rename_all = "kebab-case")] pub struct DnsPluginCore { /// Plugin ID. - pub(crate) id: String, + #[updater(skip)] + pub id: String, /// DNS API Plugin Id. - pub(crate) api: String, + pub api: String, /// Extra delay in seconds to wait before requesting validation. /// /// Allows to cope with long TTL of DNS records. #[serde(skip_serializing_if = "Option::is_none", default)] - pub(crate) validation_delay: Option, + pub validation_delay: Option, /// Flag to disable the config. #[serde(skip_serializing_if = "Option::is_none", default)] - disable: Option, + pub disable: Option, } #[api( @@ -88,17 +89,12 @@ pub struct DnsPluginCore { #[serde(rename_all = "kebab-case")] pub struct DnsPlugin { #[serde(flatten)] - pub(crate) core: DnsPluginCore, + pub core: DnsPluginCore, - // FIXME: The `Updater` should allow: - // * having different descriptions for this and the Updater version - // * having different `#[serde]` attributes for the Updater - // * or, well, leaving fields out completely in teh Updater but this means we may need to - // separate Updater and Builder deriving. // We handle this property separately in the API calls. /// DNS plugin data (base64url encoded without padding). #[serde(with = "proxmox::tools::serde::string_as_base64url_nopad")] - pub(crate) data: String, + pub data: String, } impl DnsPlugin { diff --git a/src/config/datastore.rs b/src/config/datastore.rs index 70f03847..5409c7c2 100644 --- a/src/config/datastore.rs +++ b/src/config/datastore.rs @@ -5,7 +5,7 @@ use serde::{Serialize, Deserialize}; use proxmox::api::{ api, - schema::{Schema, StringSchema}, + schema::{Schema, StringSchema, Updater}, section_config::{ SectionConfig, SectionConfigData, @@ -82,14 +82,16 @@ pub const DIR_NAME_SCHEMA: Schema = StringSchema::new("Directory name").schema() }, } )] -#[derive(Serialize,Deserialize)] +#[derive(Serialize,Deserialize,Updater)] #[serde(rename_all="kebab-case")] /// Datastore configuration properties. pub struct DataStoreConfig { + #[updater(skip)] pub name: String, + #[updater(skip)] + pub path: String, #[serde(skip_serializing_if="Option::is_none")] pub comment: Option, - pub path: String, #[serde(skip_serializing_if="Option::is_none")] pub gc_schedule: Option, #[serde(skip_serializing_if="Option::is_none")] diff --git a/src/config/node.rs b/src/config/node.rs index 6b9d3bc8..c03078bb 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -94,26 +94,26 @@ pub struct AcmeConfig { /// Node specific configuration. pub struct NodeConfig { /// The acme account to use on this node. - #[serde(skip_serializing_if = "Updater::is_empty")] - acme: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub acme: Option, - #[serde(skip_serializing_if = "Updater::is_empty")] - acmedomain0: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub acmedomain0: Option, - #[serde(skip_serializing_if = "Updater::is_empty")] - acmedomain1: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub acmedomain1: Option, - #[serde(skip_serializing_if = "Updater::is_empty")] - acmedomain2: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub acmedomain2: Option, - #[serde(skip_serializing_if = "Updater::is_empty")] - acmedomain3: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub acmedomain3: Option, - #[serde(skip_serializing_if = "Updater::is_empty")] - acmedomain4: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub acmedomain4: Option, - #[serde(skip_serializing_if = "Updater::is_empty")] - http_proxy: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub http_proxy: Option, } impl NodeConfig { diff --git a/src/config/tfa.rs b/src/config/tfa.rs index efba0c13..a93ed8c0 100644 --- a/src/config/tfa.rs +++ b/src/config/tfa.rs @@ -96,18 +96,18 @@ pub struct WebauthnConfig { /// Relying party name. Any text identifier. /// /// Changing this *may* break existing credentials. - rp: String, + pub rp: String, /// Site origin. Must be a `https://` URL (or `http://localhost`). Should contain the address /// users type in their browsers to access the web interface. /// /// Changing this *may* break existing credentials. - origin: String, + pub origin: String, /// Relying part ID. Must be the domain name without protocol, port or location. /// /// Changing this *will* break existing credentials. - id: String, + pub id: String, } impl WebauthnConfig {