public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools
@ 2023-11-20  7:22 Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH access-control 1/2] fix #1148: allow up to three levels of pool nesting Fabian Grünbichler
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2023-11-20  7:22 UTC (permalink / raw)
  To: pve-devel

this series extends the poolname to allow (for now) two '/' inside the
name to nest pools like this:

parent/child/grandchild

since '/' is a bad character for path parameters, some API shuffling is
needed. other approaches that were tried but discarded, or rejected
outright:
- urlencoding
- systemd-escape
- manual encoding/escaping
- just encoding the parent/children in user.cfg, but not in the pool name

switching over some of the UI pool lists to also nest them visually should
be do-able as a follow-up.

changes since RFC:
- some style improvements, RE adaptation

pve-access-control:

Fabian Grünbichler (2):
  allow up to three levels of pool nesting
  pools: record parent/subpool information

 src/PVE/AccessControl.pm  | 26 +++++++++++++++++++++++---
 src/test/parser_writer.pl |  4 ++++
 src/test/perm-test6.pl    | 16 ++++++++++++++++
 src/test/test6.cfg        |  5 +++++
 4 files changed, 48 insertions(+), 3 deletions(-)

pve-manager:

Fabian Grünbichler (2):
  api: pools: support nested pools
  ui: pools: switch to new API endpoints

 PVE/API2/Pool.pm                 | 243 +++++++++++++++++++++++--------
 www/manager6/dc/PoolView.js      |   3 +
 www/manager6/grid/PoolMembers.js |  14 +-
 www/manager6/pool/StatusView.js  |   2 +-
 4 files changed, 196 insertions(+), 66 deletions(-)

-- 
2.39.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH access-control 1/2] fix #1148: allow up to three levels of pool nesting
  2023-11-20  7:22 [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools Fabian Grünbichler
@ 2023-11-20  7:22 ` Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH access-control 2/2] pools: record parent/subpool information Fabian Grünbichler
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2023-11-20  7:22 UTC (permalink / raw)
  To: pve-devel

with ACLs being inherited along the pool hierarchy.

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

Notes:
    v1: encode max pool nesting in REs

 src/PVE/AccessControl.pm | 10 ++++++++--
 src/test/perm-test6.pl   | 16 ++++++++++++++++
 src/test/test6.cfg       |  5 +++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 9600e59..4e3b077 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1264,7 +1264,7 @@ sub check_path {
 	|/nodes
 	|/nodes/[[:alnum:]\.\-\_]+
 	|/pool
-	|/pool/[[:alnum:]\.\-\_]+
+	|/pool/[A-Za-z0-9\.\-_]+(?:/[A-Za-z0-9\.\-_]+){0,2}
 	|/sdn
 	|/sdn/controllers
 	|/sdn/controllers/[[:alnum:]\_\-]+
@@ -1318,8 +1318,14 @@ PVE::JSONSchema::register_format('pve-poolid', \&verify_poolname);
 sub verify_poolname {
     my ($poolname, $noerr) = @_;
 
-    if ($poolname !~ m/^[A-Za-z0-9\.\-_]+$/) {
+    if (split("/", $poolname) > 3) {
+	die "pool name '$poolname' nested too deeply (max levels = 3)\n" if !$noerr;
 
+	return undef;
+    }
+
+    # also adapt check_path above if changed!
+    if ($poolname !~ m!^[A-Za-z0-9\.\-_]+(?:/[A-Za-z0-9\.\-_]+){0,2}$!) {
 	die "pool name '$poolname' contains invalid characters\n" if !$noerr;
 
 	return undef;
diff --git a/src/test/perm-test6.pl b/src/test/perm-test6.pl
index 0b0d036..c2d40fc 100755
--- a/src/test/perm-test6.pl
+++ b/src/test/perm-test6.pl
@@ -75,6 +75,22 @@ check_roles('User4@pve', '/vms/500', '');
 # with pool
 check_permissions('User4@pve', '/vms/500', '');
 
+# without pool, checking no access on parent pool
+check_roles('intern@pve', '/vms/600', '');
+# once more, with VM in nested pool
+check_roles('intern@pve', '/vms/700', '');
+# with propagated ACL
+check_roles('User4@pve', '/vms/700', '');
+# with pool, checking no access on parent pool
+check_permissions('intern@pve', '/vms/600', '');
+# once more, with VM in nested pool
+check_permissions('intern@pve', '/vms/700', 'VM.Audit');
+# with propagated ACL
+check_permissions('User4@pve', '/vms/700', 'VM.Console');
+
+# check nested pool permissions
+check_roles('intern@pve', '/pool/marketing/interns', 'RoleINTERN');
+check_roles('User4@pve', '/pool/marketing/interns', 'RoleMARKETING');
 
 check_permissions('User1@pve', '/vms/600', 'VM.Console');
 check_permissions('User2@pve', '/vms/600', 'VM.Console');
diff --git a/src/test/test6.cfg b/src/test/test6.cfg
index 4986910..661f56a 100644
--- a/src/test/test6.cfg
+++ b/src/test/test6.cfg
@@ -2,16 +2,20 @@ user:User1@pve:1:
 user:User2@pve:1:
 user:User3@pve:1:
 user:User4@pve:1:
+user:intern@pve:1:
 
 group:DEVEL:User1@pve,User2@pve,User3@pve:
 group:MARKETING:User1@pve,User4@pve:
+group:INTERNS:intern@pve:
 
 role:RoleDEVEL:VM.PowerMgmt:
 role:RoleMARKETING:VM.Console:
+role:RoleINTERN:VM.Audit:
 role:RoleTEST1:VM.Console:
 
 acl:1:/pool/devel:@DEVEL:RoleDEVEL:
 acl:1:/pool/marketing:@MARKETING:RoleMARKETING:
+acl:1:/pool/marketing/interns:@INTERNS:RoleINTERN:
 
 acl:1:/vms:@DEVEL:RoleTEST1:
 acl:1:/vms:User3@pve:NoAccess:
@@ -19,3 +23,4 @@ acl:1:/vms/300:@MARKETING:RoleTEST1:
 
 pool:devel:MITS development:500,501,502:store1 store2:
 pool:marketing:MITS marketing:600:store1:
+pool:marketing/interns:MITS marketing intern:700:store3:
-- 
2.39.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH access-control 2/2] pools: record parent/subpool information
  2023-11-20  7:22 [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH access-control 1/2] fix #1148: allow up to three levels of pool nesting Fabian Grünbichler
@ 2023-11-20  7:22 ` Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH manager 1/2] fix #1148: api: pools: support nested pools Fabian Grünbichler
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2023-11-20  7:22 UTC (permalink / raw)
  To: pve-devel

