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

Test logfile permissions (RhBug:1910084) #940

Merged
merged 2 commits into from
Mar 3, 2021

Conversation

pkratoch
Copy link
Contributor

@kontura
Copy link
Contributor

kontura commented Feb 2, 2021

I have just one suggestion, it would make sense to me to test that rotating the logs also respects umask. I think this could be achieved by setting dnf conf log_size to some really small value so that it rotates right away for the first command, maybe 1k?

Other than that LGTM. 🙂

@kontura kontura self-assigned this Feb 2, 2021
@pkratoch
Copy link
Contributor Author

pkratoch commented Mar 2, 2021

Thanks for the review and the suggestion of new test!

There was actually another bug - the new dnf.rpm.log was sometimes created with mode 644 because rpm is setting umask during a transaction run.

I think that the best behavior is not to respect umask during the log rotation, but keep the same mode as the old log. This also means that user can change permissions of the base log file and it will be respected further on.

So, here is the dnf PR: rpm-software-management/dnf#1736

And I added test that log rotation preserves the log file permissions.

@kontura
Copy link
Contributor

kontura commented Mar 3, 2021

That is great, thanks!

Tests pass.

@kontura kontura merged commit 1e5dcbb into rpm-software-management:master Mar 3, 2021
@pkratoch pkratoch deleted the 1910084 branch March 10, 2021 08:44
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.

2 participants