Add recursive chmod/chown to remote_directory resource. #218
Conversation
remote_directory resource with mode/owner/group attributes just only
work at root directory("path" attribute).
This patch add recursively option. And set default(it is good or bad?).
Test case
---------
remote_directory '/var/www/html/webapps' do
owner 'apache'
group 'apache'
mode 'g+w'
source webapps
end
| @@ -9,6 +9,8 @@ class RemoteDirectory < Base | |||
| define_attribute :mode, type: String | |||
| define_attribute :owner, type: String | |||
| define_attribute :group, type: String | |||
| define_attribute :recursive_mode, type: [String, Symbol], default: :true | |||
There was a problem hiding this comment.
I think setting false as the default value is better.
There was a problem hiding this comment.
I think better way of recursive chown/chmod in default in my local usage.
But, this patch changes behavior from current version. It is not good.
I change to boolean false in default.
There was a problem hiding this comment.
I just change to default on false.
Sorry too late.
| @@ -9,6 +9,8 @@ class RemoteDirectory < Base | |||
| define_attribute :mode, type: String | |||
| define_attribute :owner, type: String | |||
| define_attribute :group, type: String | |||
| define_attribute :recursive_mode, type: [String, Symbol], default: :true | |||
There was a problem hiding this comment.
Could you use true instead of symbol :true?
| @@ -56,10 +58,18 @@ def show_differences | |||
|
|
|||
| def action_create(options) | |||
| if attributes.mode | |||
| run_specinfra(:change_file_mode, @temppath, attributes.mode) | |||
| if attributes.recursive_mode == :true | |||
| run_specinfra(:change_file_mode, @temppath, attributes.mode, :recursive => true) | |||
There was a problem hiding this comment.
If attributes.recursive_mode is boolean, you can simply write this run_specinfra(:change_file_mode, @temppath, attributes.mode, recursive: attributes.recursive_mode)
There was a problem hiding this comment.
Thanks to your suggestion!
Replace to your code and testing now.
* Change default value: false on default.
| @@ -9,6 +9,8 @@ class RemoteDirectory < Base | |||
| define_attribute :mode, type: String | |||
| define_attribute :owner, type: String | |||
| define_attribute :group, type: String | |||
| define_attribute :recursive_mode, type: [TrueClass, FalseClass], default: false | |||
| define_attribute :recursive_owner, type: [TrueClass, FalseClass], default: false | |||
There was a problem hiding this comment.
From the name of recursive_owner, users may not expect that group will be also changed recursively.
How about change the name to recursive_owner_and_group?
There was a problem hiding this comment.
I just mistake with commit log.
"#218: Change attribute name s/recursive_onwer/recursive_owner_and_group/"
But commented-out :-(
In my opinion. "recursive_owner_and_group" is too long.
Do you want change this attribute to "recursive" like chef / ansible?
remote_directory resource with mode/owner/group attributes just only
work at root directory("path" attribute).
This patch add recursively option. And set default(it is good or bad?).
Test case