From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <s.ivanov@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 1ACE16373E
 for <pmg-devel@lists.proxmox.com>; Wed, 25 Nov 2020 10:12:20 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 0B26516F77
 for <pmg-devel@lists.proxmox.com>; Wed, 25 Nov 2020 10:11:50 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [212.186.127.180])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 8171116F6A
 for <pmg-devel@lists.proxmox.com>; Wed, 25 Nov 2020 10:11:49 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4B0D444055
 for <pmg-devel@lists.proxmox.com>; Wed, 25 Nov 2020 10:11:49 +0100 (CET)
Date: Wed, 25 Nov 2020 10:11:48 +0100
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Message-ID: <20201125101148.40c3c1ca@rosa.proxmox.com>
In-Reply-To: <20201125085711.12574-1-d.csapak@proxmox.com>
References: <20201125085711.12574-1-d.csapak@proxmox.com>
X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.086 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_DNSWL_MED        -2.3 Sender listed at https://www.dnswl.org/,
 medium trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [nodes.pm]
Subject: Re: [pmg-devel] [PATCH pmg-api] API2/Nodes/termproxy: fix upgrade
 for new xtermjs package
X-BeenThere: pmg-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Mail Gateway development discussion
 <pmg-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pmg-devel>, 
 <mailto:pmg-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pmg-devel/>
List-Post: <mailto:pmg-devel@lists.proxmox.com>
List-Help: <mailto:pmg-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel>, 
 <mailto:pmg-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 25 Nov 2020 09:12:20 -0000

Thanks for addressing this prompty!

quickly compared the changes to the ones from pve-manager,
verified that it's currently broken, and that applying the patch fixes it.

LGTM:
Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>


On Wed, 25 Nov 2020 09:57:11 +0100
Dominik Csapak <d.csapak@proxmox.com> wrote:

> we changed how we call upgrade (cmd param instead of upgrade)
> so we have to adapt our api call
> 
> most is taken from pve
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PMG/API2/Nodes.pm | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PMG/API2/Nodes.pm b/src/PMG/API2/Nodes.pm
> index 259f8f3..e368569 100644
> --- a/src/PMG/API2/Nodes.pm
> +++ b/src/PMG/API2/Nodes.pm
> @@ -348,10 +348,24 @@ __PACKAGE__->register_method ({
>  	    node => get_standard_option('pve-node'),
>  	    upgrade => {
>  		type => 'boolean',
> -		description => "Run 'apt-get dist-upgrade' instead of normal shell.",
> +		description => "Deprecated, use the 'cmd' property instead! Run 'apt-get dist-upgrade' instead of normal shell.",
>  		optional => 1,
>  		default => 0,
>  	    },
> +	    cmd => {
> +		type => 'string',
> +		description => "Run specific command or default to login.",
> +		enum => ['login', 'upgrade'],
> +		optional => 1,
> +		default => 'login',
> +	    },
> +	    'cmd-opts' => {
> +		type => 'string',
> +		description => "Add parameters to a command. Encoded as null terminated strings.",
> +		requires => 'cmd',
> +		optional => 1,
> +		default => '',
> +	    },
>  	},
>      },
>      returns => {
> @@ -384,17 +398,21 @@ __PACKAGE__->register_method ({
>  	my $family = PVE::Tools::get_host_address_family($node);
>  	my $port = PVE::Tools::next_vnc_port($family);
>  
> +	# FIXME: remove with 7.0
> +	if ($param->{upgrade}) {
> +	    $param->{cmd} = 'upgrade';
> +	}
> +
>  	my $shcmd;
>  
>  	if ($user eq 'root@pam') {
> -	    if ($param->{upgrade}) {
> -		my $upgradecmd = "pmgupgrade --shell";
> -		# $upgradecmd = PVE::Tools::shellquote($upgradecmd) if $remip;
> -		$shcmd = [ '/bin/bash', '-c', $upgradecmd ];
> +	    if (defined($param->{cmd}) && $param->{cmd} eq 'upgrade') {
> +		$shcmd = [ 'pmgupgrade', '--shell' ];
>  	    } else {
>  		$shcmd = [ '/bin/login', '-f', 'root' ];
>  	    }
>  	} else {
> +	    # non-root must always login for now, we do not have a superuser role!
>  	    $shcmd = [ '/bin/login' ];
>  	}
>