Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixes #14517: Add a generic method to remove posix ACL #940

Merged
merged 1 commit into from
May 23, 2019
Merged

Fixes #14517: Add a generic method to remove posix ACL #940

merged 1 commit into from
May 23, 2019

Conversation

Fdall
Copy link
Contributor

@Fdall Fdall commented Mar 22, 2019

ncharles
ncharles reviewed Mar 22, 2019
View reviewed changes
tree/30_generic_methods/permissions_posix_acls_absent.cf Outdated
@@ -0,0 +1,184 @@
#####################################################################################
# Copyright 2013 Normation SAS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2019, isn't it ?

ncharles
ncharles reviewed Mar 22, 2019
View reviewed changes
tree/30_generic_methods/permissions_posix_acls_absent.cf Outdated
{
vars:
"old_class_prefix" string => canonify("permissions_posix_acls_absent_${path}");
"args" slist => { "${path}" };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are missing "recursive" as an arg

ncharles
ncharles reviewed Mar 22, 2019
View reviewed changes
tree/30_generic_methods/permissions_posix_acls_absent.cf
"full_class_prefix" string => canonify("permissions_posix_acls_absent_${report_param}");
"class_prefix" string => string_head("${full_class_prefix}", "1000");

"wc_path" string => "/usr/bin/wc";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should go in the paths.cf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still relevant

@Fdall
Copy link
Contributor Author

Fdall commented Mar 22, 2019

Commit modified

ncharles
ncharles reviewed Mar 22, 2019
View reviewed changes
tree/30_generic_methods/permissions_posix_acls_absent.cf Outdated
"error_list" slist => getvalues("error");
"success_list" slist => getvalues("success");

"report_list" slist => { "@{error_list}", "@{repaired_list}", "@{success_list}" };
Copy link
Member

@ncharles ncharles Mar 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it needs the policy => "ifdefined" also

@amousset
Copy link
Member

This methods needs tests, specially as we implemented it and did not reuse a CFEngine promise here.

@Fdall
Copy link
Contributor Author

Fdall commented Mar 28, 2019

Commit modified

4 similar comments
@Fdall
Copy link
Contributor Author

Fdall commented Mar 28, 2019

Commit modified

@Fdall
Copy link
Contributor Author

Fdall commented May 3, 2019

Commit modified

@Fdall
Copy link
Contributor Author

Fdall commented May 15, 2019

Commit modified

@Fdall
Copy link
Contributor Author

Fdall commented May 23, 2019

Commit modified

@Fdall
Copy link
Contributor Author

Fdall commented May 23, 2019

PR rebased

@Fdall Fdall force-pushed the bug_14517/add_a_generic_method_to_remove_posix_acl branch from ab80991 to 0681d7e Compare May 23, 2019 12:04
@Fdall
Copy link
Contributor Author

Fdall commented May 23, 2019

Commit modified

ncharles
ncharles reviewed May 23, 2019
View reviewed changes
tree/30_generic_methods/permissions_posix_acls_absent.cf Outdated
"class_prefix" string => string_head("${full_class_prefix}", "1000");

"wc_path" string => "/usr/bin/wc";
"detect_acl_command" string => "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unused

ncharles
ncharles reviewed May 23, 2019
View reviewed changes
tree/30_generic_methods/permissions_posix_acls_absent.cf Outdated
"success_${file_list}" usebundle => _classes_success("${inner_class_prefix[${file_list}]}"),
ifvarclass => "!acls_found_${canonified_file_array[${file_list}]}.path_exists_${canonified_file_array[${file_list}]}";

# Execute setfacl -b
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: executed

@Fdall
Copy link
Contributor Author

Fdall commented May 23, 2019

Commit modified

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/ncf/pull/940
-- Your faithful QA
Kant merge: "Live your life as though your every act were to become a universal law."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/10476/console)

@Fdall
Copy link
Contributor Author

Fdall commented May 23, 2019

OK, squash merging this PR

@Fdall Fdall force-pushed the bug_14517/add_a_generic_method_to_remove_posix_acl branch from 1c96cc6 to 13ab3cb Compare May 23, 2019 15:15
@Fdall Fdall merged commit 13ab3cb into Normation:branches/rudder/5.0 May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncharles ncharles ncharles approved these changes

Assignees
No one assigned
Labels
qa: Can't merge
Projects
None yet
Milestone
No milestone
4 participants
@Fdall @amousset @Normation-Quality-Assistant @ncharles