Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate names helper #10

Closed
igalic opened this issue Aug 22, 2017 · 9 comments · Fixed by #17
Closed

validate names helper #10

igalic opened this issue Aug 22, 2017 · 9 comments · Fixed by #17

Comments

@igalic
Copy link
Collaborator

igalic commented Aug 22, 2017

almost all of our name arguments have the same general format:

/^[a-z0-9][a-z0-9-_\.]*/i

given that many of our names are used as part of a path, we can further restrict the *

/^[a-z0-9][a-z0-9-_\.]{0,254}/i

to FreeBSD's NAME_MAX (255).


edit with conclusions from the comments

HOWEVER, until FreeBSD 12, fstab's mount paths are further restricted to 88, and not 255. Since we're supporting freebsd all the way down to 9.3, we should also limit our names.

As we've seen from experience from using UUIDs as names, 32 is a length that can stretch it. I suggest only using half of that (16)

/^[a-z0-9][a-z0-9-_\.]{0,15}/i
@igalic
Copy link
Collaborator Author

igalic commented Aug 22, 2017

this should also greatly help us with testing, btw.

@gronke
Copy link
Member

gronke commented Aug 22, 2017

Shouldn't we expect a much shorter path than that? The resulting path will be something like /iocage/jails/<NAME>/root/usr/local/etc/rc.conf, so that name is not allowed to have 255 characters.

Using uuids didn't have this limitation. Maybe rolling back to use UUIDs is the safest way to guarantee compatibility with long names. What do you think?

@igalic
Copy link
Collaborator Author

igalic commented Aug 22, 2017

we're mixing concepts here.

You're talking about MAX_PATH, which in general is 1024:

/* from /usr/src/sys/sys/syslimits.h */
#define      PATH_MAX                 1024   /* max bytes in pathname */

a single component of a path underlies the restrictions of NAME_MAX

/* from /usr/src/sys/sys/syslimits.h */
#define      NAME_MAX                  255   /* max bytes in a file name */

however!!

freebsd's fstab imposes additional restrictions on a mount path's length until FreeBSD 12 is released, this will still be 88, and until then, we should equally restrict ourselves to making /iocage/jails/<NAME>/root/usr/local/etc/rc.conf a maximum of 88 (if not shorter)

@gronke
Copy link
Member

gronke commented Aug 22, 2017

We don't know the mountpoint of the iocage root dataset, so finding a suitable max length for names will probably end up in a random guess. I'm in favor of decoupling the jail identifier and name again. It's nice to represent a jail with it's name in the filesystem, but listing jail names and paths is not a big deal with the cli list command.

@skarekrow this would remove a recently introduced feature. Each jail would keep it's name though, but always use a UUID as filesystem reference.

Also this allows us to permit arbitrary characters as jail name. Why shouldn't mycustomer/webserver or ǝƃɐɔoı be valid names name.

@igalic
Copy link
Collaborator Author

igalic commented Aug 22, 2017

so, uuid, name, hostname & tags should become distinct properties again. i'm all for it.
and, if we introduced the concept of --columns for to the cli, a user could decide for themself what they want printed on iocage list


see #12.

@skarekrow
Copy link
Contributor

I don't see the point in limiting the max name, as that value will change eventually, and we're limiting for no reason at that point.

hostname has always been it's own property, still is. tag was never meant to be a property, and I do not agree with having 3 different ways of naming the jail. It was a conscious decision to use just one name. Not a limitation.

It's very hard for a user to see all the UUIDs and map them to their jails, when doing tasks like zfs send outside of iocage. In addition it means more book keeping on our part, for something that is unneccessary. Jails shouldn't have / in their name, as it would no longer be a valid dataset, by playing by zfs's rules everybody wins. There is a clear understanding of concepts for the user.

@igalic
Copy link
Collaborator Author

igalic commented Aug 22, 2017

which brings us back to my initial proposal of having ascii-only names which are (severely;) limited in length, so we can fit them in all manner of mounts. Perhaps 16?

and we can use tags for the fancy things that @gronke suggested.

@skarekrow
Copy link
Contributor

@igalic I agree with you. tags should be reintroduced as actual tags, not just symlinks in the filesystem. But a tagging system as I suggested a while back in iocage's issue tracker.

TLDR ; Limit names to some small value and don't do anything else until we have a database.

igalic added a commit to igalic/libioc that referenced this issue Aug 22, 2017
igalic added a commit to igalic/libioc that referenced this issue Aug 22, 2017
igalic added a commit to igalic/libioc that referenced this issue Aug 23, 2017
igalic added a commit to igalic/libioc that referenced this issue Aug 23, 2017
gronke pushed a commit that referenced this issue Aug 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants