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 multiple file modes in the FileChecker #255

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

rcritten
Copy link
Collaborator

@rcritten rcritten commented Apr 5, 2022

There are some cases where a strict file mode is not
necessary. The kadmind.log is one.

It is owned root:root so there is no real difference
between 0600 and 0640. So allow both.

https://bugzilla.redhat.com/show_bug.cgi?id=2058239

Signed-off-by: Rob Crittenden rcritten@redhat.com

@rcritten
Copy link
Collaborator Author

rcritten commented Apr 5, 2022

Fixed lint issue: use-a-generator

Copy link
Contributor

@flo-renaud flo-renaud left a comment

Choose a reason for hiding this comment

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

@rcritten thanks for the PR, LGTM.
I just have one comment re. the tests: the PR adds a new file to test (with a tuple of possible modes), but there's no assert related to this new file. I would expect something like

assert my_results.results[4].result == ...
assert.my_results.results[4].kw.get('msg') == ...

in the test_files_mode method.

@flo-renaud flo-renaud self-assigned this Apr 6, 2022
There are some cases where a strict file mode is not
necessary. The kadmind.log is one.

It is owned root:root so there is no real difference
between 0600 and 0640. So allow both.

https://bugzilla.redhat.com/show_bug.cgi?id=2058239

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Copy link
Contributor

@flo-renaud flo-renaud left a comment

Choose a reason for hiding this comment

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

@rcritten thanks for the update, perfect!

0640 = 33184
0644 = 33188
0660 = 33200
0666 = 33206
Copy link
Contributor

Choose a reason for hiding this comment

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

You have no idea how thankful I am for these lines! I'm sure I would have forgotten how to map things over the night :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:-)

thanks for the review.

@rcritten rcritten merged commit 621a32b into freeipa:master Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants