[pbs-devel] [PATCH v2 proxmox-backup 0/5] remote scanning

[pbs-devel] [PATCH v2 proxmox-backup 0/5] remote scanning

[Fabian Grünbichler]

v1->v2:
- drop schedule GUI patch (alternative already applied to master)
- incorporated review feedback
- patches 4/5 are new

Fabian Grünbichler (5):
  types: extract DataStoreListItem
  api: refactor remote client and add remote scan
  www: add remote store selector
  remote.cfg: rename userid to 'auth-id'
  remote.cfg: rename password to secret

 debian/postinst                   |  10 +++
 src/api2/admin/datastore.rs       |  23 +++----
 src/api2/config/remote.rs         | 107 +++++++++++++++++++++++++-----
 src/api2/pull.rs                  |  14 +---
 src/api2/types/mod.rs             |  19 ++++++
 src/bin/proxmox-backup-manager.rs |  26 ++------
 src/config/remote.rs              |  13 ++--
 www/config/RemoteView.js          |   6 +-
 www/form/RemoteSelector.js        |   4 +-
 www/window/RemoteEdit.js          |  14 ++--
 www/window/SyncJobEdit.js         |  97 ++++++++++++++++++++++++++-
 11 files changed, 250 insertions(+), 83 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH v2 proxmox-backup 1/5] types: extract DataStoreListItem

[Fabian Grünbichler]

