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
includedir in suoders can be prefixed by "arroba" #783
includedir in suoders can be prefixed by "arroba" #783
Conversation
Hi. I already signed the individual contributors agreement. Could you retry the github action? |
Since version 1.9.1, @includedir can be used in the sudoers files instead of #includedir: https://github.com/sudo-project/sudo/releases/tag/SUDO_1_9_1 Actually "@includedir" is the modern syntax, and "#includedir" the historic syntax. It has been considered that "#includedir" was too puzzling because it started with a "#" that otherwise denotes comments. This happens to be the default in SUSE Linux enterprise sudoer package, so cloudinit should take this into account. Otherwise, cloudinit was adding an extra #includedir, which was resulting on the files under /etc/sudoers.d being included twice, one by @includedir from the SUSE package, one by the @includedir from cloudinit. The consequence of this, was that if you were defining an Cmnd_Alias inside any of those files, this was being defined twice and creating an error when using sudo. Signed-off-by: Jordi Massaguer Pla <jmassaguerpla@suse.de>
d62bd1f
to
d580807
Compare
I was missing #787 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jordimassaguerpla Thanks for this. The change looks good, but can you add a unit test or two as well? It would look similar and go in the same place as the one I'm linking here, but ensure that when a line containing the # or @ include is encountered, that it correctly keeps the line rather than appending a new one.
https://github.com/canonical/cloud-init/blob/master/tests/unittests/test_distros/test_generic.py#L109
Signed-off-by: Jordi Massaguer Pla <jmassaguerpla@suse.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the test. I left one small suggestion just to make sure we're hitting both include characters. After that, we should be good to merge!
Co-authored-by: James Falcon <TheRealFalcon@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks!
Newer verisons of /etc/sudoers prefer @includedir over #includedir. Ensure we handle that properly and don't include an additional #includedir when one isn't warranted.
Proposed Commit Message