and ensure a missing intermediate pool exists at all times.

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

Notes:
    v1: adapt RE based on Wolfgang's feedback, post-if style
    
    a "missing link" should never happen when modifying via the API (both deletion
    with children and addition without the parent existing is blocked there), but
    it could happen when manually editing the config.

 src/PVE/AccessControl.pm  | 16 +++++++++++++++-
 src/test/parser_writer.pl |  4 ++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
index 4e3b077..461a64e 100644
--- a/src/PVE/AccessControl.pm
+++ b/src/PVE/AccessControl.pm
@@ -1529,7 +1529,21 @@ sub parse_user_config {
 	    }
 
 	    # make sure to add the pool (even if there are no members)
-	    $cfg->{pools}->{$pool} = { vms => {}, storage => {} } if !$cfg->{pools}->{$pool};
+	    $cfg->{pools}->{$pool} = { vms => {}, storage => {}, pools => {} }
+		if !$cfg->{pools}->{$pool};
+
+	    if ($pool =~ m!/!) {
+		my $curr = $pool;
+		while ($curr =~ m!^(.+)/[^/]+$!) {
+		    # ensure nested pool info is correctly recorded
+		    my $parent = $1;
+		    $cfg->{pools}->{$curr}->{parent} = $parent;
+		    $cfg->{pools}->{$parent} = { vms => {}, storage => {}, pools => {} }
+			if !$cfg->{pools}->{$parent};
+		    $cfg->{pools}->{$parent}->{pools}->{$curr} = 1;
+		    $curr = $parent;
+		}
+	    }
 
 	    $cfg->{pools}->{$pool}->{comment} = PVE::Tools::decode_text($comment) if $comment;
 
diff --git a/src/test/parser_writer.pl b/src/test/parser_writer.pl
index 65a70eb..80c346b 100755
--- a/src/test/parser_writer.pl
+++ b/src/test/parser_writer.pl
@@ -237,21 +237,25 @@ my $default_cfg = {
 	'id' => 'testpool',
 	vms => {},
 	storage => {},
+	pools => {},
     },
     test_pool_members => {
 	'id' => 'testpool',
 	vms => { 123 => 1, 1234 => 1},
 	storage => { 'local' => 1, 'local-zfs' => 1},
+	pools => {},
     },
     test_pool_duplicate_vms => {
 	'id' => 'test_duplicate_vms',
 	vms => {},
 	storage => {},
+	pools => {},
     },
     test_pool_duplicate_storages => {
 	'id' => 'test_duplicate_storages',
 	vms => {},
 	storage => { 'local' => 1, 'local-zfs' => 1},
+	pools => {},
     },
     acl_simple_user => {
 	'path' => '/',
-- 
2.39.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH manager 1/2] fix #1148: api: pools: support nested pools
  2023-11-20  7:22 [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH access-control 1/2] fix #1148: allow up to three levels of pool nesting Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH access-control 2/2] pools: record parent/subpool information Fabian Grünbichler
@ 2023-11-20  7:22 ` Fabian Grünbichler
  2023-11-20  7:22 ` [pve-devel] [PATCH manager 2/2] ui: pools: switch to new API endpoints Fabian Grünbichler
  2023-11-20 11:27 ` [pve-devel] applied-series: [PATCH access-control/manager 0/4] fix #1148: nested pools Wolfgang Bumiller
  4 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2023-11-20  7:22 UTC (permalink / raw)
  To: pve-devel

since poolid can now contain `/`, it's not possible to use it (properly) as
path parameter anymore.

accordingly:
- merge `read_pool` (`GET /pools/{poolid}`) into 'index' (`GET
  /pools/?poolid={poolid}`) (requires clients to extract the only member of the returned array if they want to query an individual pool)
- move `update_pool` to `/pools`, deprecating the old variant with path parameter
- move `delete_pool` to `/pools`, deprecating the old variant with path parameter
- deprecate `read_pool` API endpoint

pool creation is blocked for nested pools where the parent does not already
exist. similarly, the checks for deletion are extended to block deletion if
sub-pools still exist.

the old API endpoints continue to work for non-nested pools. `pvesh ls /pools`
is semi-broken for nested pools, listing the entries, but no methods on them,
since they reference the old API. fixing this would require extending the REST
handling to support a new type of child reference.

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

Notes:
    requires bumped pve-access-control

 PVE/API2/Pool.pm | 243 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 184 insertions(+), 59 deletions(-)

diff --git a/PVE/API2/Pool.pm b/PVE/API2/Pool.pm
index 51ac71941..54e744558 100644
--- a/PVE/API2/Pool.pm
+++ b/PVE/API2/Pool.pm
@@ -20,14 +20,26 @@ __PACKAGE__->register_method ({
     name => 'index',
     path => '',
     method => 'GET',
-    description => "Pool index.",
+    description => "List pools or get pool configuration.",
     permissions => {
-	description => "List all pools where you have Pool.Audit permissions on /pool/<pool>.",
+	description => "List all pools where you have Pool.Audit permissions on /pool/<pool>, or the pool specific with {poolid}",
 	user => 'all',
     },
     parameters => {
 	additionalProperties => 0,
-	properties => {},
+	properties => {
+	    poolid => {
+		type => 'string',
+		format => 'pve-poolid',
+		optional => 1,
+	    },
+	    type => {
+		type => 'string',
+		enum => [ 'qemu', 'lxc', 'storage' ],
+		optional => 1,
+		requires => 'poolid',
+	    },
+	},
     },
     returns => {
 	type => 'array',
@@ -35,6 +47,38 @@ __PACKAGE__->register_method ({
 	    type => "object",
 	    properties => {
 		poolid => { type => 'string' },
+		comment => {
+		    type => 'string',
+		    optional => 1,
+		},
+		members => {
+		    type => 'array',
+		    optional => 1,
+		    items => {
+			type => "object",
+			additionalProperties => 1,
+			properties => {
+			    type => {
+				type => 'string',
+				enum => [ 'qemu', 'lxc', 'openvz', 'storage' ],
+			    },
+			    id => {
+				type => 'string',
+			    },
+			    node => {
+				type => 'string',
+			    },
+			    vmid => {
+				type => 'integer',
+				optional => 1,
+			    },
+			    storage => {
+				type => 'string',
+				optional => 1,
+			    },
+			},
+		    },
+		},
 	    },
 	},
 	links => [ { rel => 'child', href => "{poolid}" } ],
@@ -47,15 +91,63 @@ __PACKAGE__->register_method ({
 
 	my $usercfg = $rpcenv->{user_cfg};
 
-
 	my $res = [];
-	for my $pool (sort keys %{$usercfg->{pools}}) {
-	    next if !$rpcenv->check($authuser, "/pool/$pool", [ 'Pool.Audit' ], 1);
+	if (my $poolid = $param->{poolid}) {
+	    $rpcenv->check($authuser, "/pool/$poolid", [ 'Pool.Audit' ], 1);
 
-	    my $entry = { poolid => $pool };
-	    my $pool_config = $usercfg->{pools}->{$pool};
-	    $entry->{comment} = $pool_config->{comment} if defined($pool_config->{comment});
-	    push @$res, $entry;
+	    my $vmlist = PVE::Cluster::get_vmlist() || {};
+	    my $idlist = $vmlist->{ids} || {};
+
+	    my $rrd = PVE::Cluster::rrd_dump();
+
+	    my $pool_config = $usercfg->{pools}->{$poolid};
+
+	    die "pool '$poolid' does not exist\n" if !$pool_config;
+
+	    my $members = [];
+	    for my $vmid (sort keys %{$pool_config->{vms}}) {
+		my $vmdata = $idlist->{$vmid};
+		next if !$vmdata || defined($param->{type}) && $param->{type} ne $vmdata->{type};
+		my $entry = PVE::API2Tools::extract_vm_stats($vmid, $vmdata, $rrd);
+		push @$members, $entry;
+	    }
+
+	    my $nodename = PVE::INotify::nodename();
+	    my $cfg = PVE::Storage::config();
+	    if (!defined($param->{type}) || $param->{type} eq 'storage') {
+		for my $storeid (sort keys %{$pool_config->{storage}}) {
+		    my $scfg = PVE::Storage::storage_config ($cfg, $storeid, 1);
+		    next if !$scfg;
+
+		    my $storage_node = $nodename; # prefer local node
+		    if ($scfg->{nodes} && !$scfg->{nodes}->{$storage_node}) {
+			for my $node (sort keys(%{$scfg->{nodes}})) {
+			    $storage_node = $node;
+			    last;
+			}
+		    }
+
+		    my $entry = PVE::API2Tools::extract_storage_stats($storeid, $scfg, $storage_node, $rrd);
+		    push @$members, $entry;
+		}
+	    }
+
+	    my $pool_info = {
+		members => $members,
+	    };
+	    $pool_info->{comment} = $pool_config->{comment} if defined($pool_config->{comment});
+	    $pool_info->{poolid} = $poolid;
+
+	    push @$res, $pool_info;
+	} else {
+	    for my $pool (sort keys %{$usercfg->{pools}}) {
+		next if !$rpcenv->check($authuser, "/pool/$pool", [ 'Pool.Audit' ], 1);
+
+		my $entry = { poolid => $pool };
+		my $pool_config = $usercfg->{pools}->{$pool};
+		$entry->{comment} = $pool_config->{comment} if defined($pool_config->{comment});
+		push @$res, $entry;
+	    }
 	}
 
 	return $res;
@@ -92,6 +184,11 @@ __PACKAGE__->register_method ({
 	    my $pool = $param->{poolid};
 
 	    die "pool '$pool' already exists\n" if $usercfg->{pools}->{$pool};
+	    if ($pool =~ m!^(.*)/[^/]+$!) {
+		my $parent = $1;
+		die "parent '$parent' of pool '$pool' does not exist\n"
+		    if !defined($usercfg->{pools}->{$parent});
+	    }
 
 	    $usercfg->{pools}->{$pool} = {
 		vms => {},
@@ -107,7 +204,7 @@ __PACKAGE__->register_method ({
     }});
 
 __PACKAGE__->register_method ({
-    name => 'update_pool',
+    name => 'update_pool_deprecated',
     protected => 1,
     path => '{poolid}',
     method => 'PUT',
@@ -115,9 +212,56 @@ __PACKAGE__->register_method ({
 	description => "You also need the right to modify permissions on any object you add/delete.",
 	check => ['perm', '/pool/{poolid}', ['Pool.Allocate']],
     },
-    description => "Update pool data.",
+    description => "Update pool data (deprecated, no support for nested pools - use 'PUT /pools/?poolid={poolid}' instead).",
     parameters => {
-   	additionalProperties => 0,
+	additionalProperties => 0,
+	properties => {
+	    poolid => { type => 'string', format => 'pve-poolid' },
+	    comment => { type => 'string', optional => 1 },
+	    vms => {
+		description => 'List of guest VMIDs to add or remove from this pool.',
+		type => 'string',  format => 'pve-vmid-list',
+		optional => 1,
+	    },
+	    storage => {
+		description => 'List of storage IDs to add or remove from this pool.',
+		type => 'string',  format => 'pve-storage-id-list',
+		optional => 1,
+	    },
+	    'allow-move' => {
+		description => 'Allow adding a guest even if already in another pool.'
+		    .' The guest will be removed from its current pool and added to this one.',
+		type => 'boolean',
+		optional => 1,
+		default => 0,
+	    },
+	    delete => {
+		description => 'Remove the passed VMIDs and/or storage IDs instead of adding them.',
+		type => 'boolean',
+		optional => 1,
+		default => 0,
+	    },
+	},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	return __PACKAGE__->update_pool($param);
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'update_pool',
+    protected => 1,
+    path => '',
+    method => 'PUT',
+    permissions => {
+	description => "You also need the right to modify permissions on any object you add/delete.",
+	check => ['perm', '/pool/{poolid}', ['Pool.Allocate']],
+    },
+    description => "Update pool.",
+    parameters => {
+	additionalProperties => 0,
 	properties => {
 	    poolid => { type => 'string', format => 'pve-poolid' },
 	    comment => { type => 'string', optional => 1 },
@@ -215,7 +359,7 @@ __PACKAGE__->register_method ({
     permissions => {
 	check => ['perm', '/pool/{poolid}', ['Pool.Audit']],
     },
-    description => "Get pool configuration.",
+    description => "Get pool configuration (deprecated, no support for nested pools, use 'GET /pools/?poolid={poolid}').",
     parameters => {
 	additionalProperties => 0,
 	properties => {
@@ -270,60 +414,38 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	my $usercfg = cfs_read_file("user.cfg");
-
-	my $vmlist = PVE::Cluster::get_vmlist() || {};
-	my $idlist = $vmlist->{ids} || {};
-
-	my $rrd = PVE::Cluster::rrd_dump();
-
-	my $pool = $param->{poolid};
-
-	my $pool_config = $usercfg->{pools}->{$pool};
-
-	die "pool '$pool' does not exist\n" if !$pool_config;
-
-	my $members = [];
-	for my $vmid (sort keys %{$pool_config->{vms}}) {
-	    my $vmdata = $idlist->{$vmid};
-	    next if !$vmdata || defined($param->{type}) && $param->{type} ne $vmdata->{type};
-	    my $entry = PVE::API2Tools::extract_vm_stats($vmid, $vmdata, $rrd);
-	    push @$members, $entry;
-	}
+	my $pool_info = __PACKAGE__->index($param);
+	return $pool_info->[0];
+    }});
 
-	my $nodename = PVE::INotify::nodename();
-	my $cfg = PVE::Storage::config();
-	if (!defined($param->{type}) || $param->{type} eq 'storage') {
-	    for my $storeid (sort keys %{$pool_config->{storage}}) {
-		my $scfg = PVE::Storage::storage_config ($cfg, $storeid, 1);
-		next if !$scfg;
-
-		my $storage_node = $nodename; # prefer local node
-		if ($scfg->{nodes} && !$scfg->{nodes}->{$storage_node}) {
-		    for my $node (sort keys(%{$scfg->{nodes}})) {
-			$storage_node = $node;
-			last;
-		    }
-		}
 
-		my $entry = PVE::API2Tools::extract_storage_stats($storeid, $scfg, $storage_node, $rrd);
-		push @$members, $entry;
-	    }
+__PACKAGE__->register_method ({
+    name => 'delete_pool_deprecated',
+    protected => 1,
+    path => '{poolid}',
+    method => 'DELETE',
+    permissions => {
+	description => "You can only delete empty pools (no members).",
+	check => ['perm', '/pool/{poolid}', ['Pool.Allocate']],
+    },
+    description => "Delete pool (deprecated, no support for nested pools, use 'DELETE /pools/?poolid={poolid}').",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    poolid => { type => 'string', format => 'pve-poolid' },
 	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
 
-	my $res = {
-	    members => $members,
-	};
-	$res->{comment} = $pool_config->{comment} if defined($pool_config->{comment});
-
-	return $res;
+	return __PACKAGE__->delete_pool($param);
     }});
 
-
 __PACKAGE__->register_method ({
     name => 'delete_pool',
     protected => 1,
-    path => '{poolid}',
+    path => '',
     method => 'DELETE',
     permissions => {
 	description => "You can only delete empty pools (no members).",
@@ -354,6 +476,9 @@ __PACKAGE__->register_method ({
 
 	    my $pool_config = $usercfg->{pools}->{$pool};
 	    die "pool '$pool' does not exist\n" if !$pool_config;
+	    for my $subpool (sort keys %{$pool_config->{pools}}) {
+		die "pool '$pool' is not empty (contains pool '$subpool')\n";
+	    }
 
 	    for my $vmid (sort keys %{$pool_config->{vms}}) {
 		next if !$idlist->{$vmid}; # ignore destroyed guests
-- 
2.39.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] [PATCH manager 2/2] ui: pools: switch to new API endpoints
  2023-11-20  7:22 [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools Fabian Grünbichler
                   ` (2 preceding siblings ...)
  2023-11-20  7:22 ` [pve-devel] [PATCH manager 1/2] fix #1148: api: pools: support nested pools Fabian Grünbichler
@ 2023-11-20  7:22 ` Fabian Grünbichler
  2023-11-20 11:27 ` [pve-devel] applied-series: [PATCH access-control/manager 0/4] fix #1148: nested pools Wolfgang Bumiller
  4 siblings, 0 replies; 6+ messages in thread
From: Fabian Grünbichler @ 2023-11-20  7:22 UTC (permalink / raw)
  To: pve-devel

which support nested pools. mostly straight-forward, only pool deletion and the
members grid need some special attention.

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

Notes:
    Thanks Dominik for pointing me at the 'root' part of the proxy :)

 www/manager6/dc/PoolView.js      |  3 +++
 www/manager6/grid/PoolMembers.js | 14 ++++++++------
 www/manager6/pool/StatusView.js  |  2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/www/manager6/dc/PoolView.js b/www/manager6/dc/PoolView.js
index db97cbe72..741b2025b 100644
--- a/www/manager6/dc/PoolView.js
+++ b/www/manager6/dc/PoolView.js
@@ -31,6 +31,9 @@ Ext.define('PVE.dc.PoolView', {
 	    callback: function() {
 		reload();
 	    },
+	    getUrl: function(rec) {
+		return '/pools/?poolid=' + rec.getId();
+	    },
 	});
 
 	var run_editor = function() {
diff --git a/www/manager6/grid/PoolMembers.js b/www/manager6/grid/PoolMembers.js
index 74950d80e..75f20cab1 100644
--- a/www/manager6/grid/PoolMembers.js
+++ b/www/manager6/grid/PoolMembers.js
@@ -17,8 +17,9 @@ Ext.define('PVE.pool.AddVM', {
 	    throw "no pool specified";
 	}
 
-	me.url = "/pools/" + me.pool;
+	me.url = '/pools/';
 	me.method = 'PUT';
+	me.extraRequestParams.poolid = me.pool;
 
 	var vmsField = Ext.create('Ext.form.field.Text', {
 	    name: 'vms',
@@ -120,8 +121,9 @@ Ext.define('PVE.pool.AddStorage', {
 
 	me.isCreate = true;
 	me.isAdd = true;
-	me.url = "/pools/" + me.pool;
+	me.url = "/pools/";
 	me.method = 'PUT';
+	me.extraRequestParams.poolid = me.pool;
 
 	Ext.apply(me, {
 	    subject: gettext('Storage'),
@@ -168,8 +170,8 @@ Ext.define('PVE.grid.PoolMembers', {
 	    ],
 	    proxy: {
 		type: 'proxmox',
-		root: 'data.members',
-		url: "/api2/json/pools/" + me.pool,
+		root: 'data[0].members',
+		url: "/api2/json/pools/?poolid=" + me.pool,
 	    },
 	});
 
@@ -192,7 +194,7 @@ Ext.define('PVE.grid.PoolMembers', {
 					 "'" + rec.data.id + "'");
 	    },
 	    handler: function(btn, event, rec) {
-		var params = { 'delete': 1 };
+		var params = { 'delete': 1, poolid: me.pool };
 		if (rec.data.type === 'storage') {
 		    params.storage = rec.data.storage;
 		} else if (rec.data.type === 'qemu' || rec.data.type === 'lxc' || rec.data.type === 'openvz') {
@@ -202,7 +204,7 @@ Ext.define('PVE.grid.PoolMembers', {
 		}
 
 		Proxmox.Utils.API2Request({
-		    url: '/pools/' + me.pool,
+		    url: '/pools/',
 		    method: 'PUT',
 		    params: params,
 		    waitMsgTarget: me,
diff --git a/www/manager6/pool/StatusView.js b/www/manager6/pool/StatusView.js
index 302ae5ab0..3d46b3b1a 100644
--- a/www/manager6/pool/StatusView.js
+++ b/www/manager6/pool/StatusView.js
@@ -24,7 +24,7 @@ Ext.define('PVE.pool.StatusView', {
 	};
 
 	Ext.apply(me, {
-	    url: "/api2/json/pools/" + pool,
+	    url: "/api2/json/pools/?poolid=" + pool,
 	    rows: rows,
 	});
 
-- 
2.39.2





^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] applied-series: [PATCH access-control/manager 0/4] fix #1148: nested pools
  2023-11-20  7:22 [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools Fabian Grünbichler
                   ` (3 preceding siblings ...)
  2023-11-20  7:22 ` [pve-devel] [PATCH manager 2/2] ui: pools: switch to new API endpoints Fabian Grünbichler
@ 2023-11-20 11:27 ` Wolfgang Bumiller
  4 siblings, 0 replies; 6+ messages in thread