for reuse in remote scan API call

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/admin/datastore.rs | 23 ++++++++---------------
 src/api2/types/mod.rs       | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index a5d3e979..b051e8dd 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -902,15 +902,7 @@ pub fn garbage_collection_status(
         type: Array,
         items: {
             description: "Datastore name and description.",
-            properties: {
-                store: {
-                    schema: DATASTORE_SCHEMA,
-                },
-                comment: {
-                    optional: true,
-                    schema: SINGLE_LINE_COMMENT_SCHEMA,
-                },
-            },
+            type: DataStoreListItem,
         },
     },
     access: {
@@ -922,7 +914,7 @@ fn get_datastore_list(
     _param: Value,
     _info: &ApiMethod,
     rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Value, Error> {
+) -> Result<Vec<DataStoreListItem>, Error> {
 
     let (config, _digest) = datastore::config()?;
 
@@ -935,11 +927,12 @@ fn get_datastore_list(
         let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
         let allowed = (user_privs & (PRIV_DATASTORE_AUDIT| PRIV_DATASTORE_BACKUP)) != 0;
         if allowed {
-            let mut entry = json!({ "store": store });
-            if let Some(comment) = data["comment"].as_str() {
-                entry["comment"] = comment.into();
-            }
-            list.push(entry);
+            list.push(
+                DataStoreListItem {
+                    store: store.clone(),
+                    comment: data["comment"].as_str().map(String::from),
+                }
+            );
         }
     }
 
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 7ee89f57..31fd89d2 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -370,6 +370,25 @@ pub const BLOCKDEVICE_NAME_SCHEMA: Schema = StringSchema::new("Block device name
 
 // Complex type definitions
 
+#[api(
+    properties: {
+        store: {
+            schema: DATASTORE_SCHEMA,
+        },
+        comment: {
+            optional: true,
+            schema: SINGLE_LINE_COMMENT_SCHEMA,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all="kebab-case")]
+/// Basic information about a datastore.
+pub struct DataStoreListItem {
+    pub store: String,
+    pub comment: Option<String>,
+}
+
 #[api(
     properties: {
         "backup-type": {
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 proxmox-backup 1/5] types: extract DataStoreListItem

[Thomas Lamprecht]

On 05.11.20 12:12, Fabian Grünbichler wrote:
> for reuse in remote scan API call
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  src/api2/admin/datastore.rs | 23 ++++++++---------------
>  src/api2/types/mod.rs       | 19 +++++++++++++++++++
>  2 files changed, 27 insertions(+), 15 deletions(-)
> 
>

applied, thanks!

[pbs-devel] [PATCH v2 proxmox-backup 2/5] api: refactor remote client and add remote scan

[Fabian Grünbichler]

to allow on-demand scanning of remote datastores accessible for the
configured remote user.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v2:
    - map remote API errors to 500 to prevent client confusion

 src/api2/config/remote.rs         | 79 ++++++++++++++++++++++++++++++-
 src/api2/pull.rs                  | 12 +----
 src/bin/proxmox-backup-manager.rs | 26 ++--------
 3 files changed, 84 insertions(+), 33 deletions(-)

diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index ffbba1d2..f053ef36 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -1,11 +1,13 @@
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
 use serde_json::Value;
 use ::serde::{Deserialize, Serialize};
 
 use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
+use proxmox::http_err;
 use proxmox::tools::fs::open_file_locked;
 
 use crate::api2::types::*;
+use crate::client::{HttpClient, HttpClientOptions};
 use crate::config::cached_user_info::CachedUserInfo;
 use crate::config::remote;
 use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
@@ -301,10 +303,83 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
     Ok(())
 }
 
+/// Helper to get client for remote.cfg entry
+pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
+    let options = HttpClientOptions::new()
+        .password(Some(remote.password.clone()))
+        .fingerprint(remote.fingerprint.clone());
+
+    let client = HttpClient::new(
+        &remote.host,
+        remote.port.unwrap_or(8007),
+        &remote.userid,
+        options)?;
+    let _auth_info = client.login() // make sure we can auth
+        .await
+        .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
+
+    Ok(client)
+}
+
+
+#[api(
+    input: {
+        properties: {
+            name: {
+                schema: REMOTE_ID_SCHEMA,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Privilege(&["remote", "{name}"], PRIV_REMOTE_AUDIT, false),
+    },
+    returns: {
+        description: "List the accessible datastores.",
+        type: Array,
+        items: {
+            description: "Datastore name and description.",
+            type: DataStoreListItem,
+        },
+    },
+)]
+/// List datastores of a remote.cfg entry
+pub async fn scan_remote_datastores(name: String) -> Result<Vec<DataStoreListItem>, Error> {
+    let (remote_config, _digest) = remote::config()?;
+    let remote: remote::Remote = remote_config.lookup("remote", &name)?;
+
+    let map_remote_err = |api_err| {
+        http_err!(INTERNAL_SERVER_ERROR,
+                  "failed to scan remote '{}' - {}",
+                  &name,
+                  api_err)
+    };
+
+    let client = remote_client(remote)
+        .await
+        .map_err(map_remote_err)?;
+    let api_res = client
+        .get("api2/json/admin/datastore", None)
+        .await
+        .map_err(map_remote_err)?;
+    let parse_res = match api_res.get("data") {
+        Some(data) => serde_json::from_value::<Vec<DataStoreListItem>>(data.to_owned()),
+        None => bail!("remote {} did not return any datastore list data", &name),
+    };
+
+    match parse_res {
+        Ok(parsed) => Ok(parsed),
+        Err(_) => bail!("Failed to parse remote scan api result."),
+    }
+}
+
+const SCAN_ROUTER: Router = Router::new()
+    .get(&API_METHOD_SCAN_REMOTE_DATASTORES);
+
 const ITEM_ROUTER: Router = Router::new()
     .get(&API_METHOD_READ_REMOTE)
     .put(&API_METHOD_UPDATE_REMOTE)
-    .delete(&API_METHOD_DELETE_REMOTE);
+    .delete(&API_METHOD_DELETE_REMOTE)
+    .subdirs(&[("scan", &SCAN_ROUTER)]);
 
 pub const ROUTER: Router = Router::new()
     .get(&API_METHOD_LIST_REMOTES)
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index d9e9d31d..87015c72 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -9,7 +9,7 @@ use proxmox::api::{ApiMethod, Router, RpcEnvironment, Permission};
 
 use crate::server::{WorkerTask, jobstate::Job};
 use crate::backup::DataStore;
-use crate::client::{HttpClient, HttpClientOptions, BackupRepository, pull::pull_store};
+use crate::client::{HttpClient, BackupRepository, pull::pull_store};
 use crate::api2::types::*;
 use crate::config::{
     remote,
@@ -50,17 +50,9 @@ pub async fn get_pull_parameters(
     let (remote_config, _digest) = remote::config()?;
     let remote: remote::Remote = remote_config.lookup("remote", remote)?;
 
-    let options = HttpClientOptions::new()
-        .password(Some(remote.password.clone()))
-        .fingerprint(remote.fingerprint.clone());
-
     let src_repo = BackupRepository::new(Some(remote.userid.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string());
 
-    let client = HttpClient::new(&src_repo.host(), src_repo.port(), &src_repo.auth_id(), options)?;
-    let _auth_info = client.login() // make sure we can auth
-        .await
-        .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
-
+    let client = crate::api2::config::remote::remote_client(remote).await?;
 
     Ok((client, src_repo, tgt_store))
 }
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 7499446b..e52c2f76 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -413,29 +413,13 @@ pub fn complete_remote_datastore_name(_arg: &str, param: &HashMap<String, String
 
     let _ = proxmox::try_block!({
         let remote = param.get("remote").ok_or_else(|| format_err!("no remote"))?;
-        let (remote_config, _digest) = config::remote::config()?;
 
-        let remote: config::remote::Remote = remote_config.lookup("remote", &remote)?;
+        let data = crate::tools::runtime::block_on(async move {
+            crate::api2::config::remote::scan_remote_datastores(remote.clone()).await
+        })?;
 
-        let options = HttpClientOptions::new()
-            .password(Some(remote.password.clone()))
-            .fingerprint(remote.fingerprint.clone());
-
-        let client = HttpClient::new(
-            &remote.host,
-            remote.port.unwrap_or(8007),
-            &remote.userid,
-            options,
-        )?;
-
-        let result = crate::tools::runtime::block_on(client.get("api2/json/admin/datastore", None))?;
-
-        if let Some(data) = result["data"].as_array() {
-            for item in data {
-                if let Some(store) = item["store"].as_str() {
-                    list.push(store.to_owned());
-                }
-            }
+        for item in data {
+            list.push(item.store);
         }
 
         Ok(())
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 proxmox-backup 2/5] api: refactor remote client and add remote scan

[Thomas Lamprecht]

On 05.11.20 12:12, Fabian Grünbichler wrote:
> to allow on-demand scanning of remote datastores accessible for the
> configured remote user.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     v2:
>     - map remote API errors to 500 to prevent client confusion
> 
>  src/api2/config/remote.rs         | 79 ++++++++++++++++++++++++++++++-
>  src/api2/pull.rs                  | 12 +----
>  src/bin/proxmox-backup-manager.rs | 26 ++--------
>  3 files changed, 84 insertions(+), 33 deletions(-)
> 
>

applied, thanks!

[pbs-devel] [PATCH v2 proxmox-backup 3/5] www: add remote store selector

[Fabian Grünbichler]

(hopefully) improved upon NFS export selection in PVE

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v2:
    - rename actualChange to firstLoad and add comment
    - drop monStoreErrors (Dominik sent a patch to do this directly and properly in ComboGrid)

 www/window/SyncJobEdit.js | 97 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index e4e47fa4..c3c0e228 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -1,3 +1,90 @@
+Ext.define('PBS.form.RemoteStoreSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: 'widget.pbsRemoteStoreSelector',
+
+    queryMode: 'local',
+
+    valueField: 'store',
+    displayField: 'store',
+    notFoundIsValid: true,
+
+    matchFieldWidth: false,
+    listConfig: {
+	loadingText: gettext('Scanning...'),
+	width: 350,
+	columns: [
+	    {
+		header: gettext('Datastore'),
+		sortable: true,
+		dataIndex: 'store',
+		renderer: Ext.String.htmlEncode,
+		flex: 1,
+	    },
+	    {
+		header: gettext('Comment'),
+		dataIndex: 'comment',
+		renderer: Ext.String.htmlEncode,
+		flex: 1,
+	    },
+	],
+    },
+
+    doRawQuery: function() {
+	// do nothing.
+    },
+
+    setRemote: function(remote) {
+	let me = this;
+
+	if (me.remote === remote) {
+	    return;
+	}
+
+	me.remote = remote;
+
+	let store = me.store;
+	store.removeAll();
+
+	if (me.remote) {
+	    me.setDisabled(false);
+	    if (!me.firstLoad) {
+		me.clearValue();
+	    }
+
+	    store.proxy.url = '/api2/json/config/remote/' + encodeURIComponent(me.remote) + '/scan';
+	    store.load();
+
+	    me.firstLoad = false;
+	} else {
+	    me.setDisabled(true);
+	    me.clearValue();
+	}
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	me.firstLoad = true;
+
+	let store = Ext.create('Ext.data.Store', {
+	    fields: ['store', 'comment'],
+	    proxy: {
+		type: 'proxmox',
+		url: '/api2/json/config/remote/' + encodeURIComponent(me.remote) + '/scan',
+	    },
+	});
+
+	store.sort('store', 'ASC');
+
+	Ext.apply(me, {
+	    store: store,
+	});
+
+	me.callParent();
+    },
+});
+
+
 Ext.define('PBS.window.SyncJobEdit', {
     extend: 'Proxmox.window.Edit',
     alias: 'widget.pbsSyncJobEdit',
@@ -80,12 +167,20 @@ Ext.define('PBS.window.SyncJobEdit', {
 		xtype: 'pbsRemoteSelector',
 		allowBlank: false,
 		name: 'remote',
+		listeners: {
+		    change: function(f, value) {
+			let me = this;
+			let remoteStoreField = me.up('pbsSyncJobEdit').down('field[name=remote-store]');
+			remoteStoreField.setRemote(value);
+		    },
+		},
 	    },
 	    {
 		fieldLabel: gettext('Source Datastore'),
-		xtype: 'proxmoxtextfield',
+		xtype: 'pbsRemoteStoreSelector',
 		allowBlank: false,
 		name: 'remote-store',
+		disabled: true,
 	    },
 	    {
 		fieldLabel: gettext('Sync Schedule'),
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 proxmox-backup 3/5] www: add remote store selector

[Thomas Lamprecht]

On 05.11.20 12:12, Fabian Grünbichler wrote:
> (hopefully) improved upon NFS export selection in PVE
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     v2:
>     - rename actualChange to firstLoad and add comment
>     - drop monStoreErrors (Dominik sent a patch to do this directly and properly in ComboGrid)
> 
>  www/window/SyncJobEdit.js | 97 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

[pbs-devel] [PATCH v2 proxmox-backup 4/5] remote.cfg: rename userid to 'auth-id'

[Fabian Grünbichler]

and fixup config file on upgrades accordingly

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 debian/postinst            |  9 +++++++++
 src/api2/config/remote.rs  | 10 +++++-----
 src/api2/pull.rs           |  2 +-
 src/config/remote.rs       |  5 +++--
 www/config/RemoteView.js   |  6 +++---
 www/form/RemoteSelector.js |  4 ++--
 www/window/RemoteEdit.js   |  6 ++----
 7 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/debian/postinst b/debian/postinst
index af5ee097..6a0cf9fd 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -28,6 +28,15 @@ case "$1" in
 		if dpkg --compare-versions "$2" 'le' '0.9.5-1'; then
 			chown --quiet backup:backup /var/log/proxmox-backup/api/auth.log || true
 		fi
+		if dpkg --compare-versions "$2" 'le' '0.9.6-1'; then
+			if [ -e /etc/proxmox-backup/remote.cfg ]; then
+				echo "NOTE: Switching over remote.cfg to new field names.."
+				flock -w 30 /etc/proxmox-backup/.remote.lck \
+				    sed -i \
+				        -e 's/^\s\+userid /\tauth-id /g' \
+				        /etc/proxmox-backup/remote.cfg || true
+			fi
+		fi
 	fi
 	# FIXME: Remove in future version once we're sure no broken entries remain in anyone's files
 	if grep -q -e ':termproxy::[^@]\+: ' /var/log/proxmox-backup/tasks/active; then
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index f053ef36..29b14472 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -78,7 +78,7 @@ pub fn list_remotes(
                 optional: true,
                 default: 8007,
             },
-            userid: {
+            "auth-id": {
                 type: Authid,
             },
             password: {
@@ -178,7 +178,7 @@ pub enum DeletableProperty {
                 type: u16,
                 optional: true,
             },
-            userid: {
+            "auth-id": {
                 optional: true,
                 type: Authid,
             },
@@ -214,7 +214,7 @@ pub fn update_remote(
     comment: Option<String>,
     host: Option<String>,
     port: Option<u16>,
-    userid: Option<Authid>,
+    auth_id: Option<Authid>,
     password: Option<String>,
     fingerprint: Option<String>,
     delete: Option<Vec<DeletableProperty>>,
@@ -252,7 +252,7 @@ pub fn update_remote(
     }
     if let Some(host) = host { data.host = host; }
     if port.is_some() { data.port = port; }
-    if let Some(userid) = userid { data.userid = userid; }
+    if let Some(auth_id) = auth_id { data.auth_id = auth_id; }
     if let Some(password) = password { data.password = password; }
 
     if let Some(fingerprint) = fingerprint { data.fingerprint = Some(fingerprint); }
@@ -312,7 +312,7 @@ pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error>
     let client = HttpClient::new(
         &remote.host,
         remote.port.unwrap_or(8007),
-        &remote.userid,
+        &remote.auth_id,
         options)?;
     let _auth_info = client.login() // make sure we can auth
         .await
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index 87015c72..88ce750c 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -50,7 +50,7 @@ pub async fn get_pull_parameters(
     let (remote_config, _digest) = remote::config()?;
     let remote: remote::Remote = remote_config.lookup("remote", remote)?;
 
-    let src_repo = BackupRepository::new(Some(remote.userid.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string());
+    let src_repo = BackupRepository::new(Some(remote.auth_id.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string());
 
     let client = crate::api2::config::remote::remote_client(remote).await?;
 
diff --git a/src/config/remote.rs b/src/config/remote.rs
index 14c57c0e..20fd39d4 100644
--- a/src/config/remote.rs
+++ b/src/config/remote.rs
@@ -44,7 +44,7 @@ pub const REMOTE_PASSWORD_SCHEMA: Schema = StringSchema::new("Password or auth t
             description: "The (optional) port",
             type: u16,
         },
-        userid: {
+        "auth-id": {
             type: Authid,
         },
         password: {
@@ -57,6 +57,7 @@ pub const REMOTE_PASSWORD_SCHEMA: Schema = StringSchema::new("Password or auth t
     }
 )]
 #[derive(Serialize,Deserialize)]
+#[serde(rename_all = "kebab-case")]
 /// Remote properties.
 pub struct Remote {
     pub name: String,
@@ -65,7 +66,7 @@ pub struct Remote {
     pub host: String,
     #[serde(skip_serializing_if="Option::is_none")]
     pub port: Option<u16>,
-    pub userid: Authid,
+    pub auth_id: Authid,
     #[serde(skip_serializing_if="String::is_empty")]
     #[serde(with = "proxmox::tools::serde::string_as_base64")]
     pub password: String,
diff --git a/www/config/RemoteView.js b/www/config/RemoteView.js
index 685503fe..fa4c8825 100644
--- a/www/config/RemoteView.js
+++ b/www/config/RemoteView.js
@@ -1,6 +1,6 @@
 Ext.define('pmx-remotes', {
     extend: 'Ext.data.Model',
-    fields: ['name', 'host', 'port', 'userid', 'fingerprint', 'comment',
+    fields: ['name', 'host', 'port', 'auth-id', 'fingerprint', 'comment',
 	{
 	    name: 'server',
 	    calculate: function(data) {
@@ -127,11 +127,11 @@ Ext.define('PBS.config.RemoteView', {
 	    dataIndex: 'server',
 	},
 	{
-	    header: gettext('User name'),
+	    header: gettext('Auth ID'),
 	    width: 200,
 	    sortable: true,
 	    renderer: Ext.String.htmlEncode,
-	    dataIndex: 'userid',
+	    dataIndex: 'auth-id',
 	},
 	{
 	    header: gettext('Fingerprint'),
diff --git a/www/form/RemoteSelector.js b/www/form/RemoteSelector.js
index 3f55ef1f..8dae6682 100644
--- a/www/form/RemoteSelector.js
+++ b/www/form/RemoteSelector.js
@@ -29,9 +29,9 @@ Ext.define('PBS.form.RemoteSelector', {
 		flex: 1,
 	    },
 	    {
-		header: gettext('User name'),
+		header: gettext('Auth ID'),
 		sortable: true,
-		dataIndex: 'userid',
+		dataIndex: 'auth-id',
 		renderer: Ext.String.htmlEncode,
 		flex: 1,
 	    },
diff --git a/www/window/RemoteEdit.js b/www/window/RemoteEdit.js
index 56a248e0..4a4d8114 100644
--- a/www/window/RemoteEdit.js
+++ b/www/window/RemoteEdit.js
@@ -5,8 +5,6 @@ Ext.define('PBS.window.RemoteEdit', {
 
     onlineHelp: 'backup_remote',
 
-    userid: undefined,
-
     isAdd: true,
 
     subject: gettext('Remote'),
@@ -93,8 +91,8 @@ Ext.define('PBS.window.RemoteEdit', {
 	    {
 		xtype: 'proxmoxtextfield',
 		allowBlank: false,
-		name: 'userid',
-		fieldLabel: gettext('Userid'),
+		name: 'auth-id',
+		fieldLabel: gettext('Auth ID'),
 	    },
 	    {
 		xtype: 'textfield',
-- 
2.20.1

[pbs-devel] applied: [PATCH v2 proxmox-backup 4/5] remote.cfg: rename userid to 'auth-id'

[Thomas Lamprecht]

On 05.11.20 12:12, Fabian Grünbichler wrote:
> and fixup config file on upgrades accordingly
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  debian/postinst            |  9 +++++++++
>  src/api2/config/remote.rs  | 10 +++++-----
>  src/api2/pull.rs           |  2 +-
>  src/config/remote.rs       |  5 +++--
>  www/config/RemoteView.js   |  6 +++---
>  www/form/RemoteSelector.js |  4 ++--
>  www/window/RemoteEdit.js   |  6 ++----
>  7 files changed, 25 insertions(+), 17 deletions(-)
> 
>

applied, thanks!

[pbs-devel] [RFC v2 proxmox-backup 5/5] remote.cfg: rename password to secret

[Fabian Grünbichler]

to make it a bit less confusing

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
or split it into two entirely? not sure..

 debian/postinst           |  1 +
 src/api2/config/remote.rs | 22 +++++++++++-----------
 src/config/remote.rs      |  8 ++++----
 www/window/RemoteEdit.js  |  8 ++++----
 4 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/debian/postinst b/debian/postinst
index 6a0cf9fd..030b4021 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -34,6 +34,7 @@ case "$1" in
 				flock -w 30 /etc/proxmox-backup/.remote.lck \
 				    sed -i \
 				        -e 's/^\s\+userid /\tauth-id /g' \
+				        -e 's/^\s\+password /\tsecret /g' \
 				        /etc/proxmox-backup/remote.cfg || true
 			fi
 		fi
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index 29b14472..3b9c1427 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -43,7 +43,7 @@ pub fn list_remotes(
     let mut list: Vec<remote::Remote> = config.convert_to_typed_array("remote")?;
     // don't return password in api
     for remote in &mut list {
-        remote.password = "".to_string();
+        remote.secret = "".to_string();
     }
 
     let list = list
@@ -81,8 +81,8 @@ pub fn list_remotes(
             "auth-id": {
                 type: Authid,
             },
-            password: {
-                schema: remote::REMOTE_PASSWORD_SCHEMA,
+            secret: {
+                schema: remote::REMOTE_SECRET_SCHEMA,
             },
             fingerprint: {
                 optional: true,
@@ -95,12 +95,12 @@ pub fn list_remotes(
     },
 )]
 /// Create new remote.
-pub fn create_remote(password: String, param: Value) -> Result<(), Error> {
+pub fn create_remote(secret: String, param: Value) -> Result<(), Error> {
 
     let _lock = open_file_locked(remote::REMOTE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
 
     let mut data = param.clone();
-    data["password"] = Value::from(base64::encode(password.as_bytes()));
+    data["secret"] = Value::from(base64::encode(secret.as_bytes()));
     let remote: remote::Remote = serde_json::from_value(data)?;
 
     let (mut config, _digest) = remote::config()?;
@@ -140,7 +140,7 @@ pub fn read_remote(
 ) -> Result<remote::Remote, Error> {
     let (config, digest) = remote::config()?;
     let mut data: remote::Remote = config.lookup("remote", &name)?;
-    data.password = "".to_string(); // do not return password in api
+    data.secret = "".to_string(); // do not return password in api
     rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
     Ok(data)
 }
@@ -182,9 +182,9 @@ pub enum DeletableProperty {
                 optional: true,
                 type: Authid,
             },
-            password: {
+            secret: {
                 optional: true,
-                schema: remote::REMOTE_PASSWORD_SCHEMA,
+                schema: remote::REMOTE_SECRET_SCHEMA,
             },
             fingerprint: {
                 optional: true,
@@ -215,7 +215,7 @@ pub fn update_remote(
     host: Option<String>,
     port: Option<u16>,
     auth_id: Option<Authid>,
-    password: Option<String>,
+    secret: Option<String>,
     fingerprint: Option<String>,
     delete: Option<Vec<DeletableProperty>>,
     digest: Option<String>,
@@ -253,7 +253,7 @@ pub fn update_remote(
     if let Some(host) = host { data.host = host; }
     if port.is_some() { data.port = port; }
     if let Some(auth_id) = auth_id { data.auth_id = auth_id; }
-    if let Some(password) = password { data.password = password; }
+    if let Some(secret) = secret { data.secret = secret; }
 
     if let Some(fingerprint) = fingerprint { data.fingerprint = Some(fingerprint); }
 
@@ -306,7 +306,7 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
 /// Helper to get client for remote.cfg entry
 pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
     let options = HttpClientOptions::new()
-        .password(Some(remote.password.clone()))
+        .password(Some(remote.secret.clone()))
         .fingerprint(remote.fingerprint.clone());
 
     let client = HttpClient::new(
diff --git a/src/config/remote.rs b/src/config/remote.rs
index 20fd39d4..ee537c9a 100644
--- a/src/config/remote.rs
+++ b/src/config/remote.rs
@@ -21,7 +21,7 @@ lazy_static! {
     static ref CONFIG: SectionConfig = init();
 }
 
-pub const REMOTE_PASSWORD_SCHEMA: Schema = StringSchema::new("Password or auth token for remote host.")
+pub const REMOTE_SECRET_SCHEMA: Schema = StringSchema::new("Password or auth token for remote host.")
     .format(&PASSWORD_FORMAT)
     .min_length(1)
     .max_length(1024)
@@ -47,8 +47,8 @@ pub const REMOTE_PASSWORD_SCHEMA: Schema = StringSchema::new("Password or auth t
         "auth-id": {
             type: Authid,
         },
-        password: {
-            schema: REMOTE_PASSWORD_SCHEMA,
+        secret: {
+            schema: REMOTE_SECRET_SCHEMA,
         },
         fingerprint: {
             optional: true,
@@ -69,7 +69,7 @@ pub struct Remote {
     pub auth_id: Authid,
     #[serde(skip_serializing_if="String::is_empty")]
     #[serde(with = "proxmox::tools::serde::string_as_base64")]
-    pub password: String,
+    pub secret: String,
     #[serde(skip_serializing_if="Option::is_none")]
     pub fingerprint: Option<String>,
 }
diff --git a/www/window/RemoteEdit.js b/www/window/RemoteEdit.js
index 4a4d8114..b65f2da6 100644
--- a/www/window/RemoteEdit.js
+++ b/www/window/RemoteEdit.js
@@ -22,7 +22,7 @@ Ext.define('PBS.window.RemoteEdit', {
 	me.method = name ? 'PUT' : 'POST';
 	me.autoLoad = !!name;
 	return {
-	    passwordEmptyText: me.isCreate ? '' : gettext('Unchanged'),
+	    secretEmptyText: me.isCreate ? '' : gettext('Unchanged'),
 	};
     },
 
@@ -97,10 +97,10 @@ Ext.define('PBS.window.RemoteEdit', {
 	    {
 		xtype: 'textfield',
 		inputType: 'password',
-		fieldLabel: gettext('Password'),
-		name: 'password',
+		fieldLabel: gettext('Password')+'/'+gettext('Secret'),
+		name: 'secret',
 		cbind: {
-		    emptyText: '{passwordEmptyText}',
+		    emptyText: '{secretEmptyText}',
 		    allowBlank: '{!isCreate}',
 		},
 	    },
-- 
2.20.1

Re: [pbs-devel] [RFC v2 proxmox-backup 5/5] remote.cfg: rename password to secret

[Thomas Lamprecht]

On 05.11.20 12:12, Fabian Grünbichler wrote:
> to make it a bit less confusing
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> or split it into two entirely? not sure..
> 

also not so sure about this one, password is not completely wrong as
a use for more general "secret", IMO, I omit it for now but no hard
feelings against it.