Re: [pve-devel] [PATCH docs 1/2] document API tokens

2020-02-03 Thread Aaron Lauterer
Inline suggestions for restructuring some paragraphs for an easier 
understanding and some other small things.

I hope I didn't lose information in the restructured paragraphs.

On 1/30/20 1:54 PM, Fabian Grünbichler wrote:

Signed-off-by: Fabian Grünbichler 
---
Note: we don't yet have a basic 'intro to REST API' section in the user
docs. it might make sense to copy some of the related wiki content,
explain the ticketing mechanism, etc.pp.

  pveum.adoc | 53 ++---
  1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/pveum.adoc b/pveum.adoc
index 59a2824..cf5f70b 100644
--- a/pveum.adoc
+++ b/pveum.adoc
@@ -75,6 +75,31 @@ way to organize access permissions. You should always grant 
permission
  to groups instead of using individual users. That way you will get a
  much shorter access control list which is easier to handle.
  
+[[pveum_tokens]]

+API Tokens
+~~
+
+For situations where giving a system/software/API client permanent access to a
+{pve} system is required, API tokens can be generated for individual users.
+These tokens allow stateless access to most of the REST API, and can be given
+separate permissions and expiry dates to limit the scope and duration of
+access. In case an API token was compromised, it can be revoked without
+disabling the whole user.


API tokens allow stateless access to most parts of the REST API for 
another system, software or API client. They can be generated for 
individual users and can be given separate permissions and expiration 
dates to limit the scope and duration of the access. Should the API 
token get compromised it can be revoked without disabling the user itself.




+
+API tokens come in two basic types:
+
+* separated privileges: token needs to be given explicit access with ACLs,

s/token/The token/
s/ACLs,/ACLs./


+  effective permissions are calculated by intersecting user and token

s/effective/The effective/
s/user/the user/

+  permissions

s/permissions/permissions./


+* full privileges: token permissions are automatically identical to that of the
+  associated user

s/token/The token/
s/automatically// #I think this could be removed and the sentence still 
works.

s/user/user./


+
+WARNING: the token value is only displayed/returned once when initially 
generating
+the token, and not retrievable over the API later on!
WARNING: The token value is only displayed/returned *once* when the 
token is generated. It cannot be retrieved over the API at a later time!



+
+To use an API token, simple set the HTTP header 'Authorization' to the

s/simple//


+displayed value of the form `PVEAPIToken=USER@REALM!TOKENID=UUID` when making

s/of/in/
s/form/form of/

Would it make sense to set the example to 
`PVEAPIToken=@!TOKENID=` or some other 
placeholder that will make it more obvious where data needs to be inserted?


Is the header really mixed in regards to Capitalization?


+API requests, or refer to your API client documentation.
  
  [[pveum_authentication_realms]]

  Authentication Realms
@@ -283,10 +308,10 @@ deleting a parts of a VM configuration), the user needs 
to have the
  appropriate permissions.
  
  {pve} uses a role and path based permission management system. An entry in

-the permissions table allows a user or group to take on a specific role
+the permissions table allows a user, group or token to take on a specific role
  when accessing an 'object' or 'path'. This means an such an access rule can
-be represented as a triple of '(path, user, role)' or '(path, group,
-role)', with the role containing a set of allowed actions, and the path
+be represented as a triple of '(path, user, role)', '(path, group,
+role)' or '(path, token, role)', with the role containing a set of allowed 
actions, and the path
  representing the target of these actions.
  
  
@@ -419,6 +444,8 @@ by default). We use the following inheritance rules:

  * Permissions for groups apply when the user is member of that group.
  * Permissions replace the ones inherited from an upper level.
  
+Additionally, privilege separated tokens can never have a permission on any

+given path that their associated user does not have.
  
  [[pveum_pools]]

  Pools
@@ -597,6 +624,26 @@ are members of group `customers`:
  NOTE: The user is able to add other users, but only if they are
  members of group `customers` and within realm `pve`.
  
+Limited API token for monitoring

+
+
+Given a user `joe@pve` with PVEVMAdmin role on all vms:

s/with/with the/
s/vms/VMs/


+
+[source,bash]
+ pveum aclmod /vms -user joe@pve -role PVEVMAdmin
+
+Add a new API token with separate privileges, which is only allowed to view VM
+information (e.g., for monitoring purposes):
+
+[source,bash]
+ pveum user token add joe@pve monitoring -privsep 1
+ pveum aclmod /vms -token 'joe@pve!monitoring' -role PVEAuditor
+
+Verify permissions of user and token:

s/permissions/the permissions/
s/user/the user/

+
+[sourc

[pve-devel] applied: [PATCH ifupdown2 0/2] ifupdown 2.0 fixes

2020-02-03 Thread Wolfgang Bumiller
applied both patches

On Mon, Feb 03, 2020 at 06:30:27AM +0100, Alexandre Derumier wrote:
> - 1 fix for ovs not having ip address setup on start
> - 1 fix for new state_dir feature, where directory need to be on tmpfs
> 
> Alexandre Derumier (2):
>   patch: execute addons scripts before modules
>   patch: config_tuning: state_dir=/run/network/
> 
>  debian/patches/pve/0006-config-tuning.patch   | 16 +++--
>  ...xecute-addons-scripts-before-modules.patch | 71 +++
>  debian/patches/series |  1 +
>  3 files changed, 83 insertions(+), 5 deletions(-)
>  create mode 100644 
> debian/patches/pve/0008-execute-addons-scripts-before-modules.patch
> 
> -- 
> 2.20.1

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] [PATCH widget-toolkit] ComboGrid: fix validation for !allowBlank disabled fields

2020-02-03 Thread Stefan Reiter
Used in "Add USB to VM" dialog for example.

This was broken before 15206214d9 "ComboGrid: fix on-load validation for blank
values" (only the one you enabled first was validated, the other always showed
as valid), and afterwards too, but in a different way (both are now immediately
marked invalid until you select and unselect them) - which is how I noticed.

With this the validation now works correctly.

Signed-off-by: Stefan Reiter 
---

Before my last patch they didn't validate enough, now they validate too much...
I tried my best to test all of our ComboGrids once again, hope I didn't miss
anything this time.

 form/ComboGrid.js | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/form/ComboGrid.js b/form/ComboGrid.js
index 54bc239..9bdf721 100644
--- a/form/ComboGrid.js
+++ b/form/ComboGrid.js
@@ -378,6 +378,13 @@ Ext.define('Proxmox.form.ComboGrid', {
return true;
 },
 
+// validate after enabling a field, otherwise blank fields with !allowBlank
+// are sometimes not marked as invalid
+setDisabled: function(value) {
+   this.callParent([value]);
+   this.validate();
+},
+
 initComponent: function() {
var me = this;
 
@@ -461,7 +468,7 @@ Ext.define('Proxmox.form.ComboGrid', {
me.setValue(def, true);
} else if (!me.allowBlank && !(Ext.isArray(def) ? 
def.length : def)) {
me.setValue(def);
-   if (!me.notFoundIsValid) {
+   if (!me.notFoundIsValid && !me.isDisabled()) {
me.markInvalid(me.blankText);
}
}
-- 
2.20.1


___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] [PATCH qemu-server] fix #2578: check if $target is provided in clone

2020-02-03 Thread Oguz Bektas
regression introduced with commit a85ff91b

previously we set $target to undef if it's localnode or localhost, then
we check if node exists.

with regression commit, behaviour changes as we do the node check in
else, but $target may be undef. this causes an error:

no such cluster node ''

Signed-off-by: Oguz Bektas 
---
 PVE/API2/Qemu.pm | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index e15c0c3..fe68e87 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -2749,10 +2749,12 @@ __PACKAGE__->register_method({
 
 my $localnode = PVE::INotify::nodename();
 
-if ($target && ($target eq $localnode || $target eq 'localhost')) {
-   undef $target;
-   } else {
-   PVE::Cluster::check_node_exists($target);
+   if ($target) {
+   if ($target eq $localnode || $target eq 'localhost') {
+   undef $target;
+   } else {
+   PVE::Cluster::check_node_exists($target);
+   }
}
 
my $storecfg = PVE::Storage::config();
-- 
2.20.1

___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH manager v2 1/2] Fix #2124: Add support for zstd

2020-02-03 Thread Stefan Reiter

On 1/31/20 5:00 PM, Alwin Antreich wrote:

Adds the zstd to the compression selection for backup on the GUI and the
.zst extension to the backup file filter.

Signed-off-by: Alwin Antreich 
---

  PVE/VZDump.pm| 6 --
  www/manager6/form/CompressionSelector.js | 3 ++-
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 3caa7ab8..21032fd6 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -592,6 +592,8 @@ sub compressor_info {
} else {
return ('gzip --rsyncable', 'gz');
}
+} elsif ($opt_compress eq 'zstd') {
+   return ('zstd', 'zst');


Did some testing, two things I noticed, first one regarding this patch 
especially:


1) By default zstd uses only one core. I feel like this should be 
increased (or made configurable as with pigz?). Also, zstd has an 
'--rsyncable flag' like gzip, might be good to include that too 
(according to the man page it only has a 'negligible impact on 
compression ratio').


2) The task log is somewhat messed up... It seems zstd prints a status 
as well, additionally to our own progress meter:



_03-17_05_09.vma.zst : 13625 MB... progress 94% (read 32298172416 
bytes, duration 34 sec)


_03-17_05_09.vma.zst : 13668 MB...
_03-17_05_09.vma.zst : 13721 MB...
_03-17_05_09.vma.zst : 13766 MB...
_03-17_05_09.vma.zst : 13821 MB...
_03-17_05_09.vma.zst : 13869 MB...
_03-17_05_09.vma.zst : 13933 MB... progress 95% (read 32641777664 
bytes, duration 35 sec)


_03-17_05_09.vma.zst : 14014 MB...
_03-17_05_09.vma.zst : 14091 MB...


Looks a bit unsightly IMO.

But functionality wise it works fine, tried with a VM and a container, so

Tested-by: Stefan Reiter 

for the series.


  } else {
die "internal error - unknown compression option '$opt_compress'";
  }
@@ -603,7 +605,7 @@ sub get_backup_file_list {
  my $bklist = [];
  foreach my $fn (<$dir/${bkname}-*>) {
next if $exclude_fn && $fn eq $exclude_fn;
-   if ($fn =~ 
m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|((tar|vma)(\.(gz|lzo))?)))$!)
 {
+   if ($fn =~ 
m!/(${bkname}-(\d{4})_(\d{2})_(\d{2})-(\d{2})_(\d{2})_(\d{2})\.(tgz|((tar|vma)(\.(gz|lzo|zst))?)))$!)
 {
$fn = "$dir/$1"; # untaint
my $t = timelocal ($7, $6, $5, $4, $3 - 1, $2);
push @$bklist, [$fn, $t];
@@ -863,7 +865,7 @@ sub exec_backup_task {
debugmsg ('info', "delete old backup '$d->[0]'", $logfd);
unlink $d->[0];
my $logfn = $d->[0];
-   $logfn =~ s/\.(tgz|((tar|vma)(\.(gz|lzo))?))$/\.log/;
+   $logfn =~ s/\.(tgz|((tar|vma)(\.(gz|lzo|zst))?))$/\.log/;
unlink $logfn;
}
}
diff --git a/www/manager6/form/CompressionSelector.js 
b/www/manager6/form/CompressionSelector.js
index 8938fc0e..e8775e71 100644
--- a/www/manager6/form/CompressionSelector.js
+++ b/www/manager6/form/CompressionSelector.js
@@ -4,6 +4,7 @@ Ext.define('PVE.form.CompressionSelector', {
  comboItems: [
  ['0', Proxmox.Utils.noneText],
  ['lzo', 'LZO (' + gettext('fast') + ')'],
-['gzip', 'GZIP (' + gettext('good') + ')']
+['gzip', 'GZIP (' + gettext('good') + ')'],
+['zstd', 'ZSTD (' + gettext('better') + ')']
  ]
  });



___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel