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

feat: Add the sudo role #2

Merged
merged 17 commits into from
May 21, 2024
Merged

feat: Add the sudo role #2

merged 17 commits into from
May 21, 2024

Conversation

radosroka
Copy link
Collaborator

@radosroka radosroka commented May 16, 2024 •
edited by spetrosi
Loading

Enhancement: Add the sudo system role to manage sudo and generate sudoers files.

@radosroka radosroka requested review from spetrosi and richm as code owners May 16, 2024 12:16
spetrosi
spetrosi reviewed May 16, 2024
View reviewed changes
templates/sudoers.j2 Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
templates/sudoers.j2 Outdated Show resolved Hide resolved
@radosroka radosroka force-pushed the code branch 3 times, most recently from 92c3c77 to dcff139 Compare May 16, 2024 17:40
@spetrosi
Copy link
Contributor

[citest]

@radosroka radosroka force-pushed the code branch 2 times, most recently from b0d5a20 to 94d998d Compare May 16, 2024 18:55
@radosroka
Copy link
Collaborator Author

[citest]

@radosroka
Copy link
Collaborator Author

[citest]

1 similar comment
@spetrosi
Copy link
Contributor

[citest]

@radosroka radosroka force-pushed the code branch 11 times, most recently from 44476b7 to 651c4b9 Compare May 17, 2024 09:25
@radosroka
Copy link
Collaborator Author

[citest]

@radosroka
feat: import code
6ec3d6a 
Signed-off-by: Radovan Sroka <[email protected]>
@radosroka
Copy link
Collaborator Author

[citest]

@radosroka
Copy link
Collaborator Author

[citest]

@radosroka
Copy link
Collaborator Author

[citest]

spetrosi
spetrosi reviewed May 17, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
defaults/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
tasks/main.yml Show resolved Hide resolved
tasks/main.yml Outdated Show resolved Hide resolved
tests/files/tests_multiple_sudoers_pingers.ok Outdated Show resolved Hide resolved
tests/tests_multiple_sudoers.yml Show resolved Hide resolved
@radosroka radosroka force-pushed the code branch 3 times, most recently from a9ed55b to 5c902c9 Compare May 20, 2024 07:51
@radosroka
Fix other review suggestions
2fd26b1 
Signed-off-by: Radovan Sroka <[email protected]>
@radosroka
Copy link
Collaborator Author

[citest]

@radosroka
Copy link
Collaborator Author

[citest]

1 similar comment
@spetrosi
Copy link
Contributor

[citest]

spetrosi added 5 commits May 20, 2024 14:45
@spetrosi
Clarify and structurize README
74f7459
@spetrosi
Add default_overrides to simplify user_specifications
7539fb6 
user_specifications included standart specifications and default
overrides.
This commit takes away default_overrides to a separate variable.

Fix ansible-lint and tests
@spetrosi
Add tests/tasks/assert_files_identical.yml
c3bff30
@spetrosi
Remove info about ansible-sudoers from README
0bf1251
@spetrosi
Return and fix ansible-sudoers reference
8ec9ac1
@spetrosi
Copy link
Contributor

[citest]

@spetrosi
Copy link
Contributor

spetrosi commented May 21, 2024 •
edited
Loading

I will merge this to get it into a build sooner. Tests are passing for me locally against RHEL 8 and 9.

Here is what I think the role lacks so far and needs future work:

  1. Add validation for sudo_sudoers_files. Now, if it is not provided correctly, the role fails at the template generation step with weird errors.
  2. Some variables are not tested: sudo_visudo_path, sudoers: include_files, sudoers: user_specifications: selinux_*, tags.
  3. Make example playbooks in README more distinguished, so that each example serves some scenario and they do not repeat each other.
  4. Check header and role fingerprint in the generated sudoers.
  5. Fix examples in examples/ to use default_overrides, I forgot to do it.
  6. Ensure all tests pass in the upstream CI. Currently, there is an outage in BaseOS Jenkins.

@radosroka do you have something else to add to this todo list? Does the role currenlty cover all requested functionality?

@spetrosi
Copy link
Contributor

From RHELBU-2348, I see that the role currently lacks the following:

  1. Role should support backing up sudoers file(s) before changing them
  2. Ability to gather the current sudoers configuration from an existing system and return it as an Ansible variable. There is a module for this in https://github.com/ahuffman/ansible-role-scan-sudoers

@spetrosi spetrosi changed the title feat: Add initial version of the sudo role feat: Add the sudo role May 21, 2024
@spetrosi spetrosi merged commit d0bbb87 into linux-system-roles:main May 21, 2024
9 checks passed
@spetrosi
Copy link
Contributor

[citest]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spetrosi spetrosi spetrosi approved these changes

@richm richm Awaiting requested review from richm richm is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@radosroka @spetrosi