From: Wolfgang Bumiller @ 2023-11-20 11:27 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: pve-devel

applied series, bumped access control and manager's dep on it

On Mon, Nov 20, 2023 at 08:22:38AM +0100, Fabian Grünbichler wrote:
> this series extends the poolname to allow (for now) two '/' inside the
> name to nest pools like this:
> 
> parent/child/grandchild
> 
> since '/' is a bad character for path parameters, some API shuffling is
> needed. other approaches that were tried but discarded, or rejected
> outright:
> - urlencoding
> - systemd-escape
> - manual encoding/escaping
> - just encoding the parent/children in user.cfg, but not in the pool name
> 
> switching over some of the UI pool lists to also nest them visually should
> be do-able as a follow-up.
> 
> changes since RFC:
> - some style improvements, RE adaptation
> 
> pve-access-control:
> 
> Fabian Grünbichler (2):
>   allow up to three levels of pool nesting
>   pools: record parent/subpool information
> 
>  src/PVE/AccessControl.pm  | 26 +++++++++++++++++++++++---
>  src/test/parser_writer.pl |  4 ++++
>  src/test/perm-test6.pl    | 16 ++++++++++++++++
>  src/test/test6.cfg        |  5 +++++
>  4 files changed, 48 insertions(+), 3 deletions(-)
> 
> pve-manager:
> 
> Fabian Grünbichler (2):
>   api: pools: support nested pools
>   ui: pools: switch to new API endpoints
> 
>  PVE/API2/Pool.pm                 | 243 +++++++++++++++++++++++--------
>  www/manager6/dc/PoolView.js      |   3 +
>  www/manager6/grid/PoolMembers.js |  14 +-
>  www/manager6/pool/StatusView.js  |   2 +-
>  4 files changed, 196 insertions(+), 66 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-11-20 11:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-20  7:22 [pve-devel] [PATCH access-control/manager 0/4] fix #1148: nested pools Fabian Grünbichler
2023-11-20  7:22 ` [pve-devel] [PATCH access-control 1/2] fix #1148: allow up to three levels of pool nesting Fabian Grünbichler
2023-11-20  7:22 ` [pve-devel] [PATCH access-control 2/2] pools: record parent/subpool information Fabian Grünbichler
2023-11-20  7:22 ` [pve-devel] [PATCH manager 1/2] fix #1148: api: pools: support nested pools Fabian Grünbichler
2023-11-20  7:22 ` [pve-devel] [PATCH manager 2/2] ui: pools: switch to new API endpoints Fabian Grünbichler
2023-11-20 11:27 ` [pve-devel] applied-series: [PATCH access-control/manager 0/4] fix #1148: nested pools Wolfgang Bumiller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal