Fixes #14514: Add a generic method to add ACLs on a given file #939
Conversation
| @@ -0,0 +1,192 @@ | |||
| ##################################################################################### | |||
| # Copyright 2013 Normation SAS | |||
|
|
||
| } | ||
|
|
||
| body acl add_posix_acl(user_acls, group_acls, other_acls) |
There was a problem hiding this comment.
This should got into the library.
| # @parameter_constraint group "regex" : "^$|^(([A-z0-9._-]+|\\*):[+-=]r?w?x?,? *)+$" | ||
| # @parameter_constraint other "regex" : "^$|^[+-=^]r?w?x?$" | ||
| # | ||
| # @class_prefix permissions_add_acl |
There was a problem hiding this comment.
We need to chose between set and add (name vs. class/bundle name).
Will there be another method to overwrite ACLs?
| # @description Verify that an ace is present on a file or directory. | ||
| # This method will append the given aces to the current POSIX ACLs of | ||
| # the target. | ||
| # @documentation The `permissions_*acl|ace_*` manage the POSIX ACL on files and directorieS. |
There was a problem hiding this comment.
Could you add an example section with the resulting getfacl output?
| # It can also be left blank to let the `Other` ACE unchanged. | ||
| # | ||
| # @parameter path Path of the file or directory | ||
| # @parameter recursive Recursive Should ACLs cleanup be recursive, "true" or "false" (defaults to "false") |
There was a problem hiding this comment.
we should have
# @parameter_constraint recursive "select" : [ "", "true", "false" ]
|
Commit modified |
|
Fixed to match the review, also found a bug when there were no report when no files at all could be determined from the entry path |
| "report" usebundle => _log_v3("No files could be found matching ${path}, add POSIX acl user:${user}, group:${group} and other:${other}", "${path}", "${old_class_prefix}", "${class_prefix}", @{args}); | ||
|
|
||
| empty_acls.files_found:: | ||
| "failure_${file_list}" usebundle => _classes_success("${inner_class_prefix[${file_list}]}"), |
There was a problem hiding this comment.
i'm quite confused there - should it be success or failure ? (the left hand side says failure, the right one says success)
There was a problem hiding this comment.
from the doc, it should be success (nothing to append), but i'd like it to be clear there
| # | ||
| # ~~~~ | ||
| # | ||
| # This method can not remove a given ACE, see here how the vagrant ACE is handled. |
| # * * matches any filename or directory at one level, e.g. *.cf will match all files in one directory that end in .cf but it won't search across directories. */*.cf on the other hand will look two levels deep. | ||
| # * ? matches a single letter | ||
| # * [a-z] matches any letter from a to z | ||
| # * {x,y,anything} will match x or y or anything. |
There was a problem hiding this comment.
`{x,y,anything}` (and also for others, it will be easier to read
|
Commit modified |
4 similar comments
|
Commit modified |
|
Commit modified |
|
Commit modified |
|
Commit modified |
Fdall
force-pushed
the
ust_14514/add_a_generic_method_to_add_aces_on_a_given_file
branch
from
750b372
to
129c8b7
Compare
|
PR rebased |
|
Commit modified |
|
This PR is not mergeable to upper versions. |
|
OK, squash merging this PR |
Fdall
force-pushed
the
ust_14514/add_a_generic_method_to_add_aces_on_a_given_file
branch
from
92ebf25
to
ab30440
Compare
|
OK, merging this PR |
https://issues.rudder.io/issues/14514