Dan Guernsey wrote:
Michael DeHaan wrote:
Dan Guernsey wrote:
I am unable to test these at this time (I get an error from api.py when it encounters:
import acls
What package/file does this come from?)
Meanwhile, here are two patches. The first moves the builtins into cheetah_macros and provides documentation. The second adds cheetah_macros to setup.py and MANIFEST.in (you'll want to check my work).
I'll be as prompt as possible to help resolve any errors that this untested code may (will) introduce.
~ Dan
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Thanks!
I tested this briefly with a doubly nested snippet and it looks good. I've additionally modified cobbler.spec so it can be packaged with the right permissions (and is marked "noreplace" so upgrades do not remove user changes).
OK, I'll remember that in any future patches
I think the cheetah_macros file could still use considerable cleanup though this now it makes things much easier to edit.
Points regarding the macro file:
- Examples in things like "includeall" don't seem to be complete --
perhaps have someone unfamiliar with what they do try to understand them?
I didn't mean to confuse anyone with all the wget and tar commands or anything, but all it does is include all files in a given directory. The result is basically the catenation of every file in the named directory.
Sure, and it explodes them on the client? Basically a bit more text about the how would clear that up, I'd think. Others are welcome to help review too if they have comments.
- Lack of indentation. #raw... #endraw might help with this.
I wanted to avoid any unexplained indentation in the resulting kickstart file. I must admit I didn't do much experimentation with the indentation (as far as what ends up in the result), but if I remember right, it would have required a lot of #slurp commands.
That's reasonable then.
- Code for things like "set permissions" seems a bit weird (things like
file[4] and repeated calls to chown, why not a loop?)
It does use a loop, (yes, the indentation -- rather lack of -- makes it confusing, but a loop encompasses the entire function, the multiple calls to chown represents the different ways of calling it (plain, 'p', just calls chown. recursive, 'r', adds the -R flag. find, 'f', uses 'find' to call chown recursively with restrictions.
The 'file' tuple is basically just a set of parameters. Each tuple in the list passed to set_permissions with be translated into a chown and/or chmod command: the first element of the tuple specifies which form (discussed above), the next are the owner, group, and mode. Finally is the file or directory name. In the case a find statement ('f' form) is used, an additional element is used to specify a pattern. Note that it will only call chmod if a mode is specified. Likewise, chown is used if an owner is specified, and chgrp is used if a group is specified. If both are specified, just chown is used (i.e. chown owner:group file). This did end up being more complicated than I intended, but it works.
Maybe an example showing results will help:
$set_permissions([ ('p', 'root', 'root', '600', 'my_file'), ])
yields:
chown root:root my_file chmod 600 my_file
another:
$set_permissions([ ('r', '', 'audio', '', '/usr/share/sounds'), ])
yields:
chgrp -R audio /usr/share/sounds
both with one call:
$set_permissions([ ('p', 'root', 'root', '600', 'my_file'), ('r', '', ''audio', '', '/usr/share/sounds'), ])
yields:
chown root:root my_file chmod 600 my_file chgrp -R audio /usr/share/sounds
This encourages a central location for setting file permissions. Even if permissions are scattered among several snippets, we can keep the chown/chgrp/chmod commands centralized in the result:
among the snippets:
$my_permissions.append(('p', 'root', 'root', '600', 'my_file'))
later in the kickstart template:
$set_permissions($my_permissions)
also, because there is a predictable format for the list of tuples (essentially a table), if the permissions are stored in a variable, they can be checked for compliance by the server.
Yeah, I understand the idea here, but it still seems a bit complicated to me, seeing just having a call to chown would make it available and be readily obvious to *nix admins. Perhaps we keep the macros file fairly minimalistic (perhaps containing only one or two simple examples) and then manage the more complex things (such as these macros) as part of the STIG templates if someone wants to use them? I believe it would be safe for a snippet to include another snippet with those defs.
Some of the config munging is starting to get into things that would be better managed by something like augeas's augtool, though I suppose we can't assume it would be installed on all systems. It would definitely make a lot of the config munging easier.
- Lacking examples for some of the copy file functions
The operation of the two copy file functions is so similar, that I provided an example only for one. I assume the difference between truncating and appending files is well-understood by most system admins.
I find it's usually better to err on the side of too much documentation, as folks often have limited time or don't read things the same way. Saves trouble re-explaining things later over email and/or IRC.
It might actually be easier to start a Wiki page for documentation on the macros feature and explain those there, where there's more room for elaboration.
I'll see what time I have to work on it. Perhaps there I should show the resulting kickstart alongside my examples.
That would probably be good... perhaps follow a common format for the comments for each one that always includes the example input and the example output.
Anyhow, this is all applied in git devel now. Much appreciated!
--Michael
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler
Side note: do you have any suggestions for resolving the 'import acls' issue I'm having?
Should have been added into version control as of yesterday.
Anyway, I think we need to figure out if we want to maintain a library of "supported" macros here, or we just want to support the ability to include your own, and we include a few demos.
I used to know how to read perl -npe 's/^([ \t]*${param_name}[ \t]+)[\x21-\x7E]*([ \t]*(#.*)?)$/${1}${sedesc($value)}, but it's not something I want to encourage seeing more of.
Snippets referencing augtool instead may really be worth exploring.
~ Dan
cobbler mailing list cobbler@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/cobbler