-
Notifications
You must be signed in to change notification settings - Fork 550
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
ImageCustomizer: Add Support for Configuring User Sudoers File #11196
base: 3.0-dev
Are you sure you want to change the base?
Conversation
return nil | ||
} | ||
|
||
func UpdateSudoAccess(installChroot safechroot.ChrootInterface, username string, enableSudo bool) error { |
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.
Was trying to put this function inside userutils.py but will create cyclic dependency issue.
@@ -20,6 +20,7 @@ type User struct { | |||
SecondaryGroups []string `yaml:"secondaryGroups"` | |||
StartupCommand string `yaml:"startupCommand"` | |||
HomeDirectory string `yaml:"homeDirectory"` | |||
Sudo bool `yaml:"sudo"` |
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.
Is this setting specifically enabling no-password sudo? If that's the case, then could we make the setting name appropriately specific to that? Given that this is a security-impacting setting, I think we should be as clear as we can about what the image owner is opening up.
}, | ||
} | ||
|
||
err := CopyAdditionalFiles(sudoersFile, additionalFiles, installChroot) |
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.
You don't need to use CopyAdditionalFiles
. Just write to the file using file.Write. (You can use installChroot .RootDir()
to get the rootfs directory.)
@@ -17,8 +17,8 @@ const ( | |||
defaultFilePermissions = 0o755 | |||
) | |||
|
|||
func copyAdditionalFiles(baseConfigPath string, additionalFiles imagecustomizerapi.AdditionalFileList, | |||
imageChroot *safechroot.Chroot, | |||
func CopyAdditionalFiles(baseConfigPath string, additionalFiles imagecustomizerapi.AdditionalFileList, |
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.
You don't need to make a function public if you are accessing it from the same package/directory.
Merge Checklist
All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)
Summary
What does the PR accomplish, why was it needed?
Change Log
Does this affect the toolchain?
NO
Associated issues
Test Methodology