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

(#1203) Fix lib directory and file permissions #1204

Merged
merged 1 commit into from
Nov 8, 2023
Merged

(#1203) Fix lib directory and file permissions #1204

merged 1 commit into from
Nov 8, 2023

Conversation

ttousai
Copy link

@ttousai ttousai commented Nov 6, 2023 •
edited
Loading

Pull Request (PR) description

This PR fixes an issue where puppet-elasticsearch changes the file mode of files in /usr/share/elasticsearch/lib every time Elasticsearch is upgraded with Apt. This is both unnecessary and undesirable.

From this commit , the intention was to fix the permission on the directory, but it is now changing the file mode of all the files in the directory as well.

From tests it is alright to set the file mode on the directory to 0644 as Puppet does the right thing setting the directory mode to 0755, irrespective of the umask value, while setting the file mode to 0644 as expected.

This Pull Request (PR) fixes the following issues

Fixes #1203

(voxpupuli#1203) Fix lib directory permissions without changing the m…
4a4c739 
…ode of files in the directory

This module changes the file mode of files in /usr/share/elasticsearch/lib everytime
Elasticsearch is upgraded with Apt. This is unnecessary and undesirable.

From this commit voxpupuli@854d902,
the intention was to fix the permission on the directory, but it is now
changing the file mode of all the files in the directory as well.

From tests it is alright to set the file mode on the directory to
0644 as Puppet does the right thing setting the directory mode to 0755,
irrespective of the umask value, while setting the file mode to 0644 as
expected.
@ttousai ttousai mentioned this pull request Nov 6, 2023
Closed
smortex
smortex approved these changes Nov 6, 2023
View reviewed changes
Copy link
Member

@smortex smortex left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ah, setting files mode with recurse => true 🙃…

That's probably fine (if we don't care about the settings being readable by all system users 😨).

@ttousai
Copy link
Author

ttousai commented Nov 7, 2023

The lib directory contains (so far) only jar files so I don't foresee a problem with making that directory readable by all system users.

@ttousai
Copy link
Author

ttousai commented Nov 7, 2023

@smortex it looks like there is a problem with the CI / Puppet /7 - Ubuntu 20.04 (pull_request) check. I don't think it is related to my PR.

@smortex
Copy link
Member

smortex commented Nov 8, 2023 •
edited
Loading

The lib directory contains (so far) only jar files so I don't foresee a problem with making that directory readable by all system users.

I read too fast and though it was the config directory.

@smortex it looks like there is a problem with the CI / Puppet /7 - Ubuntu 20.04 (pull_request) check. I don't think it is related to my PR.

Maybe the CI failure is a transient one, I re-triggered it.

@smortex
Copy link
Member

smortex commented Nov 8, 2023

Maybe the CI failure is a transient one, I re-triggered it.

Looks good now ;-) This feel safe to me.

@smortex smortex merged commit bd5560a into voxpupuli:master Nov 8, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smortex smortex smortex approved these changes

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

Successfully merging this pull request may close these issues.

fix lib directory permissions without changing the mode of files in the directory
2 participants
@ttousai @smortex