Re: [pbs-devel] [PATCH v3 proxmox-backup 16/33] config: jobs: add `sync-push` config type for push sync jobs

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On September 12, 2024 4:33 pm, Christian Ebner wrote:
> In order for sync jobs to be either pull or push jobs, allow to
> configure the direction of the job.
> 
> Adds an additional config type `sync-push` to the sync job config, to
> clearly distinguish sync jobs configured in pull and in push
> direction.
> 
> This approach was chosen in order to limit possible misconfiguration,
> as unintentionally switching the sync direction could potentially
> delete still required snapshots.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 2:
> - no changes
> 
>  pbs-api-types/src/jobs.rs | 52 +++++++++++++++++++++++++++++++++++++++
>  pbs-config/src/sync.rs    | 11 +++++++--
>  2 files changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
> index 868702bc0..12b39782c 100644
> --- a/pbs-api-types/src/jobs.rs
> +++ b/pbs-api-types/src/jobs.rs
> @@ -20,6 +20,8 @@ const_regex! {
>      pub VERIFICATION_JOB_WORKER_ID_REGEX = concatcp!(r"^(", PROXMOX_SAFE_ID_REGEX_STR, r"):");
>      /// Regex for sync jobs '(REMOTE|\-):REMOTE_DATASTORE:LOCAL_DATASTORE:(?:LOCAL_NS_ANCHOR:)ACTUAL_JOB_ID'
>      pub SYNC_JOB_WORKER_ID_REGEX = concatcp!(r"^(", PROXMOX_SAFE_ID_REGEX_STR, r"|\-):(", PROXMOX_SAFE_ID_REGEX_STR, r"):(", PROXMOX_SAFE_ID_REGEX_STR, r")(?::(", BACKUP_NS_RE, r"))?:");
> +    /// Regex for sync direction'(pull|push)'
> +    pub SYNC_DIRECTION_REGEX = r"^(pull|push)$";
>  }
>  
>  pub const JOB_ID_SCHEMA: Schema = StringSchema::new("Job ID.")
> @@ -498,6 +500,56 @@ pub const TRANSFER_LAST_SCHEMA: Schema =
>          .minimum(1)
>          .schema();
>  
> +pub const SYNC_DIRECTION_SCHEMA: Schema = StringSchema::new("Sync job direction (pull|push)")
> +    .format(&ApiStringFormat::Pattern(&SYNC_DIRECTION_REGEX))
> +    .schema();
> +
> +/// Direction of the sync job, push or pull
> +#[derive(Clone, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash, UpdaterType)]
> +pub enum SyncDirection {
> +    #[default]
> +    Pull,
> +    Push,
> +}
> +
> +impl ApiType for SyncDirection {
> +    const API_SCHEMA: Schema = SYNC_DIRECTION_SCHEMA;
> +}
> +
> +// used for serialization using `proxmox_serde::forward_serialize_to_display` macro
> +impl std::fmt::Display for SyncDirection {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        match self {
> +            SyncDirection::Pull => f.write_str("pull"),
> +            SyncDirection::Push => f.write_str("push"),
> +        }
> +    }
> +}
> +
> +impl std::str::FromStr for SyncDirection {
> +    type Err = anyhow::Error;
> +
> +    fn from_str(s: &str) -> Result<Self, Self::Err> {
> +        match s {
> +            "pull" => Ok(SyncDirection::Pull),
> +            "push" => Ok(SyncDirection::Push),
> +            _ => bail!("invalid sync direction"),
> +        }
> +    }
> +}
> +
> +proxmox_serde::forward_deserialize_to_from_str!(SyncDirection);
> +proxmox_serde::forward_serialize_to_display!(SyncDirection);

wouldn't just implementing Display, and deriving Serialize/Deserialize
be enough?

> +
> +impl SyncDirection {
> +    pub fn as_config_type_str(&self) -> &'static str {
> +        match self {
> +            SyncDirection::Pull => "sync",
> +            SyncDirection::Push => "sync-push",
> +        }
> +    }
> +}
> +
>  #[api(
>      properties: {
>          id: {
> diff --git a/pbs-config/src/sync.rs b/pbs-config/src/sync.rs
> index 45453abb1..143b73e78 100644
> --- a/pbs-config/src/sync.rs
> +++ b/pbs-config/src/sync.rs
> @@ -18,9 +18,16 @@ fn init() -> SectionConfig {
>          _ => unreachable!(),
>      };
>  
> -    let plugin = SectionConfigPlugin::new("sync".to_string(), Some(String::from("id")), obj_schema);
> +    let pull_plugin =
> +        SectionConfigPlugin::new("sync".to_string(), Some(String::from("id")), obj_schema);
> +    let push_plugin = SectionConfigPlugin::new(
> +        "sync-push".to_string(),

these here should probably use SyncDirection's as_config_type_str to
make the connection obvious when adapting that at some point in the
future?

> +        Some(String::from("id")),
> +        obj_schema,
> +    );
>      let mut config = SectionConfig::new(&JOB_ID_SCHEMA);
> -    config.register_plugin(plugin);
> +    config.register_plugin(pull_plugin);
> +    config.register_plugin(push_plugin);
>  
>      config
>  }
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel