From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <d.csapak@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 UTF8SMTPS id E832A780F8
for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 14:13:57 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with UTF8SMTP id D3E881BE60
for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 14:13:27 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 9D9401BE52
for <pve-devel@lists.proxmox.com>; Thu, 29 Apr 2021 14:13:26 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 72A13464CB;
Thu, 29 Apr 2021 14:13:26 +0200 (CEST)
Message-ID: <9e0cf6d2-c583-fd21-4507-321632840644@proxmox.com>
Date: Thu, 29 Apr 2021 14:13:24 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101
Thunderbird/88.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Lorenz Stechauner <l.stechauner@proxmox.com>
References: <20210428141346.240896-1-l.stechauner@proxmox.com>
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <20210428141346.240896-1-l.stechauner@proxmox.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.050 Adjusted score from AWL reputation of From: address
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -0.001 Looks like a legit reply (A)
PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far
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. [me.storage, sencha.com]
Subject: Re: [pve-devel] [PATCH manager 1/1] fix #1710: add retrieve from
url button for storage
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 29 Apr 2021 12:13:58 -0000
one high level comment here:
we try to avoid a long 'initComponent' in favor
of declaring classes schematically with extjs
mvvm (model, view, viewmodel), similar to mvc
(model view controller)
for the edit window this would mean it looking like this:
Ext.define(....
...
controller: {
xclass: 'Ext.app.ViewController',
fooChange: ...
},
items: [
{
xtype: 'foo',
property1: 'bar',
listener: {
change: 'fooChange',
},
},
...
],
});
this makes it easier to separate layout/hierarchy and the
actual behaviours (i.e. some events or actions)
more comments inline
On 4/28/21 16:13, Lorenz Stechauner wrote:
> Add PVE.storage.Retrieve window and PVE.form.hashAlgorithmSelector.
> Users are now able to download/retrieve any .iso/... file onto their
> storages and verify file integrity with checksums.
>
> Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
> ---
> www/manager6/Makefile | 1 +
> www/manager6/form/HashAlgorithmSelector.js | 21 +++
> www/manager6/storage/ContentView.js | 161 +++++++++++++++++++++
> 3 files changed, 183 insertions(+)
> create mode 100644 www/manager6/form/HashAlgorithmSelector.js
>
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index afed3283..8e6557d8 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -38,6 +38,7 @@ JSSRC= \
> form/GlobalSearchField.js \
> form/GroupSelector.js \
> form/GuestIDSelector.js \
> + form/HashAlgorithmSelector.js \
> form/HotplugFeatureSelector.js \
> form/IPProtocolSelector.js \
> form/IPRefSelector.js \
> diff --git a/www/manager6/form/HashAlgorithmSelector.js b/www/manager6/form/HashAlgorithmSelector.js
> new file mode 100644
> index 00000000..4a72cc08
> --- /dev/null
> +++ b/www/manager6/form/HashAlgorithmSelector.js
> @@ -0,0 +1,21 @@
> +Ext.define('PVE.form.hashAlgorithmSelector', {
> + extend: 'Proxmox.form.KVComboBox',
> + alias: ['widget.pveHashAlgorithmSelector'],
> + comboItems: [],
> + hasNoneOption: false,
> + initComponent: function() {
> + var me = this;
> + me.comboItems = [
> + ['md5', 'MD5'],
> + ['sha1', 'SHA-1'],
> + ['sha224', 'SHA-224'],
> + ['sha256', 'SHA-256'],
> + ['sha384', 'SHA-384'],
> + ['sha512', 'SHA-512'],
> + ];
> + if (me.hasNoneOption) {
> + me.comboItems.unshift(['none', 'None']);
> + }
why do you make this optional, the only user always adds it
afaics? or are there more potential users in the future?
in that case, the user may want to configure the
whole list (e.g. no md5, only none/sha256, etc.)
> + this.callParent();
> + },
> +});
> diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
> index dd6df4b1..7187ebbe 100644
> --- a/www/manager6/storage/ContentView.js
> +++ b/www/manager6/storage/ContentView.js
> @@ -191,6 +191,153 @@ Ext.define('PVE.storage.Upload', {
> },
> });
>
> +Ext.define('PVE.storage.Retrieve', {
> + extend: 'Proxmox.window.Edit',
> + alias: 'widget.pveStorageRetrieve',
> +
> + resizable: false,
this can be omitted here, it's already set
to false in the edit window
> +
> + modal: true,
same here
> +
> + isCreate: true,
> +
> + showTaskViewer: true,
> + upidFieldName: 'upid',
this is only necessary because the api call does two
things yeah?
if we change to a 'get_url_info' + 'retrive' model,
this is not necessary at all
> +
> + initComponent: function() {
> + var me = this;
> +
> + if (!me.nodename) {
> + throw "no node name specified";
> + }
> + if (!me.storage) {
> + throw "no storage ID specified";
> + }
> +
> + me.url = `/nodes/${me.nodename}/storage/${me.storage}/retrieve`;
> + me.method = 'POST';
> +
> + let defaultContent = me.contents[0] || '';
> +
> + let urlField = Ext.create('Ext.form.field.Text', {
> + name: 'url',
> + allowBlank: false,
> + fieldLabel: gettext('URL'),
> + });
> +
> + let fileNameField = Ext.create('Ext.form.field.Text', {
> + name: 'filename',
> + allowBlank: false,
> + fieldLabel: gettext('File name'),
> + });
> +
> + let fileSizeField = Ext.create('Ext.form.field.Text', {
> + name: 'size',
> + disabled: true,
> + fieldLabel: gettext('File size'),
> + emptyText: gettext('unknown'),
> + });
> +
> + let checksumField = Ext.create('Ext.form.field.Text', {
> + name: 'checksum',
> + fieldLabel: gettext('Checksum'),
> + allowBlank: true,
> + disabled: true,
> + emptyText: gettext('none'),
> + });
> +
> + let checksumAlgField = Ext.create('PVE.form.hashAlgorithmSelector', {
> + name: 'checksumalg',
> + fieldLabel: gettext('Hash algorithm'),
> + allowBlank: true,
> + hasNoneOption: true,
> + value: 'none',
> + });
> +
> + let inputPanel = Ext.create('Proxmox.panel.InputPanel', {
> + method: 'POST',
> + waitMsgTarget: true,
> + border: false,
> + columnT: [
> + urlField,
> + fileNameField,
> + ],
> + column1: [
> + {
> + xtype: 'pveContentTypeSelector',
> + cts: me.contents,
> + fieldLabel: gettext('Content'),
> + name: 'content',
> + value: defaultContent,
> + allowBlank: false,
> + },
> + ],
> + column2: [
> + fileSizeField,
> + ],
> + advancedColumn1: [
> + checksumField,
> + checksumAlgField,
> + ],
> + advancedColumn2: [
> + {
> + xtype: 'checkbox',
> + name: 'insecure',
> + fieldLabel: gettext('Trust invalid certificates'),
> + labelWidth: 150,
> + },
this btw. does not work for the filename+size, so i cannot
download a file from a url with an untrusted cert (via gui)
since the field is never valid
> + ],
> + });
> +
> + urlField.on('change', function() {
> + urlField.setValidation("Waiting for response...");
> + urlField.validate();
> + fileSizeField.setValue("");
> + Proxmox.Utils.API2Request({
> + url: me.url,
> + method: 'POST',
> + params: {
> + metaonly: 1,
> + url: me.getValues()['url'],
> + content: me.getValues()['content'],
> + },
> + failure: function(res, opt) {
> + urlField.setValidation(res.result.message);
> + urlField.validate();
> + fileSizeField.setValue("");
> + },
> + success: function(res, opt) {
> + urlField.setValidation();
> + urlField.validate();
> +
> + let data = res.result.data;
> + fileNameField.setValue(data.filename);
> + fileSizeField.setValue(Proxmox.Utils.format_size(data.size));
> + },
> + });
> + });
this needs some buffering
as it is now, it makes a request on every key stroke
i.e. when i type in:
http://www.google.com
it makes 21(!) requests
e.g. textfield has a 'checkChangeBuffer' property
https://docs.sencha.com/extjs/6.0.1/classic/Ext.form.field.Text.html#cfg-checkChangeBuffer
or the buffer property of the listener:
https://docs.sencha.com/extjs/6.0.1/classic/Ext.util.Observable.html#method-addListener
also it would probably be good if the window
gets a load mask when the request gets done
so the user has a feedback if its taking long
> +
> + checksumAlgField.on('change', function() {
> + if (this.getValue() === 'none') {
> + checksumField.setDisabled(true);
> + checksumField.setValue("");
> + checksumField.allowBlank = true;
> + } else {
> + checksumField.setDisabled(false);
> + checksumField.allowBlank = false;
> + }
> + });
> +
> + Ext.apply(me, {
> + title: gettext('Retrieve from URL'),
> + items: inputPanel,
> + submitText: gettext('Download'),
> + });
> +
> + me.callParent();
> + },
> +});
> +
> Ext.define('PVE.storage.ContentView', {
> extend: 'Ext.grid.GridPanel',
>
> @@ -262,6 +409,19 @@ Ext.define('PVE.storage.ContentView', {
> },
> });
>
> + let retrieveButton = Ext.create('Proxmox.button.Button', {
> + text: gettext('Retrieve from URL'),
> + handler: function() {
> + let win = Ext.create('PVE.storage.Retrieve', {
> + nodename: nodename,
> + storage: storage,
> + contents: [content],
> + });
> + win.show();
> + win.on('destroy', reload);
this can be simplified to
Ext.create('PVE.storage.Retrieve', {
nodename,
storage,
contents: [content],
autoShow: true,
listeners: {
destroy: () => reload(),
},
});
caution, a listener (.on(), changes the scope of the function)
> + },
> + });
> +
> let removeButton = Ext.create('Proxmox.button.StdRemoveButton', {
> selModel: sm,
> delay: 5,
> @@ -276,6 +436,7 @@ Ext.define('PVE.storage.ContentView', {
> }
> if (me.useUploadButton) {
> me.tbar.push(uploadButton);
> + me.tbar.push(retrieveButton);
> }
> if (!me.useCustomRemoveButton) {
> me.tbar.push(removeButton);
>