Hi Josh, On 31.03.22 22:39, Joshua Huber wrote: ... > > 1) The simplest: if there's no entry in storageSchema for value, just > return value as the "formatted" storage type. This seems like a great > low-cost, low-risk change. > > diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js > index aafe359a..9ebd3758 100644 > --- a/www/manager6/Utils.js > +++ b/www/manager6/Utils.js > @@ -1001,7 +1001,7 @@ Ext.define('PVE.Utils', { > if (schema) { > return schema.name; > } > - return Proxmox.Utils.unknownText; > + return value; > }, > > format_ha: function(value) {
That's actually a good point, returning unknown here isn't really helping anyone, especially as it's a place where we known that there can be values not under Proxmox VE's control, so the value is definitively the better fallback. I pushed out a commit with your proposed 1) behavior as its a good stop-gap measurement in any way and doesn't harm any fancier way we may possibly add in the future. https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=ea9b688a8234bbdb8cd9e0f6498bed29777a1e2c > > 2) Slightly fancier: provide a common plugin-layer configuration > option (e.g., "type_name"), which allows the storage to name itself, > and even allows the end-user to give friendlier names to different > storage pools they've defined. In pve-manager, all we need is a > different final return statement: > > + return record && record.get('type_name') || value; would need to be encoded to avoid HTML-element or JS injection, e.g., via the `Ext.htmlEncode()`, besides that it would seem OK from a technical POV. > > And a new entry in the default storage plugin property list (in > pve-storage, PVE/Storage/Plugin.pm): > > + type_name => { not all to hard feelings but for most new properties we use kebab case, i.e., here it'd be 'type-name' (needs the quotes then too, else perl gets confused as you probably already know), but due to backward compat we got quite the mix present, at least in Proxmox VE (the newer, rust based PBS is quite a bit more consistent). Possibly alternative: 'type-text' (as one could draw parallels to our JSONSchema's internal 'format-text, which is used roughly similarly for property format definitions). > + description => "User-friendly display name for storage", > + type => 'string', > + optional => 1, adding a generous maxLength (say something between 64 to 128) and a pattern to avoid other whitespace-types than, well, a plain space would IMO be good to add. > + }, > > This has the possible side-benefit of allowing the type name to be > used in command line tools as well as the UI, which seems like it'd be > nice for consistency. Seems like an OK argument for me, especially considering the relatively small impact. > > 3) Something extremely fancy: 3rd-party storage plugins bundle/provide > an associated front-end component. This is clearly superior, but is > much more involved. It could could enable adding and editing 3rd-party > via the pve-manager UI, provide a customized type name, and even a > custom icon! 😄 > Without encoding 2) would already be quite close to that, but I'd really like to avoid adding generic XSS injection as a feature ;-) > > Any of these ideas sound interesting? At this point, I'd be pretty > happy with #1 or #2 -- we don't have the time (nor really the > expertise with ExtJS) to take on the fancy option at the moment. Any > other ideas? I'm happy to submit a patch! (I'll get the CLA process > started, as well.) Hehe, sorry, seems like I jumped the gun on reading and already pushed out 1) as a low hanging fruit before I even got to this paragraph, maybe a good sign to call this Friday a day and go home :-) Anyhow, if you want to for 2) then please feel free to do so. cheers, Thomas _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel