Respect the UMASK environment variable#1551
Respect the UMASK environment variable#1551AidanDelaney wants to merge 2 commits intobuildpacks:mainfrom
Conversation
AidanDelaney
force-pushed
the
feature/respect-umask
branch
3 times, most recently
from
d730581 to
317922e
Compare
|
|
||
| // SetUmaskWith the injected function umaskFn | ||
| func SetUmaskWith(env Env, umaskFn func(int) int) error { | ||
| if umask := env.Get("UMASK"); umask != "" { |
There was a problem hiding this comment.
Is there precedence in using UMASK env specifically? Should it be CNB_LAUNCH_UMASK?
We may need to have this updated in the spec for documentation.
There was a problem hiding this comment.
I've used UMASK as I though of use-cases where someone might use an environment variables style buildpack to set this in the launch environment. In that use-case they would pack build foo --env BPE_UMASK=0002. I also used UMASK as it it common to most Linux distros, and other Unixes (or shells). However, I have no objection to CNB_LAUNCH_UMASK
|
|
||
| err := SetUmask(l.Env) | ||
| if err != nil { | ||
| return errors.Wrap(err, "umask") |
There was a problem hiding this comment.
Should we fail hard or warn and continue? If we think failing is the right choice I believe we need to guard this entire thing around a platform version check.
There was a problem hiding this comment.
I like the idea of warning and continuing. Will make this modification.
|
I've actually wanted this for the normal CNB lifecycle phases as well during build. We set it manually today, but if we could delegate that to |
When a container is launched with a UMASK value or the UMASK value is set in a launch environment then respect that value Signed-off-by: adelaney21 <adelaney21@bloomberg.net>
Pass a logger down to the process launcher and log when setting the umaks is incorrect and continue. Signed-off-by: adelaney21 <adelaney21@bloomberg.net>
AidanDelaney
force-pushed
the
feature/respect-umask
branch
from
e9bd379 to
93c67dd
Compare
|
@hone @AidanDelaney do you think this requires a spec change? Or is this an implementation detail of our specific implementation of the spec? 🤔 |
|
I don't feel like we're impacting the spec here. But I can run up a quick RFC if you'd like? |
|
If buildpacks/rfcs#321 were a thing, could this then not be a simple buildpack that calls |
|
Yes. I can see that buildpacks/rfcs#321 would allow a |
3 participants
When a container is launched with a UMASK value or the UMASK value is set in a launch environment
then respect that value
Summary
Before we launch a direct or shell process on Unix, set the
umaskto be whatever octal value is in theUMASKenvironment variable.Release notes
When a container is launched with a valid octal string in the UMASK environment variable then that value is used as
umaskin PID 1.Related
Resolves #1550
Context
We no-longer support Windows. However, I've factored this PR out into a
umask_unixandumask_windows. We may wish to cease this practice.