Re: [pbs-devel] [PATCH v1 proxmox-backup 03/14] api2: add verify job config endpoint

[Dominik Csapak <d.csapak@proxmox.com>]

comments inline

On 9/25/20 10:43 AM, Hannes Laimer wrote:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   src/api2/config.rs        |   2 +
>   src/api2/config/verify.rs | 272 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 274 insertions(+)
>   create mode 100644 src/api2/config/verify.rs
> 
> diff --git a/src/api2/config.rs b/src/api2/config.rs
> index be7397c8..7a5129c7 100644
> --- a/src/api2/config.rs
> +++ b/src/api2/config.rs
> @@ -4,11 +4,13 @@ use proxmox::list_subdirs_api_method;
>   pub mod datastore;
>   pub mod remote;
>   pub mod sync;
> +pub mod verify;
>   
>   const SUBDIRS: SubdirMap = &[
>       ("datastore", &datastore::ROUTER),
>       ("remote", &remote::ROUTER),
>       ("sync", &sync::ROUTER),
> +    ("verify", &verify::ROUTER)
>   ];
>   
>   pub const ROUTER: Router = Router::new()
> diff --git a/src/api2/config/verify.rs b/src/api2/config/verify.rs
> new file mode 100644
> index 00000000..5e0db40f
> --- /dev/null
> +++ b/src/api2/config/verify.rs
> @@ -0,0 +1,272 @@
> +use anyhow::{bail, Error};
> +use serde_json::Value;
> +use ::serde::{Deserialize, Serialize};
> +
> +use proxmox::api::{api, Router, RpcEnvironment};
> +use proxmox::tools::fs::open_file_locked;
> +
> +use crate::api2::types::*;
> +use crate::config::verify::{self, VerifyJobConfig};
> +
> +#[api(
> +    input: {
> +        properties: {},
> +    },
> +    returns: {
> +        description: "List configured jobs.",
> +        type: Array,
> +        items: { type: verify::VerifyJobConfig },
> +    },
> +)]
> +/// List all verify jobs
> +pub fn list_verify_jobs(
> +    _param: Value,
> +    mut rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Vec<VerifyJobConfig>, Error> {
> +
> +    let (config, digest) = verify::config()?;
> +
> +    let list = config.convert_to_typed_array("verify")?;
> +
> +    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
> +
> +    Ok(list)
> +}
> +
> +
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            id: {
> +                schema: JOB_ID_SCHEMA,
> +            },
> +            store: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +            "ignore-verified": {
> +                schema: IGNORE_VERIFIED_BACKUPS_SCHEMA,

i would find it more sensible to have this also optional, as is
already has default anyway

also i would probably make it an Option in the struct also,
since there is a documented default and a not serialized option
takes less space in the config

> +            },
> +            "outdated-after": {
> +                optional: true,
> +                schema: VERIFICATION_OUTDATED_AFTER_SCHEMA,
> +            },
> +            comment: {
> +                optional: true,
> +                schema: SINGLE_LINE_COMMENT_SCHEMA,
> +            },
> +            schedule: {
> +                optional: true,
> +                schema: VERIFY_SCHEDULE_SCHEMA,
> +            },
> +        }
> +    }
> +)]
> +/// Create a new verify job.
> +pub fn create_verify_job(param: Value) -> Result<(), Error> {
> +
> +    let _lock = open_file_locked(verify::VERIFY_CFG_LOCKFILE, std::time::Duration::new(10, 0))?;
> +
> +    let verify_job: verify::VerifyJobConfig = serde_json::from_value(param.clone())?;
> +
> +    let (mut config, _digest) = verify::config()?;
> +
> +    if let Some(_) = config.sections.get(&verify_job.id) {
> +        bail!("job '{}' already exists.", verify_job.id);
> +    }
> +
> +    config.set_data(&verify_job.id, "verify", &verify_job)?;
> +
> +    verify::save_config(&config)?;
> +
> +    crate::config::jobstate::create_state_file("verifyjob", &verify_job.id)?;
> +
> +    Ok(())
> +}
> +
> +#[api(
> +   input: {
> +        properties: {
> +            id: {
> +                schema: JOB_ID_SCHEMA,
> +            },
> +        },
> +    },
> +    returns: {
> +        description: "The verify job configuration.",
> +        type: verify::VerifyJobConfig,
> +    },
> +)]
> +/// Read a verify job configuration.
> +pub fn read_verify_job(
> +    id: String,
> +    mut rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<VerifyJobConfig, Error> {
> +    let (config, digest) = verify::config()?;
> +
> +    let verify_job = config.lookup("verify", &id)?;
> +    rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
> +
> +    Ok(verify_job)
> +}
> +
> +#[api()]
> +#[derive(Serialize, Deserialize)]
> +#[serde(rename_all="kebab-case")]
> +#[allow(non_camel_case_types)]
> +/// Deletable property name
> +pub enum DeletableProperty {
> +    /// Delete the comment property.
> +    comment,
> +    /// Delete the job schedule.
> +    schedule,
> +    /// Delete outdated_after.
> +    outdated_after > +}
> +
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            id: {
> +                schema: JOB_ID_SCHEMA,
> +            },
> +            store: {
> +                schema: DATASTORE_SCHEMA,
> +                optional: true,
> +            },
> +            "ignore-verified": {
> +                schema: IGNORE_VERIFIED_BACKUPS_SCHEMA,
> +                optional: true,
> +            },
> +            "outdated-after": {
> +                optional: true,
> +                schema: VERIFICATION_OUTDATED_AFTER_SCHEMA,
> +            },
> +            comment: {
> +                optional: true,
> +                schema: SINGLE_LINE_COMMENT_SCHEMA,
> +            },
> +            schedule: {
> +                optional: true,
> +                schema: VERIFY_SCHEDULE_SCHEMA,
> +            },
> +            delete: {
> +                description: "List of properties to delete.",
> +                type: Array,
> +                optional: true,
> +                items: {
> +                    type: DeletableProperty,
> +                }
> +            },
> +            digest: {
> +                optional: true,
> +                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Update verify job config.
> +pub fn update_verify_job(
> +    id: String,
> +    store: Option<String>,
> +    ignore_verified: Option<bool>,
> +    outdated_after: Option<i64>,
> +    comment: Option<String>,
> +    schedule: Option<String>,
> +    delete: Option<Vec<DeletableProperty>>,
> +    digest: Option<String>,
> +) -> Result<(), Error> {
> +
> +    let _lock = open_file_locked(verify::VERIFY_CFG_LOCKFILE, std::time::Duration::new(10, 0))?;
> +
> +    // pass/compare digest
> +    let (mut config, expected_digest) = verify::config()?;
> +
> +    if let Some(ref digest) = digest {
> +        let digest = proxmox::tools::hex_to_digest(digest)?;
> +        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> +    }
> +
> +    let mut data: verify::VerifyJobConfig = config.lookup("verify", &id)?;
> +
> +     if let Some(delete) = delete {
> +        for delete_prop in delete {
> +            match delete_prop {
> +                DeletableProperty::outdated_after => { data.outdated_after = None; },
> +                DeletableProperty::comment => { data.comment = None; },
> +                DeletableProperty::schedule => { data.schedule = None; },
> +            }
> +        }
> +    }
> +
> +    if let Some(comment) = comment {
> +        let comment = comment.trim().to_string();
> +        if comment.is_empty() {
> +            data.comment = None;
> +        } else {
> +            data.comment = Some(comment);
> +        }
> +    }
> +
> +    if let Some(store) = store { data.store = store; }
> +    if let Some(ignore_verified) = ignore_verified {
> +        data.ignore_verified = ignore_verified;
> +    }
> +    if outdated_after.is_some() { data.outdated_after = outdated_after }
> +    if schedule.is_some() { data.schedule = schedule; }
> +
> +    config.set_data(&id, "verify", &data)?;
> +
> +    verify::save_config(&config)?;
> +
> +    Ok(())
> +}
> +
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            id: {
> +                schema: JOB_ID_SCHEMA,
> +            },
> +            digest: {
> +                optional: true,
> +                schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
> +            },
> +        },
> +    },
> +)]
> +/// Remove a verify job configuration
> +pub fn delete_verify_job(id: String, digest: Option<String>) -> Result<(), Error> {
> +
> +    let _lock = open_file_locked(verify::VERIFY_CFG_LOCKFILE, std::time::Duration::new(10, 0))?;
> +
> +    let (mut config, expected_digest) = verify::config()?;
> +
> +    if let Some(ref digest) = digest {
> +        let digest = proxmox::tools::hex_to_digest(digest)?;
> +        crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?;
> +    }
> +
> +    match config.sections.get(&id) {
> +        Some(_) => { config.sections.remove(&id); },
> +        None => bail!("job '{}' does not exist.", id),
> +    }
> +
> +    verify::save_config(&config)?;
> +
> +    crate::config::jobstate::remove_state_file("verifyjob", &id)?;
> +
> +    Ok(())
> +}
> +
> +const ITEM_ROUTER: Router = Router::new()
> +    .get(&API_METHOD_READ_VERIFY_JOB)
> +    .put(&API_METHOD_UPDATE_VERIFY_JOB)
> +    .delete(&API_METHOD_DELETE_VERIFY_JOB);
> +
> +pub const ROUTER: Router = Router::new()
> +    .get(&API_METHOD_LIST_VERIFY_JOBS)
> +    .post(&API_METHOD_CREATE_VERIFY_JOB)
> +    .match_all("id", &ITEM_ROUTER);
> \ No newline at end of file
>