-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Allow configuration of Rootlesskit's CopyUpDirs through an environment variable #10386
base: master
Are you sure you want to change the base?
Conversation
@@ -211,13 +213,24 @@ func createParentOpt(driver portDriver, stateDir string, enableIPv6 bool) (*pare | |||
return opt, nil | |||
} | |||
|
|||
func trimWhiteSpace(slc []string) []string { |
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.
kinda weird to both in-place modify the list, and also return it, but ok.
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.
This should have been part of the standard libraries of Golang, but in the absence of a clear Map function I choose to just include a simple utility method I've found on: https://stackoverflow.com/a/49429437.
This leads me to 2 questions on K3S policy:
- Does K3S have a standard location of such utility methods?
- Can I use this example, or should I rewrite it myself for copyright purposes?
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.
Do we really need this or can we just tell folks to, I don't know, not include unnecessary whitespace in the env var? We don't trim whitespace for any other args or env vars that take a comma-separated list, we just expect people to provide valid input.
…t variable. Signed-off-by: Ludo Stellingwerff <[email protected]>
Co-authored-by: Derek Nola <[email protected]> Signed-off-by: Ludo Stellingwerff <[email protected]> Signed-off-by: Ludo Stellingwerff <[email protected]>
Additional Nit: Now the alignment needs to match. Signed-off-by: Ludo Stellingwerff <[email protected]> Signed-off-by: Ludo Stellingwerff <[email protected]>
FYI: to fix that DCO error, I've followed the advice of the DCO-page and rebased the commits, with proper sign-off statements. |
hasCUP := os.Getenv(copyUpDirsEnv) != "" | ||
if hasCUP { | ||
opt.CopyUpDirs = append(opt.CopyUpDirs, trimWhiteSpace(strings.Split(os.Getenv(copyUpDirsEnv), ","))...) |
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.
hasCUP := os.Getenv(copyUpDirsEnv) != "" | |
if hasCUP { | |
opt.CopyUpDirs = append(opt.CopyUpDirs, trimWhiteSpace(strings.Split(os.Getenv(copyUpDirsEnv), ","))...) | |
if copyUpDirs := os.Getenv(copyUpDirsEnv); copyUpDirs != "" { | |
opt.CopyUpDirs = append(opt.CopyUpDirs, strings.Split(copyUpDirs, ",")...) |
@@ -211,13 +213,24 @@ func createParentOpt(driver portDriver, stateDir string, enableIPv6 bool) (*pare | |||
return opt, nil | |||
} | |||
|
|||
func trimWhiteSpace(slc []string) []string { |
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.
Do we really need this or can we just tell folks to, I don't know, not include unnecessary whitespace in the env var? We don't trim whitespace for any other args or env vars that take a comma-separated list, we just expect people to provide valid input.
Proposed Changes
In our use-case we are running an K3S Agent in a Rootless environment at the end-user's own workstation. This works well (enough), but we also want to provide access to the local folders (through e.g. HostPath volume mounts). But the Agent runs inside the rootless namespace, disallowing read/write access to the actual file-system.
This PR allows the Agent to setup extra "CopyUpDirs" entries, through an environment variable called "K3S_ROOTLESS_COPYUPDIRS". This environment variable can receive a comma-separated list absolute paths, which will be added to the existing list.
Types of Changes
This change is non-breaking, fully backwards compatible. It only adds a new feature and a new environment variable. The setup will not start (with a fatal error) if any of the added folders doesn't exist in the host system. Which is the normal behavior of the pre-existing code.
Verification
Testing
No, I currently have no unit test for this change.
Linked Issues
See #10385 for a further description.
User-Facing Change
Further Comments