From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 2B760793FA for ; Tue, 4 May 2021 11:47:30 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1FF0B26B5D for ; Tue, 4 May 2021 11:47:30 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 88BCD26B4F for ; Tue, 4 May 2021 11:47:28 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 57FB7429D3 for ; Tue, 4 May 2021 11:47:28 +0200 (CEST) Message-ID: Date: Tue, 4 May 2021 11:47:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101 Thunderbird/89.0 Content-Language: en-US To: Proxmox VE development discussion , Lorenz Stechauner References: <20210504085530.36961-1-l.stechauner@proxmox.com> <20210504085703.37472-1-l.stechauner@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210504085703.37472-1-l.stechauner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.195 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) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes 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. [nodes.pm, me.storage, caps.storage, proxmox.com] Subject: Re: [pve-devel] [PATCH v3 manager 2/2] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 May 2021 09:47:30 -0000 On 04.05.21 10:57, 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. > > Add new method: GET /nodes/{node}/urlmeta - returns url metadata I'd separate backend and gui into at least two patches, e.g., the hash algorithm widget has nothing directly to do with querying metadata from an URL. Also, you do not handle the datacenter.cfg http_proxy setting, neither here nor in the storage patch. > > Signed-off-by: Lorenz Stechauner > --- > PVE/API2/Nodes.pm | 97 +++++++ > www/manager6/Makefile | 1 + > www/manager6/Utils.js | 1 + > www/manager6/form/HashAlgorithmSelector.js | 16 ++ > www/manager6/storage/Browser.js | 8 + > www/manager6/storage/ContentView.js | 281 +++++++++++++++++++-- > 6 files changed, 379 insertions(+), 25 deletions(-) > create mode 100644 www/manager6/form/HashAlgorithmSelector.js > > diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm > index ba6621c6..c2407d99 100644 > --- a/PVE/API2/Nodes.pm > +++ b/PVE/API2/Nodes.pm > @@ -11,6 +11,7 @@ use JSON; > use POSIX qw(LONG_MAX); > use Time::Local qw(timegm_nocheck); > use Socket; > +use IO::Socket::SSL; > > use PVE::API2Tools; > use PVE::APLInfo; > @@ -254,6 +255,7 @@ __PACKAGE__->register_method ({ > { name => 'tasks' }, > { name => 'termproxy' }, > { name => 'time' }, > + { name => 'urlmeta' }, > { name => 'version' }, > { name => 'vncshell' }, > { name => 'vzdump' }, > @@ -1596,6 +1598,101 @@ __PACKAGE__->register_method({ > return $rpcenv->fork_worker('download', undef, $user, $worker); > }}); > > + > +__PACKAGE__->register_method({ > + name => 'urlmeta', > + path => 'urlmeta', If one would just read this when doing a pvesh ls /nodes/foo they'd be probably have no idea what this would possibly do... Rather use something like "retrieve-url-metadata" or "query-url-metadata". > + method => 'GET', > + description => "Download templates and ISO images by using an URL.", But, this does not actually downloads any template, just queries some information about an url? > + proxyto => 'node', > + permissions => { > + check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]], > + }, > + protected => 1, why is this protected? This can run as unprivileged www user just fine, or? > + parameters => { > + additionalProperties => 0, > + properties => { > + node => get_standard_option('pve-node'), > + url => { > + description => "The URL to retrieve the file from.", > + type => 'string', > + }, > + insecure => { > + description => "Allow TLS certificates to be invalid.", > + type => 'boolean', > + optional => 1, > + } > + }, > + }, > + returns => { > + type => "object", > + properties => { > + filename => { > + type => 'string', > + optional => 1, > + }, > + size => { > + type => 'integer', > + renderer => 'bytes', > + optional => 1, > + }, > + mimetype => { > + type => 'string', > + optional => 1, > + }, > + }, > + }, > + code => sub { > + my ($param) = @_; > + > + my $url = $param->{url}; > + > + die "invalid https or http url" > + if $url !~ qr!^https?://!; this could get handled directly in the schema as regex > + > + my $ua = LWP::UserAgent->new(); > + $ua->ssl_opts( > + verify_hostname => 0, > + SSL_verify_mode => IO::Socket::SSL::SSL_VERIFY_NONE, > + ) if $param->{insecure}; that post-if is easily overlooked and goes against our style guide https://pve.proxmox.com/wiki/Perl_Style_Guide#Wrapping_Post-If Use a "normal" if () { here > + > + my $req = HTTP::Request->new(HEAD => $url); > + my $res = $ua->request($req); > + > + die "invalid server response: '" . $res->status_line() . "'" > + if ($res->code() != 200); > + > + my $size = $res->header("Content-Length"); > + my $dispo = $res->header("Content-Disposition"); dispo? please avoid truncating names if not really a common abbreviation where the code would actually benefit from the shorter version. > + my $type = $res->header("Content-Type"); > + > + my $filename; > + > + if ($dispo && $dispo =~ m/filename=(.+)/) { > + $filename = $1; > + } elsif ($url =~ m!^[^?]+/([^?/]*)(?:\?.*)?$!) { > + $filename = $1; > + } > + > + # Content-Type: text/html; charset=utf-8 > + if ($type && $type =~ m/^([^;]+);/) { > + $type = $1; > + } > + > + my $ret = {}; > + > + $ret->{filename} = $filename > + if $filename; if would fit in same line as expression > + > + $ret->{size} = $size + 0 > + if $size; if would fit in same line as expression, also you may want to check for definendess here. > + > + $ret->{mimetype} = $type > + if $type; if would fit in same line as expression > + > + return $ret; > + }}); > + > __PACKAGE__->register_method({ > name => 'report', > path => 'report', > 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/Utils.js b/www/manager6/Utils.js > index 581d2040..b7479e7a 100644 > --- a/www/manager6/Utils.js > +++ b/www/manager6/Utils.js > @@ -1826,6 +1826,7 @@ Ext.define('PVE.Utils', { > hastop: ['HA', gettext('Stop')], > imgcopy: ['', gettext('Copy data')], > imgdel: ['', gettext('Erase data')], > + imgdownload: [gettext('Storage'), gettext('Download data')], > lvmcreate: [gettext('LVM Storage'), gettext('Create')], > lvmthincreate: [gettext('LVM-Thin Storage'), gettext('Create')], > migrateall: ['', gettext('Migrate all VMs and Containers')], > diff --git a/www/manager6/form/HashAlgorithmSelector.js b/www/manager6/form/HashAlgorithmSelector.js > new file mode 100644 > index 00000000..5ae7a08b > --- /dev/null > +++ b/www/manager6/form/HashAlgorithmSelector.js > @@ -0,0 +1,16 @@ > +Ext.define('PVE.form.hashAlgorithmSelector', { > + extend: 'Proxmox.form.KVComboBox', > + alias: ['widget.pveHashAlgorithmSelector'], > + config: { > + deleteEmpty: false, > + }, > + comboItems: [ > + ['__default__', 'None'], > + ['md5', 'MD5'], > + ['sha1', 'SHA-1'], > + ['sha224', 'SHA-224'], > + ['sha256', 'SHA-256'], > + ['sha384', 'SHA-384'], > + ['sha512', 'SHA-512'], > + ], > +}); > diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js > index 5fee94c7..da3e66c8 100644 > --- a/www/manager6/storage/Browser.js > +++ b/www/manager6/storage/Browser.js > @@ -53,6 +53,9 @@ Ext.define('PVE.storage.Browser', { > let plugin = res.plugintype; > let contents = res.content.split(','); > > + let enableUpload = !!caps.storage['Datastore.AllocateTemplate']; > + let enableRetrieve = !!(caps.nodes['Sys.Audit'] && caps.nodes['Sys.Modify']); > + > if (contents.includes('backup')) { > me.items.push({ > xtype: 'pveStorageBackupView', > @@ -91,6 +94,8 @@ Ext.define('PVE.storage.Browser', { > itemId: 'contentIso', > content: 'iso', > pluginType: plugin, > + enableUploadButton: enableUpload, > + enableRetrieveButton: enableUpload && enableRetrieve, > useUploadButton: true, > }); > } > @@ -101,6 +106,9 @@ Ext.define('PVE.storage.Browser', { > iconCls: 'fa fa-file-o lxc', > itemId: 'contentVztmpl', > pluginType: plugin, > + enableUploadButton: enableUpload, > + enableRetrieveButton: enableUpload && enableRetrieve, > + useUploadButton: true, > }); > } > if (contents.includes('snippets')) { > diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js > index dd6df4b1..d01526e1 100644 > --- a/www/manager6/storage/ContentView.js > +++ b/www/manager6/storage/ContentView.js > @@ -191,6 +191,213 @@ Ext.define('PVE.storage.Upload', { > }, > }); > > +Ext.define('PVE.storage.Retrieve', { > + extend: 'Proxmox.window.Edit', > + alias: 'widget.pveStorageRetrieve', > + > + isCreate: true, > + > + showTaskViewer: true, > + > + title: gettext('Retrieve from URL'), > + submitText: gettext('Download'), > + > + id: 'retrieve', > + > + controller: { > + xclass: 'Ext.app.ViewController', > + > + urlChange: function(field) { > + let me = Ext.getCmp('retrieve'); > + field = me.down('[name=url]'); > + field.setValidation("Waiting for response..."); > + field.validate(); > + me.setValues({size: ""}); > + Proxmox.Utils.API2Request({ > + url: `/nodes/${me.nodename}/urlmeta`, > + method: 'GET', > + params: { > + url: field.getValue(), > + insecure: me.getValues()['insecure'], > + }, > + failure: function(res, opt) { > + field.setValidation(res.result.message); > + field.validate(); > + me.setValues({ > + size: "", > + mimetype: "", > + }); > + }, > + success: function(res, opt) { > + field.setValidation(); > + field.validate(); > + > + let data = res.result.data; > + me.setValues({ > + filename: data.filename || "", > + size: data.size && Proxmox.Utils.format_size(data.size) || "", > + mimetype: data.mimetype || "", > + }); > + }, > + }); > + }, > + > + hashChange: function(field) { > + let cecksum = Ext.getCmp('retrieveChecksum'); > + if (field.getValue() === '__default__') { > + cecksum.setDisabled(true); > + cecksum.setValue(""); > + cecksum.allowBlank = true; > + } else { > + cecksum.setDisabled(false); > + cecksum.allowBlank = false; > + } > + }, > + > + typeChange: function(field) { > + let me = Ext.getCmp('retrieve'); > + let content = me.getValues()['content']; > + let type = field.getValue(); > + > + const types = { > + iso: [ > + 'application/octet-stream', > + 'application/x-iso9660-image', > + 'application/x-ima', > + ], > + vztmpl: [ > + 'application/octet-stream', > + 'application/gzip', > + 'application/tar', > + 'application/tar+gzip', > + 'application/x-gzip', > + 'application/x-gtar', > + 'application/x-tgz', > + 'application/x-tar', > + ], > + }; > + > + if (type === "" || (types[content] && types[content].includes(type))) { > + field.setValidation(); > + field.setDisabled(true); > + } else { > + field.setDisabled(false); > + field.setValidation("Invalid type"); > + } > + field.validate(); > + }, > + }, > + > + items: [ > + { > + xtype: 'inputpanel', > + waitMsgTarget: true, > + border: false, > + columnT: [ > + { > + xtype: 'textfield', > + name: 'url', > + allowBlank: false, > + fieldLabel: gettext('URL'), > + listeners: { > + change: { > + buffer: 500, > + fn: 'urlChange', > + }, > + }, > + }, > + { > + xtype: 'textfield', > + name: 'filename', > + allowBlank: false, > + fieldLabel: gettext('File name'), > + }, > + ], > + column1: [ > + { > + xtype: 'pveContentTypeSelector', > + fieldLabel: gettext('Content'), > + name: 'content', > + allowBlank: false, > + }, > + ], > + column2: [ > + { > + xtype: 'textfield', > + name: 'size', > + disabled: true, > + fieldLabel: gettext('File size'), > + emptyText: gettext('unknown'), > + }, > + ], > + advancedColumn1: [ > + { > + xtype: 'textfield', > + name: 'checksum', > + fieldLabel: gettext('Checksum'), > + allowBlank: true, > + disabled: true, > + emptyText: gettext('none'), > + id: 'retrieveChecksum', > + }, > + { > + xtype: 'pveHashAlgorithmSelector', > + name: 'checksumalg', > + fieldLabel: gettext('Hash algorithm'), > + allowBlank: true, > + hasNoneOption: true, > + value: '__default__', > + listeners: { > + change: 'hashChange', > + }, > + }, > + ], > + advancedColumn2: [ > + { > + xtype: 'textfield', > + fieldLabel: gettext('MIME type'), > + name: 'mimetype', > + disabled: true, > + editable: false, > + emptyText: gettext('unknown'), > + listeners: { > + change: 'typeChange', > + }, > + }, > + { > + xtype: 'proxmoxcheckbox', > + name: 'insecure', > + fieldLabel: gettext('Trust invalid certificates'), > + uncheckedValue: 0, > + listeners: { > + change: 'urlChange', > + }, > + }, > + ], > + }, > + ], > + > + 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 contentTypeSel = me.items[0].column1[0]; > + contentTypeSel.cts = me.contents; > + contentTypeSel.value = me.contents[0] || ''; > + > + me.callParent(); > + }, > +}); > + > Ext.define('PVE.storage.ContentView', { > extend: 'Ext.grid.GridPanel', > > @@ -249,36 +456,60 @@ Ext.define('PVE.storage.ContentView', { > > Proxmox.Utils.monStoreErrors(me, store); > > - let uploadButton = Ext.create('Proxmox.button.Button', { > - text: gettext('Upload'), > - handler: function() { > - let win = Ext.create('PVE.storage.Upload', { > - nodename: nodename, > - storage: storage, > - contents: [content], > - }); > - win.show(); > - win.on('destroy', reload); > - }, > - }); > - > - let removeButton = Ext.create('Proxmox.button.StdRemoveButton', { > - selModel: sm, > - delay: 5, > - callback: function() { > - reload(); > - }, > - baseurl: baseurl + '/', > - }); > - > if (!me.tbar) { > me.tbar = []; > } > if (me.useUploadButton) { > - me.tbar.push(uploadButton); > + me.tbar.push( > + { > + xtype: 'button', > + text: gettext('Upload'), > + disabled: !me.enableUploadButton, > + handler: function() { > + Ext.create('PVE.storage.Upload', { > + nodename: nodename, > + storage: storage, > + contents: [content], > + autoShow: true, > + listeners:{ > + destroy: () => reload(), > + } > + }); > + }, > + }, > + { > + xtype: 'button', > + text: gettext('Retrieve from URL'), > + disabled: !me.enableRetrieveButton, > + handler: function() { > + Ext.create('PVE.storage.Retrieve', { > + nodename: nodename, > + storage: storage, > + contents: [content], > + autoShow: true, > + listeners: { > + destroy: () => reload(), > + }, > + }); > + }, > + }, > + '-', > + ); > } > - if (!me.useCustomRemoveButton) { > - me.tbar.push(removeButton); > + if (me.useCustomRemoveButton) { > + // custom remove button was inserted as first element > + // -> place it at the end of tbar > + me.tbar.push(me.tbar.shift()); > + } else { > + me.tbar.push({ > + xtype: 'proxmoxStdRemoveButton', > + selModel: sm, > + delay: 5, > + callback: function() { > + reload(); > + }, > + baseurl: baseurl + '/', > + }); > } > me.tbar.push( > '->', >