public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Leo Nunner <l.nunner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server] cloudinit: fix 'pending' api endpoint
Date: Thu, 11 May 2023 10:03:17 +0200	[thread overview]
Message-ID: <20230511080317.49367-1-l.nunner@proxmox.com> (raw)

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





             reply	other threads:[~2023-05-11  8:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-11  8:03 Leo Nunner [this message]
2023-06-07 16:36 ` [pve-devel] applied: " Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230511080317.49367-1-l.nunner@proxmox.com \
    --to=l.nunner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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