Closed nlvw closed 3 years ago
Hey thanks for the PR! I left some comments, mostly on executable config files.
Thanks again! I'm happy to add this. I'd guess I'd do have a question on why you prefer symbolic notation over octal numbers? Any reason in particular?
Also I don't know if I like X
vs x
. It seems like if we're setting the permission on the directory or file, why leave the behaviour to be defined at runtime? It seems like if we want to force the directory to a certain state, then let's do that. X
to me seems wishy washy. Like maybe we'll change this, but maybe we won't!
@johrstrom I've made some corrections per your comments. When dealing with specifically files, or directories I've removed the 'X' in favor of 'x' (or without 'x') respectively. There are a few places were the 'X' is needed such as when unarchiving the passenger tarball or copying folders (with content) in install-src.yml.
Concerning symbolic notation I prefer it for a few important reasons. First is that outside of Ansible it provides additional features when using 'chmod' compared to octal numbers ('X' is a very good example of this). Second, is that when using tools such as 'ls' or 'getfacl' permissions are listed using symbolic notation. These first two points combined is why when onboarding new team members, graduate assistants, or users I find it easier to teach symbolic notation. Now for ansible specifically, I prefer symbolic because when you are setting 'mode'/permissions in tasks there are a few instances were I feel it is required. This role runs across both of those instances (unarchive and copying folders with content). To remain consistent in the role and for better verbosity is why I used symbolic notation everywhere.
That being said if you prefer octal I can make that change where applicable. There are a few tasks where it has to use symbolic unless you want everything to be executable.
Thansk for the info. The PR looks good. Thank you for the contribution!
After merging I see you have two GH users. That's fine by me, it's just a heads up that you should fix your git settings.
In a hardened server environment it is common to change the umask of the root user (and others) to a more restrictive setting such as '027' which remove default permissions from 'other'. This breaks this role as many files that need to be read by other are not readable.
To remediate I've done the following:
To note, when setting something to empty like 'o=' it will remove all permissions for 'o'. Also, when using capital 'X' directories, binaries, and anything already executable will be made executable while everything else will not.