Re: [pve-devel] [PATCH manager] certs: early renew long-lived certificates
On 4/23/20 1:59 PM, Fabian Grünbichler wrote: > On April 23, 2020 1:07 pm, Dominik Csapak wrote: >> LGTM >> >> maybe we should shorten the lifespan to 1 year already? >> according to [0], safari on macos will reject certs >> that are longer valid than 398 days, when issued on/after >> 2020-09-01 >> >> 0: https://support.apple.com/en-us/HT211025 >> > > forgot to include this tidbit: that change was actually the reason for > looking at it, but it only affects certificates issued by CAs shipped in > the Apple Trust Stores, not those issued by CAs manually trusted by a > user. so our self-signed CA and its certificates are not affected (for > now). This all makes me thinking... Wouldn't we need to have the PMG also adapt to this? Checked a very recently from (new test) ISO installed test VM gets me a 10 year certificate lifespan.. I mean, there more may use a "trusted" one, but still.. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 5/6] ui: CPUModelSelector: use API call for store
comments inline On 4/22/20 3:39 PM, Stefan Reiter wrote: CPU models are retrieved from the new /nodes/X/cpu call and ordered by vendor to approximate the previous sort order (less change for accustomed users). With this, custom CPU models are now selectable via the GUI. Signed-off-by: Stefan Reiter --- www/manager6/form/CPUModelSelector.js | 221 ++ 1 file changed, 81 insertions(+), 140 deletions(-) diff --git a/www/manager6/form/CPUModelSelector.js b/www/manager6/form/CPUModelSelector.js index 1d28ee88..cdb7ddcb 100644 --- a/www/manager6/form/CPUModelSelector.js +++ b/www/manager6/form/CPUModelSelector.js @@ -1,9 +1,19 @@ +Ext.define('PVE.data.CPUModel', { +extend: 'Ext.data.Model', +fields: [ + {name: 'name'}, + {name: 'vendor'}, + {name: 'custom'}, + {name: 'displayname'} +] +}); + Ext.define('PVE.form.CPUModelSelector', { extend: 'Proxmox.form.ComboGrid', alias: ['widget.CPUModelSelector'], -valueField: 'value', -displayField: 'value', +valueField: 'name', +displayField: 'displayname', emptyText: Proxmox.Utils.defaultText + ' (kvm64)', allowBlank: true, @@ -19,157 +29,88 @@ Ext.define('PVE.form.CPUModelSelector', { columns: [ { header: gettext('Model'), - dataIndex: 'value', + dataIndex: 'displayname', hideable: false, sortable: true, - flex: 2 + flex: 3 }, { header: gettext('Vendor'), dataIndex: 'vendor', hideable: false, sortable: true, - flex: 1 + flex: 2 } ], - width: 320 + width: 360 }, store: { - fields: [ 'value', 'vendor' ], - data: [ - { - value: 'athlon', - vendor: 'AMD' - }, - { - value: 'phenom', - vendor: 'AMD' - }, - { - value: 'Opteron_G1', - vendor: 'AMD' - }, - { - value: 'Opteron_G2', - vendor: 'AMD' - }, - { - value: 'Opteron_G3', - vendor: 'AMD' - }, - { - value: 'Opteron_G4', - vendor: 'AMD' - }, - { - value: 'Opteron_G5', - vendor: 'AMD' - }, - { - value: 'EPYC', - vendor: 'AMD' - }, - { - value: '486', - vendor: 'Intel' - }, - { - value: 'core2duo', - vendor: 'Intel' - }, - { - value: 'coreduo', - vendor: 'Intel' - }, - { - value: 'pentium', - vendor: 'Intel' - }, - { - value: 'pentium2', - vendor: 'Intel' - }, - { - value: 'pentium3', - vendor: 'Intel' - }, - { - value: 'Conroe', - vendor: 'Intel' - }, - { - value: 'Penryn', - vendor: 'Intel' - }, - { - value: 'Nehalem', - vendor: 'Intel' - }, - { - value: 'Westmere', - vendor: 'Intel' - }, - { - value: 'SandyBridge', - vendor: 'Intel' - }, - { - value: 'IvyBridge', - vendor: 'Intel' - }, - { - value: 'Haswell', - vendor: 'Intel' - }, - { - value: 'Haswell-noTSX', - vendor: 'Intel' - }, - { - value: 'Broadwell', - vendor: 'Intel' - }, - { - value: 'Broadwell-noTSX', - vendor: 'Intel' - }, - { - value: 'Skylake-Client', - vendor: 'Intel' - }, - { - value: 'Skylake-Server', - vendor: 'Intel' - }, - { - value: 'Cascadelake-Server', - vendor: 'Intel' - }, - { - value: 'KnightsMill', - vendor: 'Intel' - }, - { - value: 'kvm32', - vendor: 'QEMU' - }, - { - value: 'kvm64', - vendor: 'QEMU' - }, - { - value: 'qemu32', - vendor: 'QEMU' - }, - { - value: 'qemu64', - vendor: 'QEMU' - }, - { -
Re: [pve-devel] [PATCH manager 4/6] ui: ProcessorEdit: fix total core calculation and use view model
nice that you cleaned up the panel :) (LGTM) but the actual fix would be much easier ;) since the 'total cores' is just informational we can set 'isFormField: false' on the totalcore field which then will not be reset (and not tracked as a formfield) if you do this you can drop the whole setValues function On 4/22/20 3:39 PM, Stefan Reiter wrote: Clean up the code in ProcessorEdit with a view model and fix a bug while at it - previously, pressing the 'Reset' button on the form would always set the value of the total core count field to 1. Signed-off-by: Stefan Reiter --- The fix is technically only the setValues part, but I started off thinking that using a view model would also fix it - it did not, but I still think it looks nicer than before. www/manager6/qemu/ProcessorEdit.js | 65 ++ 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/www/manager6/qemu/ProcessorEdit.js b/www/manager6/qemu/ProcessorEdit.js index bc17e152..d555b2d8 100644 --- a/www/manager6/qemu/ProcessorEdit.js +++ b/www/manager6/qemu/ProcessorEdit.js @@ -5,28 +5,18 @@ Ext.define('PVE.qemu.ProcessorInputPanel', { insideWizard: false, -controller: { - xclass: 'Ext.app.ViewController', - - updateCores: function() { - var me = this.getView(); - var sockets = me.down('field[name=sockets]').getValue(); - var cores = me.down('field[name=cores]').getValue(); - me.down('field[name=totalcores]').setValue(sockets*cores); - var vcpus = me.down('field[name=vcpus]'); - vcpus.setMaxValue(sockets*cores); - vcpus.setEmptyText(sockets*cores); - vcpus.validate(); +viewModel: { + data: { + socketCount: 1, + coreCount: 1, }, + formulas: { + totalCoreCount: get => get('socketCount') * get('coreCount'), + }, +}, - control: { - 'field[name=sockets]': { - change: 'updateCores' - }, - 'field[name=cores]': { - change: 'updateCores' - } - } +controller: { + xclass: 'Ext.app.ViewController', }, onGetValues: function(values) { @@ -76,6 +66,16 @@ Ext.define('PVE.qemu.ProcessorInputPanel', { return values; }, +setValues: function(values) { + let me = this; + + // set the original value here, else 'reset' clears the field + let totalCoreDisplay = me.lookupReference('totalcores'); + totalCoreDisplay.originalValue = values.cores * values.sockets; + + me.callParent([values]); +}, + cpu: {}, column1: [ @@ -86,7 +86,10 @@ Ext.define('PVE.qemu.ProcessorInputPanel', { maxValue: 4, value: '1', fieldLabel: gettext('Sockets'), - allowBlank: false + allowBlank: false, + bind: { + value: '{socketCount}', + }, }, { xtype: 'proxmoxintegerfield', @@ -95,8 +98,11 @@ Ext.define('PVE.qemu.ProcessorInputPanel', { maxValue: 128, value: '1', fieldLabel: gettext('Cores'), - allowBlank: false - } + allowBlank: false, + bind: { + value: '{coreCount}', + }, + }, ], column2: [ @@ -109,8 +115,11 @@ Ext.define('PVE.qemu.ProcessorInputPanel', { xtype: 'displayfield', fieldLabel: gettext('Total cores'), name: 'totalcores', - value: '1' - } + reference: 'totalcores', + bind: { + value: '{totalCoreCount}', + }, + }, ], advancedColumn1: [ @@ -123,7 +132,11 @@ Ext.define('PVE.qemu.ProcessorInputPanel', { fieldLabel: gettext('VCPUs'), deleteEmpty: true, allowBlank: true, - emptyText: '1' + emptyText: '1', + bind: { + emptyText: '{totalCoreCount}', + maxValue: '{totalCoreCount}', + }, }, { xtype: 'numberfield', ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-i18n] Update Arabic translation
Signed-off-by: Moayad Almalat --- ar.po | 177 +- 1 file changed, 76 insertions(+), 101 deletions(-) diff --git a/ar.po b/ar.po index eb09a68..cec3aa4 100644 --- a/ar.po +++ b/ar.po @@ -11,14 +11,14 @@ msgstr "" "Project-Id-Version: pve-manager 31813246103b2582162f422dc34d8077eaee1e01\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: Wed Apr 22 15:07:47 2020\n" -"PO-Revision-Date: YEAR-MO-DA HO:MI +ZONE\n" -"Last-Translator: FULL NAME \n" +"PO-Revision-Date: 2020-04-23 14:43+0200\n" "Language-Team: LANGUAGE \n" "Language: ar\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "X-Generator: Poedit 2.2.1\n" +"Last-Translator: \n" #: pve-manager/www/manager6/lxc/MPEdit.js:236 msgid "/some/path" @@ -138,15 +138,13 @@ msgstr "إضافة CephFS جديدة إلى تكوين تخزين الكتلة." #: pve-manager/www/manager6/ceph/Pool.js:58 msgid "Add the new pool to the cluster storage configuration." -msgstr "Add the new pool to the cluster storage configuration." +msgstr "إضافة المجموعة الجديدة إلى تكوين تخزين المجموعة." #: pve-manager/www/manager6/ceph/CephInstallWizard.js:224 msgid "" "Additional monitors are recommended. They can be created at any time in the " "Monitor tab." -msgstr "" -"Additional monitors are recommended. They can be created at any time in the " -"Monitor tab." +msgstr "يوصى بوجود شاشات إضافية. يمكن إنشائها في أي وقت في تبويب الشاشة." #: pmg-gui/js/UserBlackWhiteList.js:34 pmg-gui/js/UserBlackWhiteList.js:194 #: pve-manager/www/manager6/ceph/ServiceList.js:342 @@ -611,9 +609,8 @@ msgid "Capacity" msgstr "القدرات" #: pve-manager/www/manager6/ceph/Status.js:57 -#, fuzzy msgid "Ceph Version" -msgstr "الإصدار" +msgstr "الإصدار الـ Ceph" #: pve-manager/www/manager6/ceph/CephInstallWizard.js:183 msgid "Ceph cluster configuration" @@ -907,7 +904,7 @@ msgstr "وحدة التحكم" #: pve-manager/www/manager6/dc/OptionView.js:85 msgid "Console Viewer" -msgstr "عارض لوحة المفاتيح" +msgstr "Console Viewer" #: pve-manager/www/manager6/lxc/Options.js:96 #: pve-manager/www/manager6/lxc/Options.js:100 @@ -960,14 +957,12 @@ msgstr "تصفية نوع المحتوى" #: pve-manager/www/manager6/form/SDNControllerSelector.js:26 #: pve-manager/www/manager6/sdn/zones/EvpnEdit.js:38 -#, fuzzy msgid "Controller" -msgstr "وحدة تحكم SCSI" +msgstr "المراقب" #: pve-manager/www/manager6/dc/Config.js:151 -#, fuzzy msgid "Controllers" -msgstr "وحدة تحكم SCSI" +msgstr "وحدات التحكم" #: proxmox-widget-toolkit/Utils.js:476 proxmox-widget-toolkit/Utils.js:494 #: pve-manager/www/manager6/lxc/CmdMenu.js:157 @@ -1057,9 +1052,8 @@ msgid "Create Cluster" msgstr "إنشاء مجموعة" #: pve-manager/www/manager6/lxc/FeaturesEdit.js:67 -#, fuzzy msgid "Create Device Nodes" -msgstr "أجهزة بوساطة" +msgstr "إنشاء عقد الأجهزة" #: pve-manager/www/manager6/Workspace.js:262 #: pve-manager/www/manager6/node/CmdMenu.js:9 @@ -1068,16 +1062,16 @@ msgstr "إنشاء VM" #: pve-manager/www/manager6/node/ACME.js:178 msgid "Created" -msgstr "خلقت" +msgstr "تم الإنشاء" #: pmg-gui/js/SpamQuarantineOptions.js:18 msgid "Custom" -msgstr "المخصصة" +msgstr "مخصص" #: pve-manager/www/manager6/dc/TFAEdit.js:167 msgid "" "Custom 2nd factor configuration is not supported on realms with '{0}' TFA." -msgstr "تكوين العامل الثاني المخصص غير مدعوم في النطاقات باستخدام '{0}' TFA." +msgstr "لا يتم دعم تكوين المعامل الثاني المخصص في ملفات التخصيص مع '{0}' TFA." #: pmg-gui/js/SpamDetectorCustom.js:112 pmg-gui/js/SpamDetectorCustom.js:157 msgid "Custom Rule Score" @@ -1085,7 +1079,7 @@ msgstr "نقاط القاعدة المخصصة" #: pmg-gui/js/SpamDetectorConfiguration.js:28 msgid "Custom Scores" -msgstr "درجات مخصصة" +msgstr "علامات مخصصة" #: pve-manager/www/manager6/ceph/OSD.js:51 msgid "DB Disk" @@ -1319,7 +1313,7 @@ msgstr "تدمير \"{0}\"" #: proxmox-widget-toolkit/Utils.js:526 msgid "Destroy image from unknown guest" -msgstr "" +msgstr "إتلاف الصورة من ضيف مجهول" #: pve-manager/www/manager6/qemu/HardwareView.js:444 msgid "Detach" @@ -1373,9 +1367,8 @@ msgid "Directory Storage" msgstr "تخزين الدليل" #: pmg-gui/js/MailProxyRelaying.js:29 -#, fuzzy msgid "Disable MX lookup (SMTP)" -msgstr "تعطيل MX lookup" +msgstr "تعطيل MX lookup (SMTP)" #: proxmox-widget-toolkit/Utils.js:43 pve-manager/www/manager6/Utils.js:250 msgid "Disabled" @@ -1386,10 +1379,11 @@ msgid "" "Disabling the limiter can potentially allow a guest to overload the host. " "Proceed with caution." msgstr "" +"من المحتمل أن يسمح تعطيل المحدد للضيف بزيادة التحميل على المضيف. التقدم بحذر." #: pve-manager/www/manager6/qemu/HDEdit.js:230 msgid "Discard" -msgstr "نبذ" +msgstr "تجاهل" #: pmg-gui/js/PostfixQShape.js:110 msgid "Discard address verification database" @@ -1813,7 +1807,7 @@ msgstr "مجموعات وحدات التخزين الموجودة" #: pve-manager/www/manager6/lxc/FeaturesEdit.js:68 msgid "Experimental" -msgstr "" +msgstr "تجريبي" #:
Re: [pve-devel] [PATCH guest-common 2/2] fix 2682: make sure configuration file is up-to-date for lock_config-variants
On 23.04.20 13:51, Fabian Ebner wrote: See [0] for the details. The call tree for the variants is lock_config -> lock_config_full -> lock_config_mode so it is sufficient to adapt lock_config_mode. [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=2682 Suggested-by: Fabian Grünbichler Signed-off-by: Fabian Ebner --- PVE/AbstractConfig.pm | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm index f1b395c..15368de 100644 --- a/PVE/AbstractConfig.pm +++ b/PVE/AbstractConfig.pm @@ -292,7 +292,13 @@ sub lock_config_mode { my $filename = $class->config_file_lock($vmid); -my $res = lock_file_full($filename, $timeout, $shared, $code, @param); +# make sure configuration file is up-to-date +my $realcode = sub { + PVE::Cluster::cfs_update(); + $code->(@param); +}; + +my $res = lock_file_full($filename, $timeout, $shared, $realcode, @param); @param can/should be dropped from the lock_file_full call, as it's unused now. die $@ if $@; ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] certs: early renew long-lived certificates
On April 23, 2020 1:07 pm, Dominik Csapak wrote: > LGTM > > maybe we should shorten the lifespan to 1 year already? > according to [0], safari on macos will reject certs > that are longer valid than 398 days, when issued on/after > 2020-09-01 > > 0: https://support.apple.com/en-us/HT211025 > forgot to include this tidbit: that change was actually the reason for looking at it, but it only affects certificates issued by CAs shipped in the Apple Trust Stores, not those issued by CAs manually trusted by a user. so our self-signed CA and its certificates are not affected (for now). I don't have any objections to shortening both the issuance and the check here to 1 year though. > On 4/23/20 12:20 PM, Fabian Grünbichler wrote: >> if our self-signed certificate expires in more than 825 days, but was >> created after July 2019 it won't be accepted by modern Apple devices. we >> fixed the issuance to generate shorter-lived certificates in November >> 2019, this cleans up the existing ones to fix this and similar future >> issues. >> >> two years / 730 days as cut-off was chosen since it's our new maximum >> self-signed certificate lifetime, and should thus catch all old-style >> certificates. >> >> another positive side-effect is that we can now phase out support for >> older certificates faster, e.g. if we want to move to bigger keys, >> different signature algorithms, or anything else in that direction. >> >> Signed-off-by: Fabian Grünbichler >> --- >> I'd also be fine with reducing both even more, e.g. to 1 year ;) >> >> bin/pveupdate | 15 --- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/bin/pveupdate b/bin/pveupdate >> index 15a2accc..36ac6814 100755 >> --- a/bin/pveupdate >> +++ b/bin/pveupdate >> @@ -79,8 +79,9 @@ eval { >> my $certpath = >> PVE::CertHelpers::default_cert_path_prefix($nodename).".pem"; >> my $capath = "/etc/pve/pve-root-ca.pem"; >> >> -# check if expiry is < 2W >> -if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) { >> +my $renew = sub { >> +my ($msg) = @_; >> + >> # get CA info >> my $cainfo = PVE::Certificate::get_certificate_info($capath); >> >> @@ -94,13 +95,21 @@ eval { >> # TODO: replace by low level ssleay interface if version 1.86 is >> available >> PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', >> $capath, $certpath]); >> >> -print "PVE certificate expires soon, renewing...\n"; >> +print "PVE certificate $msg\n"; >> # create new certificate >> my $ip = PVE::Cluster::remote_node_ip($nodename); >> PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip); >> >> print "Restarting pveproxy after renewing certificate\n"; >> PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']); >> +}; >> + >> +if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) { >> +# expires in next 2 weeks >> +$renew->("expires soon, renewing..."); >> +} elsif (!PVE::Certificate::check_expiry($certpath, time() + >> 2*365*24*60*60)) { >> +# expires in more than 2 years >> +$renew->("expires in more than 2 years, renewing to reduce certificate >> life-span..."); >> } >> }; >> syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@; >> > > ___ > pve-devel mailing list > pve-devel@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH guest-common 1/2] Avoid duplication by using lock_config_mode
No functional change is intended. Signed-off-by: Fabian Ebner --- PVE/AbstractConfig.pm | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm index beb10c7..f1b395c 100644 --- a/PVE/AbstractConfig.pm +++ b/PVE/AbstractConfig.pm @@ -259,13 +259,7 @@ sub load_current_config { sub lock_config_full { my ($class, $vmid, $timeout, $code, @param) = @_; -my $filename = $class->config_file_lock($vmid); - -my $res = lock_file($filename, $timeout, $code, @param); - -die $@ if $@; - -return $res; +return $class->lock_config_mode($vmid, $timeout, 0, $code, @param); } sub create_and_lock_config { -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH guest-common 2/2] fix 2682: make sure configuration file is up-to-date for lock_config-variants
See [0] for the details. The call tree for the variants is lock_config -> lock_config_full -> lock_config_mode so it is sufficient to adapt lock_config_mode. [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=2682 Suggested-by: Fabian Grünbichler Signed-off-by: Fabian Ebner --- PVE/AbstractConfig.pm | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/PVE/AbstractConfig.pm b/PVE/AbstractConfig.pm index f1b395c..15368de 100644 --- a/PVE/AbstractConfig.pm +++ b/PVE/AbstractConfig.pm @@ -292,7 +292,13 @@ sub lock_config_mode { my $filename = $class->config_file_lock($vmid); -my $res = lock_file_full($filename, $timeout, $shared, $code, @param); +# make sure configuration file is up-to-date +my $realcode = sub { + PVE::Cluster::cfs_update(); + $code->(@param); +}; + +my $res = lock_file_full($filename, $timeout, $shared, $realcode, @param); die $@ if $@; -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 1/4] tests: print more info when ovmf parsing fails
the subjects for this and the 2/4 patch are wrong, replace ovmf with ovf^^ (damn acronyms) ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] certs: early renew long-lived certificates
LGTM maybe we should shorten the lifespan to 1 year already? according to [0], safari on macos will reject certs that are longer valid than 398 days, when issued on/after 2020-09-01 0: https://support.apple.com/en-us/HT211025 On 4/23/20 12:20 PM, Fabian Grünbichler wrote: if our self-signed certificate expires in more than 825 days, but was created after July 2019 it won't be accepted by modern Apple devices. we fixed the issuance to generate shorter-lived certificates in November 2019, this cleans up the existing ones to fix this and similar future issues. two years / 730 days as cut-off was chosen since it's our new maximum self-signed certificate lifetime, and should thus catch all old-style certificates. another positive side-effect is that we can now phase out support for older certificates faster, e.g. if we want to move to bigger keys, different signature algorithms, or anything else in that direction. Signed-off-by: Fabian Grünbichler --- I'd also be fine with reducing both even more, e.g. to 1 year ;) bin/pveupdate | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/bin/pveupdate b/bin/pveupdate index 15a2accc..36ac6814 100755 --- a/bin/pveupdate +++ b/bin/pveupdate @@ -79,8 +79,9 @@ eval { my $certpath = PVE::CertHelpers::default_cert_path_prefix($nodename).".pem"; my $capath = "/etc/pve/pve-root-ca.pem"; -# check if expiry is < 2W -if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) { +my $renew = sub { + my ($msg) = @_; + # get CA info my $cainfo = PVE::Certificate::get_certificate_info($capath); @@ -94,13 +95,21 @@ eval { # TODO: replace by low level ssleay interface if version 1.86 is available PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', $capath, $certpath]); - print "PVE certificate expires soon, renewing...\n"; + print "PVE certificate $msg\n"; # create new certificate my $ip = PVE::Cluster::remote_node_ip($nodename); PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip); print "Restarting pveproxy after renewing certificate\n"; PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']); +}; + +if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) { + # expires in next 2 weeks + $renew->("expires soon, renewing..."); +} elsif (!PVE::Certificate::check_expiry($certpath, time() + 2*365*24*60*60)) { + # expires in more than 2 years + $renew->("expires in more than 2 years, renewing to reduce certificate life-span..."); } }; syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@; ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 4/4] importovf: die with error when disk file is missing
also add missing '\n' at the end of error messages Signed-off-by: Dominik Csapak --- PVE/QemuServer/OVF.pm | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm index 536e0eb..dbcc361 100644 --- a/PVE/QemuServer/OVF.pm +++ b/PVE/QemuServer/OVF.pm @@ -213,12 +213,16 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); my $ovf_dir = realpath(dirname(File::Spec->rel2abs($ovf))); my $backing_file_path = realpath(join ('/', $ovf_dir, $filepath)); if ($backing_file_path !~ /^\Q${ovf_dir}\E/) { - die "error parsing $filepath, are you using a symlink ?"; + die "error parsing $filepath, are you using a symlink ?\n"; + } + + if (!-e $backing_file_path) { + die "error parsing $filepath, file seems not to exist at $backing_file_path\n"; } my $virtual_size; if ( !($virtual_size = PVE::Storage::file_size_info($backing_file_path)) ) { - die "error parsing $backing_file_path, size seems to be $virtual_size"; + die "error parsing $backing_file_path, size seems to be $virtual_size\n"; } $pve_disk = { -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 3/4] importovf: fix import of ovfs without default namespaces
some ovfs to not declare 'rasd' as a default namespace (in the top level Envelope element), but inline in each element (e.g. ...) this trips up our relative findvalue with XPath error : Undefined namespace prefix to avoid this, search in the global XPathContext (where we register those namespaces ourselves) and give the item_node as context this works in both cases Signed-off-by: Dominik Csapak --- PVE/QemuServer/OVF.pm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm index 7ee4dc8..536e0eb 100644 --- a/PVE/QemuServer/OVF.pm +++ b/PVE/QemuServer/OVF.pm @@ -155,7 +155,7 @@ sub parse_ovf { # from Item, find corresponding Disk node # here the dot means the search should start from the current element in dom - my $host_resource = $item_node->findvalue('./rasd:HostResource'); + my $host_resource = $xpc->findvalue('rasd:HostResource', $item_node); my $disk_section_path; my $disk_id; @@ -194,7 +194,7 @@ ovf:Disk[\@ovf:diskId='%s']/\@ovf:fileRef", $disk_id); print "file path: $filepath\n" if $debug; # from Item, find owning Controller type - my $controller_id = $item_node->findvalue('./rasd:Parent'); + my $controller_id = $xpc->findvalue('rasd:Parent', $item_node); my $xpath_find_parent_type = sprintf("/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/\ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); my $controller_type = $xpc->findvalue($xpath_find_parent_type); @@ -205,7 +205,7 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id); print "owning controller type: $controller_type\n" if $debug; # extract corresponding Controller node details - my $adress_on_controller = $item_node->findvalue('./rasd:AddressOnParent'); + my $adress_on_controller = $xpc->findvalue('rasd:AddressOnParent', $item_node); my $pve_disk_address = id_to_pve($controller_type) . $adress_on_controller; # resolve symlinks and relative path components -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 1/4] tests: print more info when ovmf parsing fails
when one of the ovmf tests fails to parse at all, we just get the 'die' message of the failing component, but not which file actually failed to parse to get better output, convert the parsing also to a test and ok() and fail() respectively and then printing the error Signed-off-by: Dominik Csapak --- test/run_ovf_tests.pl | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/run_ovf_tests.pl b/test/run_ovf_tests.pl index b9e4825..74c53bc 100755 --- a/test/run_ovf_tests.pl +++ b/test/run_ovf_tests.pl @@ -12,8 +12,22 @@ use Data::Dumper; my $test_manifests = join ('/', $Bin, 'ovf_manifests'); -my $win2008 = PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win_2008_R2_two-disks.ovf"); -my $win10 = PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win10-Liz.ovf"); +print "parsing ovfs\n"; + +my $win2008 = eval { PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win_2008_R2_two-disks.ovf") }; +if (my $err = $@) { +fail('parse win2008'); +warn("error: $err\n"); +} else { +ok('parse win2008'); +} +my $win10 = eval { PVE::QemuServer::OVF::parse_ovf("$test_manifests/Win10-Liz.ovf") }; +if (my $err = $@) { +fail('parse win10'); +warn("error: $err\n"); +} else { +ok('parse win10'); +} print "testing disks\n"; -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH qemu-server 2/4] tests: add test for ovmf with missing default rasd namespace
sometimes vendors do not put the 'rasd' namespaces in the top level Envelope, but in every 'rasd' element this adds a test for this Signed-off-by: Dominik Csapak --- this test will fail when applied, but will be corrected by the next patch in this series (3/4) for a real world example of this see: https://forum.proxmox.com/threads/qm-importovf-xpath-error-on-importing-ovf.68597/ .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ++ test/run_ovf_tests.pl | 15 ++ 2 files changed, 157 insertions(+) create mode 100755 test/ovf_manifests/Win10-Liz_no_default_ns.ovf diff --git a/test/ovf_manifests/Win10-Liz_no_default_ns.ovf b/test/ovf_manifests/Win10-Liz_no_default_ns.ovf new file mode 100755 index 000..b93540f --- /dev/null +++ b/test/ovf_manifests/Win10-Liz_no_default_ns.ovf @@ -0,0 +1,142 @@ + + +http://schemas.dmtf.org/ovf/envelope/1; xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common; xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1; xmlns:vmw="http://www.vmware.com/schema/ovf; xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;> + + + + +Virtual disk information +http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized; ovf:populatedSize="16798056448"/> + + +The list of logical networks + + The bridged network + + + +A virtual machine +Win10-Liz + + The kind of installed guest operating system + + + Virtual hardware requirements + +Virtual Hardware Family +0 +Win10-Liz +vmx-11 + + +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>hertz * 10^6 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>Number of Virtual CPUs +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4 virtual CPU(s) +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>1 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>3 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4 + + +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>byte * 2^20 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>Memory Size +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>6144MB of memory +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>2 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>6144 + + +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>SATA Controller +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>sataController0 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>3 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>vmware.sata.ahci +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>20 + + +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>USB Controller (XHCI) +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>usb3 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>4 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>vmware.usb.xhci +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>23 + + +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>USB Controller (EHCI) +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>usb +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>5 +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>vmware.usb.ehci +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>23 + + + +http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData;>0 +
Re: [pve-devel] [PATCH storage v4 00/12] Fix: #2124 zstd
On Thu, Apr 23, 2020 at 12:35:34PM +0200, Dominic Jäger wrote: > Thank you for merging the test files! No problem. I hope I didn't miss any. :) > > Love the dropdown to set a compression: > GZIP (good) > ZSTD (better) > > Tests work and creating and restoring backups in the GUI with the new option, > too. Thanks for testing. > > Tested-by: Dominic Jäger > > On Wed, Apr 22, 2020 at 04:57:51PM +0200, Alwin Antreich wrote: > > Zstandard (zstd) [0] is a data compression algorithm, in addition to > > gzip, lzo for our backup/restore. It can utilize multiple core CPUs. But > > by default it has one compression and one writer thread. > > > > > > [0] https://facebook.github.io/zstd/ > > > > Alwin Antreich (12): > > __pve-storage__ > > storage: test: split archive format/compressor > > storage: replace build-in stat with File::stat > > test: parse_volname > > test: list_volumes > > Fix: backup: ctime was from stat not file name > > test: path_to_volume_id > > Fix: path_to_volume_id returned wrong content > > Fix: add missing snippets subdir > > backup: compact regex for backup file filter > > Fix: #2124 storage: add zstd support > > test: get_subdir > > test: filesystem_path > > ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v4 00/12] Fix: #2124 zstd
Thank you for merging the test files! Love the dropdown to set a compression: GZIP (good) ZSTD (better) Tests work and creating and restoring backups in the GUI with the new option, too. Tested-by: Dominic Jäger On Wed, Apr 22, 2020 at 04:57:51PM +0200, Alwin Antreich wrote: > Zstandard (zstd) [0] is a data compression algorithm, in addition to > gzip, lzo for our backup/restore. It can utilize multiple core CPUs. But > by default it has one compression and one writer thread. > > > [0] https://facebook.github.io/zstd/ > > Alwin Antreich (12): > __pve-storage__ > storage: test: split archive format/compressor > storage: replace build-in stat with File::stat > test: parse_volname > test: list_volumes > Fix: backup: ctime was from stat not file name > test: path_to_volume_id > Fix: path_to_volume_id returned wrong content > Fix: add missing snippets subdir > backup: compact regex for backup file filter > Fix: #2124 storage: add zstd support > test: get_subdir > test: filesystem_path > ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v4 04/12] test: list_volumes
I like it. Having for example one $mocked_vmlist is clean. Two ideas for further testing (in the future): - Passing only a subset of $types to list_volumes - Unintended situations, like when $mocked_vmlist has a VM without type => list_volumes roughly does if (defined($type) && $type eq 'lxc') ... else ... On Wed, Apr 22, 2020 at 04:58:00PM +0200, Alwin Antreich wrote: > Test to reduce the potential for accidental breakage on regex changes. > > Co-Authored-by: Dominic Jaeger > Signed-off-by: Alwin Antreich > --- > test/list_volumes_test.pm | 519 ++ > test/run_plugin_tests.pl | 6 +- > 2 files changed, 524 insertions(+), 1 deletion(-) > create mode 100644 test/list_volumes_test.pm > > diff --git a/test/list_volumes_test.pm b/test/list_volumes_test.pm > new file mode 100644 > index 000..c1428f2 > --- /dev/null > +++ b/test/list_volumes_test.pm > @@ -0,0 +1,519 @@ > +package PVE::Storage::TestListVolumes; > + > +use strict; > ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] certs: early renew long-lived certificates
if our self-signed certificate expires in more than 825 days, but was created after July 2019 it won't be accepted by modern Apple devices. we fixed the issuance to generate shorter-lived certificates in November 2019, this cleans up the existing ones to fix this and similar future issues. two years / 730 days as cut-off was chosen since it's our new maximum self-signed certificate lifetime, and should thus catch all old-style certificates. another positive side-effect is that we can now phase out support for older certificates faster, e.g. if we want to move to bigger keys, different signature algorithms, or anything else in that direction. Signed-off-by: Fabian Grünbichler --- I'd also be fine with reducing both even more, e.g. to 1 year ;) bin/pveupdate | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/bin/pveupdate b/bin/pveupdate index 15a2accc..36ac6814 100755 --- a/bin/pveupdate +++ b/bin/pveupdate @@ -79,8 +79,9 @@ eval { my $certpath = PVE::CertHelpers::default_cert_path_prefix($nodename).".pem"; my $capath = "/etc/pve/pve-root-ca.pem"; -# check if expiry is < 2W -if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) { +my $renew = sub { + my ($msg) = @_; + # get CA info my $cainfo = PVE::Certificate::get_certificate_info($capath); @@ -94,13 +95,21 @@ eval { # TODO: replace by low level ssleay interface if version 1.86 is available PVE::Tools::run_command(['/usr/bin/openssl', 'verify', '-CAfile', $capath, $certpath]); - print "PVE certificate expires soon, renewing...\n"; + print "PVE certificate $msg\n"; # create new certificate my $ip = PVE::Cluster::remote_node_ip($nodename); PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip); print "Restarting pveproxy after renewing certificate\n"; PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pveproxy']); +}; + +if (PVE::Certificate::check_expiry($certpath, time() + 14*24*60*60)) { + # expires in next 2 weeks + $renew->("expires soon, renewing..."); +} elsif (!PVE::Certificate::check_expiry($certpath, time() + 2*365*24*60*60)) { + # expires in more than 2 years + $renew->("expires in more than 2 years, renewing to reduce certificate life-span..."); } }; syslog ('err', "Checking/Renewing SSL certificate failed: $@") if $@; -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage v4 02/12] storage: replace build-in stat with File::stat
On Thu, Apr 23, 2020 at 05:52:29AM +0200, Dietmar Maurer wrote: > > On April 22, 2020 6:00 PM Alwin Antreich wrote: > > > > > > On Wed, Apr 22, 2020 at 05:35:05PM +0200, Dietmar Maurer wrote: > > > AFAIK this can have ugly side effects ... > > Okay, I was not aware of any know side effects. > > > > I took the File::stat, since we use it already in pve-cluster, > > qemu-server, pve-common, ... . And a off-list discussion with Thomas and > > Fabian G. > > > > If there is a better solution, I am happy to work on it. > > > # grep -r "use File::stat" /usr/share/perl5/PVE/ > /usr/share/perl5/PVE/QemuServer/Helpers.pm:use File::stat; > /usr/share/perl5/PVE/Storage/ISCSIPlugin.pm:use File::stat; > /usr/share/perl5/PVE/APIServer/AnyEvent.pm:use File::stat qw(); > /usr/share/perl5/PVE/AccessControl.pm:use File::stat; > /usr/share/perl5/PVE/Cluster.pm:use File::stat qw(); > /usr/share/perl5/PVE/LXC/Setup/Base.pm:use File::stat; > /usr/share/perl5/PVE/QemuServer.pm:use File::stat; > /usr/share/perl5/PVE/INotify.pm:use File::stat; > /usr/share/perl5/PVE/API2/APT.pm:use File::stat (); > > So I would use: > > use File::stat qw(); > > to avoid override the core stat() and lstat() functions. Thank you. I will do that and add it to a v5. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common 1/1] JSONSchema: extend pve-configid regex by '-'
On April 23, 2020 7:56 am, Thomas Lamprecht wrote: > On 4/9/20 4:10 PM, Dominik Csapak wrote: >> we use this format for all 'delete' options but we have some options >> that have a '-' in the name (e.g. 'sync-defaults-options') that cannot >> be deleted if it is not included >> >> Signed-off-by: Dominik Csapak >> --- >> src/PVE/JSONSchema.pm | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm >> index 01a3cce..1d28f36 100644 >> --- a/src/PVE/JSONSchema.pm >> +++ b/src/PVE/JSONSchema.pm >> @@ -169,7 +169,7 @@ register_format('pve-configid', \_verify_configid); >> sub pve_verify_configid { >> my ($id, $noerr) = @_; >> >> -if ($id !~ m/^[a-z][a-z0-9_]+$/i) { >> +if ($id !~ m/^[a-z][a-z0-9_-]+$/i) { >> return undef if $noerr; >> die "invalid configuration ID '$id'\n"; >> } > > Fabian, Wolfgang: Any objection here? I think it should be OK, but we may > want to adapt the pve-container and qemu-server config parsers to accept > also the minus in keys, as it will be for sure sooner or later used there > then too. Quick-checking other parsers wouldn't hurt either. :) had some off-list discussions with Dominik about this already, it seems like he checked all the relevant call sites. the only thing I'd still like to see as follow up is re-using this verifier for the guest config parsers when parsing the snapshot section header (by chance, they already contain the '-', but the next time we add a character that might not be the case and we might miss it which would cause some fun bugs ;)). Reviewed-By: Fabian Grünbichler ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common 1/1] JSONSchema: extend pve-configid regex by '-'
On 4/23/20 8:44 AM, Wolfgang Bumiller wrote: > >> On April 23, 2020 7:56 AM Thomas Lamprecht wrote: >> >> >> On 4/9/20 4:10 PM, Dominik Csapak wrote: >>> we use this format for all 'delete' options but we have some options >>> that have a '-' in the name (e.g. 'sync-defaults-options') that cannot >>> be deleted if it is not included >>> >>> Signed-off-by: Dominik Csapak >>> --- >>> src/PVE/JSONSchema.pm | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm >>> index 01a3cce..1d28f36 100644 >>> --- a/src/PVE/JSONSchema.pm >>> +++ b/src/PVE/JSONSchema.pm >>> @@ -169,7 +169,7 @@ register_format('pve-configid', \_verify_configid); >>> sub pve_verify_configid { >>> my ($id, $noerr) = @_; >>> >>> -if ($id !~ m/^[a-z][a-z0-9_]+$/i) { >>> +if ($id !~ m/^[a-z][a-z0-9_-]+$/i) { >>> return undef if $noerr; >>> die "invalid configuration ID '$id'\n"; >>> } >> >> Fabian, Wolfgang: Any objection here? I think it should be OK, but we may >> want to adapt the pve-container and qemu-server config parsers to accept >> also the minus in keys, as it will be for sure sooner or later used there >> then too. Quick-checking other parsers wouldn't hurt either. :) > > You know my answer ;-) Yeah, but I do not want the "I wish for - separate keys" answer I want the "does it break stuff we may not have noticed" answer ;-P > > Btw. those parsers are extremely similar and we could probably get away with > one in guest-common (with a parameter for the pending section name) with > either some post-processing or a callback for some things. A line callback > might definitely be good to handle things like `lxc.*` raw keys. > Yes, the unifying our tons of mostly similar but still different parsers would be great, Fabian and I had a few talks about that already. :) > Oh and, neither accepts `-` in keys yet. That's why I mentioned it here ^^ ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH access-control v2 1/3] domain sync: make options actually required
we want the api options to be optional, but only as long as there are default values set in the realm config since they are all marked as optional (else they would be required in the api) this check did not work as intended instead, set the result to the value of: * the parameter * the set default in the config * the api default in this order if it is undef after this, raise a parameter exception Signed-off-by: Dominik Csapak --- changes from v1: * better structure code to see the precedence of parameters PVE/API2/Domains.pm | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm index b3c3ac4..e139869 100644 --- a/PVE/API2/Domains.pm +++ b/PVE/API2/Domains.pm @@ -372,21 +372,18 @@ my $parse_sync_opts = sub { my $sync_opts_fmt = PVE::JSONSchema::get_format('realm-sync-options'); my $res = {}; +my $defaults = {}; if (defined(my $cfg_opts = $realmconfig->{'sync-defaults-options'})) { - $res = PVE::JSONSchema::parse_property_string($sync_opts_fmt, $cfg_opts); + $defaults = PVE::JSONSchema::parse_property_string($sync_opts_fmt, $cfg_opts); } for my $opt (sort keys %$sync_opts_fmt) { my $fmt = $sync_opts_fmt->{$opt}; - if (exists $param->{$opt}) { - $res->{$opt} = $param->{$opt}; - } elsif (!exists $res->{$opt}) { - raise_param_exc({ - "$opt" => 'Not passed as parameter and not defined in realm default sync options.' - }) if !$fmt->{optional}; - $res->{$opt} = $fmt->{default} if exists $fmt->{default}; - } + $res->{$opt} = $param->{$opt} // $defaults->{$opt} // $fmt->{default}; + raise_param_exc({ + "$opt" => 'Not passed as parameter and not defined in realm default sync options.' + }) if !defined($res->{$opt}); } return $res; }; -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH access-control v2 3/3] domain sync: add 'dry-run' parameter
this can be used to test the resulting config before actually changing anything Signed-off-by: Dominik Csapak --- changes from v1: * rename parameter from 'no-write' to 'dry-run * drop the print sub, instead just mention at the beginning and end that this is just a dry run PVE/API2/Domains.pm | 24 +++- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm index e139869..18698c0 100644 --- a/PVE/API2/Domains.pm +++ b/PVE/API2/Domains.pm @@ -408,7 +408,13 @@ __PACKAGE__->register_method ({ additionalProperties => 0, properties => get_standard_option('realm-sync-options', { realm => get_standard_option('realm'), - }) + 'dry-run' => { + description => "If set, does not write anything.", + type => 'boolean', + optional => 1, + default => 0, + }, + }), }, returns => { description => 'Worker Task-UPID', @@ -420,6 +426,9 @@ __PACKAGE__->register_method ({ my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); + my $write = !(extract_param($param, 'dry-run')); + my $dryrunstring = $write ? '' : ' (dry run)'; + my $realm = $param->{realm}; my $cfg = cfs_read_file($domainconfigfile); my $realmconfig = $cfg->{ids}->{$realm}; @@ -439,7 +448,7 @@ __PACKAGE__->register_method ({ my $plugin = PVE::Auth::Plugin->lookup($type); my $worker = sub { - print "starting sync for realm $realm\n"; + print "starting sync$dryrunstring for realm $realm\n"; my ($synced_users, $dnmap) = $plugin->get_users($realmconfig, $realm); my $synced_groups = {}; @@ -459,12 +468,17 @@ __PACKAGE__->register_method ({ $update_groups->($usercfg, $realm, $synced_groups, $opts); } - cfs_write_file("user.cfg", $usercfg); - print "successfully updated $whatstring configuration\n"; + if ($write) { + cfs_write_file("user.cfg", $usercfg) if $write; + print "successfully updated $whatstring configuration\n" if $write; + } else { + print "NOTE: This is just a dry run. No actual data was written.\n"; + } }, "syncing $whatstring failed"); }; - return $rpcenv->fork_worker('auth-realm-sync', $realm, $authuser, $worker); + my $workerid = $write ? 'auth-realm-sync' : 'auth-realm-sync-test'; + return $rpcenv->fork_worker($workerid, $realm, $authuser, $worker); }}); 1; -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH access-control v2 2/3] auth ldap/ad: introduce connection 'mode'
instead of having only a 'secure' flag which switches between ldap/ldaps we now have a mode which also contains 'ldap+starttls' our connection code in PVE::LDAP can handle this already (used in pmg) so that is no problem if we want to really remove the 'secure' flag, e.g. in 7.0 we'd either have to rewrite the config or have it as an error in a pve6to7 script Signed-off-by: Dominik Csapak --- changes from v1: * refactor get_scheme_and_port PVE/Auth/AD.pm | 9 - PVE/Auth/LDAP.pm | 25 + 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/PVE/Auth/AD.pm b/PVE/Auth/AD.pm index 24b0e9f..4d64c20 100755 --- a/PVE/Auth/AD.pm +++ b/PVE/Auth/AD.pm @@ -27,7 +27,7 @@ sub properties { maxLength => 256, }, secure => { - description => "Use secure LDAPS protocol.", + description => "Use secure LDAPS protocol. DEPRECATED: use 'mode' instead.", type => 'boolean', optional => 1, }, @@ -93,6 +93,7 @@ sub options { group_filter => { optional => 1 }, group_classes => { optional => 1 }, 'sync-defaults-options' => { optional => 1 }, + mode => { optional => 1 }, }; } @@ -110,9 +111,7 @@ sub authenticate_user { my $servers = [$config->{server1}]; push @$servers, $config->{server2} if $config->{server2}; -my $default_port = $config->{secure} ? 636: 389; -my $port = $config->{port} // $default_port; -my $scheme = $config->{secure} ? 'ldaps' : 'ldap'; +my ($scheme, $port) = $class->get_scheme_and_port($config); my %ad_args; if ($config->{verify}) { @@ -130,7 +129,7 @@ sub authenticate_user { $ad_args{verify} = 'none'; } -if ($config->{secure}) { +if ($scheme ne 'ldap') { $ad_args{sslversion} = $config->{sslversion} // 'tlsv1_2'; } diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm index 6b6b184..64250cb 100755 --- a/PVE/Auth/LDAP.pm +++ b/PVE/Auth/LDAP.pm @@ -122,6 +122,13 @@ sub properties { format => 'realm-sync-options', optional => 1, }, + mode => { + description => "LDAP protocol mode.", + type => 'string', + enum => [ 'ldap', 'ldaps', 'ldap+starttls'], + optional => 1, + default => 'ldap', + }, }; } @@ -151,18 +158,28 @@ sub options { group_filter => { optional => 1 }, group_classes => { optional => 1 }, 'sync-defaults-options' => { optional => 1 }, + mode => { optional => 1 }, }; } +sub get_scheme_and_port { +my ($class, $config) = @_; + +my $scheme = $config->{mode} // ($config->{secure} ? 'ldaps' : 'ldap'); + +my $default_port = $scheme eq 'ldaps' ? 636 : 389; +my $port = $config->{port} // $default_port; + +return ($scheme, $port); +} + sub connect_and_bind { my ($class, $config, $realm) = @_; my $servers = [$config->{server1}]; push @$servers, $config->{server2} if $config->{server2}; -my $default_port = $config->{secure} ? 636: 389; -my $port = $config->{port} // $default_port; -my $scheme = $config->{secure} ? 'ldaps' : 'ldap'; +my ($scheme, $port) = $class->get_scheme_and_port($config); my %ldap_args; if ($config->{verify}) { @@ -180,7 +197,7 @@ sub connect_and_bind { $ldap_args{verify} = 'none'; } -if ($config->{secure}) { +if ($scheme ne 'ldap') { $ldap_args{sslversion} = $config->{sslversion} || 'tlsv1_2'; } -- 2.20.1 ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common 1/1] JSONSchema: extend pve-configid regex by '-'
> On April 23, 2020 7:56 AM Thomas Lamprecht wrote: > > > On 4/9/20 4:10 PM, Dominik Csapak wrote: > > we use this format for all 'delete' options but we have some options > > that have a '-' in the name (e.g. 'sync-defaults-options') that cannot > > be deleted if it is not included > > > > Signed-off-by: Dominik Csapak > > --- > > src/PVE/JSONSchema.pm | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm > > index 01a3cce..1d28f36 100644 > > --- a/src/PVE/JSONSchema.pm > > +++ b/src/PVE/JSONSchema.pm > > @@ -169,7 +169,7 @@ register_format('pve-configid', \_verify_configid); > > sub pve_verify_configid { > > my ($id, $noerr) = @_; > > > > -if ($id !~ m/^[a-z][a-z0-9_]+$/i) { > > +if ($id !~ m/^[a-z][a-z0-9_-]+$/i) { > > return undef if $noerr; > > die "invalid configuration ID '$id'\n"; > > } > > Fabian, Wolfgang: Any objection here? I think it should be OK, but we may > want to adapt the pve-container and qemu-server config parsers to accept > also the minus in keys, as it will be for sure sooner or later used there > then too. Quick-checking other parsers wouldn't hurt either. :) You know my answer ;-) Btw. those parsers are extremely similar and we could probably get away with one in guest-common (with a parameter for the pending section name) with either some post-processing or a callback for some things. A line callback might definitely be good to handle things like `lxc.*` raw keys. Oh and, neither accepts `-` in keys yet. ___ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel