Feature/74 file owner #549
Conversation
rebeccaskinner
force-pushed
the
feature/74-file-owner
branch
from
a75f932
to
ea3a69c
Compare
rebeccaskinner
force-pushed
the
feature/74-file-owner
branch
from
ea3a69c
to
ef4313f
Compare
| "golang.org/x/net/context" | ||
| ) | ||
|
|
||
| // Preparer for Owern |
|
|
||
| // Recursive indicates whether ownership changes should be applied | ||
| // recursively. Symlinks are not followed. | ||
| Recursive bool `hcl:"recursive" default:"false" export:"recursive"` |
There was a problem hiding this comment.
we do not have a default tag
| UID *int `hcl:"uid" mutually_exclusive:"user" export:"uid"` | ||
|
|
||
| // Groupname specifies group-ownership by groupname | ||
| Groupname string `hcl:"group" mutually_exclusive:"gid" export:"gid"` |
There was a problem hiding this comment.
You should change the mutually_exclusive tags for Groupname and GID to contain both of the fields:
mutually_exclusive:"group,gid"
| Groupname string `hcl:"group" mutually_exclusive:"gid" export:"gid"` | ||
|
|
||
| // GID specifies group ownership by gid | ||
| GID *int `hcl:"gid" mutually_exclusive:"group" export:"group"` |
There was a problem hiding this comment.
Probably should remove the export tags in Preparer
| @@ -0,0 +1,21 @@ | |||
| /* This file demonstrates proper usage of the file.owner module by creating a | |||
There was a problem hiding this comment.
your sample has an error:
root/file.owner.owner-test:
Error: stat to-change: no such file or directory
Messages:
Has Changes: no
Changes: No changes
| } | ||
| w := &fileWalker{Status: status, NewOwner: newOwner, Executor: o.executor} | ||
| if o.Recursive { | ||
| status.AddMessage("Recursively updating permissions in " + o.Destination) |
There was a problem hiding this comment.
You should check whether destination is a directory
| if idx > 1 { | ||
| return "Error executing diff generation on " + d.Path | ||
| } | ||
| if nil != d.UIDs && d.UIDs[0] != d.UIDs[1] { |
There was a problem hiding this comment.
There are a few if nil !=s in this file
| func (o *Owner) Apply(context.Context) (resource.TaskStatus, error) { | ||
| showDetails := !(o.Recursive && o.HideDetails) | ||
| status := resource.NewStatus() | ||
| if o.Recursive { |
There was a problem hiding this comment.
and if destination is not a directory?
| return os.Chown(path, uid, gid) | ||
| } | ||
|
|
||
| func osStat(path string) (*syscall.Stat_t, error) { |
There was a problem hiding this comment.
maybe move this to the end of the file?
| t.Parallel() | ||
| assert.Implements(t, (*resource.Task)(nil), new(owner.Owner)) | ||
| } | ||
|
|
There was a problem hiding this comment.
maybe add some tests to check/apply when destination is and is not a directory with recursive = true
| showDetails := !(o.Recursive && o.HideDetails) | ||
|
|
||
| if _, err := os.Stat(o.Destination); os.IsNotExist(err) { | ||
| return nil, fmt.Errorf("cannot change ownership of non-existant file: %s", o.Destination) |
There was a problem hiding this comment.
correct spelling is existent
rebeccaskinner
force-pushed
the
feature/74-file-owner
branch
from
8394c86
to
323d025
Compare
fixes #74