Re: [pve-devel] [PATCH manager 2/2] fix#5570 ui: ceph: make MDS ID configurable

[Christoph Heiss <c.heiss@proxmox.com>]

On Wed, Jul 10, 2024 at 04:18:25PM GMT, Aaron Lauterer wrote:
>
>
> On  2024-07-10  15:59, Christoph Heiss wrote:
> > On Wed, Jul 10, 2024 at 02:43:00PM GMT, Aaron Lauterer wrote:
> > > [..]
> > >   www/manager6/ceph/ServiceList.js | 21 +++++++++++----------
> > >   1 file changed, 11 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/www/manager6/ceph/ServiceList.js b/www/manager6/ceph/ServiceList.js
> > > index 76710146..9075f197 100644
> > > --- a/www/manager6/ceph/ServiceList.js
> > > +++ b/www/manager6/ceph/ServiceList.js
> > > [..]
> > > @@ -40,15 +39,17 @@ Ext.define('PVE.CephCreateService', {
> > >   	    listeners: {
> > >   		change: function(f, value) {
> > >   		    let view = this.up('pveCephCreateService');
> > > +		    view.lookup('mds-id').setValue(value);
> > >   		    view.setNode(value);
> > >   		},
> > >   	    },
> > >   	},
> > >   	{
> > >   	    xtype: 'textfield',
> > > -	    fieldLabel: gettext('Extra ID'),
> > > -	    regex: /[a-zA-Z0-9]+/,
> > > -	    regexText: gettext('ID may only consist of alphanumeric characters'),
> > > +	    reference: 'mds-id',
> > > +	    fieldLabel: gettext('MDS ID'),
> > > +	    regex: /^([a-zA-Z]([-a-zA-Z0-9]*[a-zA-Z0-9])?)$/,
> > > +	    regexText: gettext('ID may consist of alphanumeric characters and hyphen. It cannot start with a number or end in a hyphen.'),
> >
> > Is there a check in the backend whether the name ends with a hyphen? If
> > not, should there be one?
>
> Yes, the parameter checks [0] against the Ceph Service Regex [1] which is
> basically a hostname regex.

Perfect, that's fine then.

With that resolved, you can also consider the entire series:

Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>

>
> >
> > Also, while at it, maybe also set `maxLength` to a sensible value?
> > E.g. while the API schema allows up to 200 characters, it then fails to
> > create a task log file with "File name too long" :^)
> > ~ 180 characters seem to be fine, but probably something like 128
> > characters is already a pretty sensible limit.
>
> nice catch. we might want to lower the limit in the API then, and could also
> set the limit in the GUI.
> There might be other places in the API with a similar issue... glancing at
> all the other Ceph Services which can get customized names with the API/CLI.
>

I really was just mashing the keyboard in the usual "lets see what
breaks" fashion here :^)

Can be done in a separate patch(-series) though, as I don't think its
*that* important - esp. considering such long names are (hopefully) very
much an edge case.

>
> [0] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=PVE/API2/Ceph/MDS.pm;h=6fc0ae450b7f65b5b5b70f72fb23f2b8dbef5a98;hb=HEAD#l107
> [1] https://git.proxmox.com/?p=pve-manager.git;a=blob;f=PVE/Ceph/Services.pm;h=e0f31e8eb6bc9b3777b3d0d548497276efaa5c41;hb=HEAD#l14


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel