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.

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

Allow override built-in fcontext #119

Open
ywei2017 opened this issue Apr 21, 2024 · 5 comments
Open

Allow override built-in fcontext #119

ywei2017 opened this issue Apr 21, 2024 · 5 comments
Labels
Feature Request Enhancement to existing functionality or new functionality

Comments

@ywei2017
Copy link
Contributor

ywei2017 commented Apr 21, 2024

🙍 Problem Statement

The selinux_fcontext::manage/modify does not allow override of built-in contexts. :add action would skip if semanage fcontext -l returns an entry, and :modify would fail if there is no such entry in the .local spec file. Hence there is no way to override a built-in context.

❔ Possible Solution

The most straightforward solution is to check whether the type matches at the conditional statement. Instead of checking "if fcontext is already registered", it should check "if the desired fcontext is already registered". So the conditional check will the same as the :modify action.

⤴️ Describe alternatives you've considered

One possibility is to clone and hack it, but that defeats the purpose of a re-usable cookbook.

➕ Additional context

I can submit a PR if the proposed solution is acceptable.

@ywei2017 ywei2017 added the Feature Request Enhancement to existing functionality or new functionality label Apr 21, 2024
@Stromweld
Copy link
Contributor

A PR will help to better understand the issue and the fix.

@ywei2017
Copy link
Contributor Author

@Stromweld I will submit a PR in the next day or 2. Thanks.

@ywei2017
Copy link
Contributor Author

@Stromweld , please take a look at PR #120. If the approach makes sense, I will update the test cases and the rest for the PR.

Thanks

@Stromweld
Copy link
Contributor

That looks good to me. I'm not very versed in selinux though. I think it'll help to add the test cases for each scenario as well as to make sure future regression isn't introduced.

Would you also be able to open PR for the same thing here https://github.com/chef/chef/blob/main/lib/chef/resource/selinux_fcontext.rb. This resource was based on this cookbooks resource. It'll help chef-client as well as cinc-client since it's based on chef-client.

@ywei2017
Copy link
Contributor Author

@Stromweld Will do. Let me do it in 2 steps.

  1. Add the test case for this PR so it all good. I can use some education to make sure the proper procedures are followed.
  2. Then I will submit a PR to https://github.com/chef/chef/blob/main/lib/chef/resource/selinux_fcontext.rb

Thanks for the quick feedback.

ywei2017 added a commit to ywei2017/selinux that referenced this issue Apr 26, 2024
ywei2017 added a commit to ywei2017/selinux that referenced this issue Apr 26, 2024
Stromweld pushed a commit that referenced this issue Jul 15, 2024
* [issue #119] Update fcontext to allow override of built-in types

---------

Signed-off-by: Yansheng Wei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Enhancement to existing functionality or new functionality
Projects
None yet
Development

No branches or pull requests

2 participants