Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upFix dir perms #784
Merged
Fix dir perms #784
Conversation
Merged
|
Just one note otherwise LGTM! |
internal/exec/util/file.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| return os.Chown(f.Path, uid, gid) | ||
| return os.Chown(node.Path, uid, gid) |
This comment has been minimized.
This comment has been minimized.
jlebon
Apr 4, 2019
Member
Minor: how about just catching and prefixing the error with the path here? Right now we're doing it in the dirEntry path on the calling side, but not in the fileEntry path.
This comment has been minimized.
This comment has been minimized.
ajeddeloh
Apr 4, 2019
Author
Contributor
This comment has been minimized.
This comment has been minimized.
|
Improved error handling and fixed the |
| } | ||
|
|
||
| if err := u.SetPermissions(d.Mode, d.Node); err != nil { | ||
| return fmt.Errorf("error setting permissions of %s: %v", d.Path, err) |
This comment has been minimized.
This comment has been minimized.
jlebon
Apr 4, 2019
Member
We can drop this now and just return u.SetPermissions(...) like in the fileEntry case now, right?
This comment has been minimized.
This comment has been minimized.
If overwrite is false and the config specifies a directory should exist, don't fail if a directory already exists, just change the permissions. If the user wants to ensure the directory is empty, use overwrite=true.
Ignition should not bring up an incorrect system. If there is no restorecon command we should fail hard. This also lets us drop the FileExists() function.
Clarify what Ignition will do with various combinations of files existing, overwrite and (for files) source being unspecified.
Add test that if overwrite=false and a directory already exists, we just fix the permissions and don't fail.
This comment has been minimized.
This comment has been minimized.
|
LGTM! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
ajeddeloh commentedApr 2, 2019
No description provided.