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
Add support for env var secret sources #3598
Conversation
fd2c84d
to
5c3128c
Compare
|
||
for _, path := range artifacts.TmpFiles { | ||
err := os.Remove(path) | ||
if !os.IsNotExist(err) { |
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.
Could you save the err in a prevErr field and only return after you delete all TmpFiles.
var prevErr error
for _, path := range artifacts.TmpFiles {
err := os.Remove(path)
if !os.IsNotExist(err) {
if prevErr != nil {
logrus.Error(err)
}
prevErr = err
}
}
return prevErr
switch secr.SourceType { | ||
case "env": | ||
data = []byte(os.Getenv(secr.Source)) | ||
tmpFile, err := ioutil.TempFile("/dev/shm", "buildah*") |
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.
Make sure the buildah-* files are not world readable.
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.
Would this just be a chmod or something else?
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.
The TempFile()
implementation specifies permissions of 0600
when creating the file, so that's fine.
Tests are very unhappy @ashley-cui |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashley-cui The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
run_linux.go
Outdated
err := os.Remove(path) | ||
if !os.IsNotExist(err) { | ||
if prevErr != nil { | ||
logrus.Error(err) |
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 be logrus.Error(prevErr)
Run secrets can now be created from an environment variable. The environment variable is read and is briefly stored as a file on /dev/shm when it's being used, and the file is removed after the RUN command is finished. Fixes: containers#3524 Signed-off-by: Ashley Cui <acui@redhat.com>
/lgtm |
Run secrets can now be created from an environment variable. The
environment variable is read and is briefly stored as a file on /dev/shm
when it's being used, and the file is removed after the RUN command is
finished.
Fixes: #3524
Signed-off-by: Ashley Cui acui@redhat.com
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?