[pve-devel] [PATCH qemu-server] cloudinit: fix 'pending' api endpoint

[pve-devel] [PATCH qemu-server] cloudinit: fix 'pending' api endpoint

[Leo Nunner]

This patch partially reverts commit 1b5706cd168fedc5e494e24300069ee4ff25761f,
by reintroducing the old format for return values (key, value, pending,
delete), but drops the "force-delete" return value. Right now, this
endpoint does not conform to its own format, because the return values
are as follows:

{
    key => {
	old => 'foo',
	new => 'bar',
    },
    […]
}

While the format specified is

[
    {
	key => 'baz',
	old => 'foo',
	new => 'bar',
    },
    […]
]

This leads to the endpoint being broken when used through 'qm' and
'pvesh'. Using the API works fine, because the format doesn't get
verified there. Reverting this change brings the advantage that we can
also use PVE::GuestHelpers::format_pending when calling the endpoint
through qm again.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
---
I'm not sure whether or not this constitutes a breaking change. We are
returning to the old format for this endpoint, and up until now it was
broken anyway (well, for the CLI that is).

 PVE/API2/Qemu.pm | 48 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 587bb22..dd52fdc 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1344,16 +1344,23 @@ __PACKAGE__->register_method({
 		    description => "Configuration option name.",
 		    type => 'string',
 		},
-		old => {
+		value => {
 		    description => "Value as it was used to generate the current cloudinit image.",
 		    type => 'string',
 		    optional => 1,
 		},
-		new => {
+		pending => {
 		    description => "The new pending value.",
 		    type => 'string',
 		    optional => 1,
 		},
+		delete => {
+		    description => "Indicates a pending delete request if present and not 0. ",
+		    type => 'integer',
+		    minimum => 0,
+		    maximum => 1,
+		    optional => 1,
+		},
 	    },
 	},
     },
@@ -1365,26 +1372,39 @@ __PACKAGE__->register_method({
 
 	my $ci = $conf->{cloudinit};
 
-	my $res = {};
+	$conf->{cipassword} = '**********' if exists $conf->{cipassword};
+	$ci->{cipassword} = '**********' if exists $ci->{cipassword};
+
+	my $res = [];
+
+	# All the values that got added
 	my $added = delete($ci->{added}) // '';
 	for my $key (PVE::Tools::split_list($added)) {
-	    $res->{$key} = { new => $conf->{$key} };
+	    push @$res, { key => $key, pending => $conf->{$key} };
 	}
 
-	for my $key (keys %$ci) {
-	    if (!exists($conf->{$key})) {
-		$res->{$key} = { old => $ci->{$key} };
+	# All already existing values (+ their new value, if it exists)
+	for my $opt (keys %$cloudinitoptions) {
+	    next if !$conf->{$opt};
+	    next if $added =~ m/$opt/;
+	    my $item = {
+		key => $opt,
+	    };
+
+	    if (my $pending = $ci->{$opt}) {
+		$item->{value} = $pending;
+		$item->{pending} = $conf->{$opt};
 	    } else {
-		$res->{$key} = {
-		    old => $ci->{$key},
-		    new => $conf->{$key},
-		};
+		$item->{value} = $conf->{$opt},
 	    }
+
+	    push @$res, $item;
 	}
 
-	if (defined(my $pw = $res->{cipassword})) {
-	    $pw->{old} = '**********' if exists $pw->{old};
-	    $pw->{new} = '**********' if exists $pw->{new};
+	# Now, we'll find the deleted ones
+	for my $opt (keys %$ci) {
+	    next if $conf->{$opt};
+	    push @$res, { key => $opt, delete => 1 };
 	}
 
 	return $res;
-- 
2.30.2

[pve-devel] applied: [PATCH qemu-server] cloudinit: fix 'pending' api endpoint

[Thomas Lamprecht]

Am 11/05/2023 um 10:03 schrieb Leo Nunner:
> This patch partially reverts commit 1b5706cd168fedc5e494e24300069ee4ff25761f,
> by reintroducing the old format for return values (key, value, pending,
> delete), but drops the "force-delete" return value. Right now, this
> endpoint does not conform to its own format, because the return values
> are as follows:
> 
> {
>     key => {
> 	old => 'foo',
> 	new => 'bar',
>     },
>     […]
> }
> 
> While the format specified is
> 
> [
>     {
> 	key => 'baz',
> 	old => 'foo',
> 	new => 'bar',
>     },
>     […]
> ]
> 
> This leads to the endpoint being broken when used through 'qm' and
> 'pvesh'. Using the API works fine, because the format doesn't get
> verified there. Reverting this change brings the advantage that we can
> also use PVE::GuestHelpers::format_pending when calling the endpoint
> through qm again.
> 
> Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
> ---
> I'm not sure whether or not this constitutes a breaking change. We are
> returning to the old format for this endpoint, and up until now it was
> broken anyway (well, for the CLI that is).
> 
>  PVE/API2/Qemu.pm | 48 ++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
>

applied, thanks! BUT, mainly @Wolfgang, I thought we figured that out ok back then,
can you re-check this please, I'd really like to stop this from coming back haunting
us 👻