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

consider not modifying the node exporter user if it already exists #183

Closed
bluppfisk opened this issue Aug 4, 2023 · 2 comments · Fixed by #184
Closed

consider not modifying the node exporter user if it already exists #183

bluppfisk opened this issue Aug 4, 2023 · 2 comments · Fixed by #184

Comments

@bluppfisk
Copy link

Using the node_exporter role without fully checking all the steps (mea culpa), I was astonished to find out that it had reduced the user I wanted to run node-exporter as to a no-login, no-home, system user. This then caused other issues with other services running under this user's name and that very much expect to have a home directory of their own.

I think the role should check whether the user already exists and not modify the user if so.

The service file should then also not include ProtectHome=yes as this would effectively disable it from accessing its own home, where, in my case all the metrics prom files are written to.

@SuperQ
Copy link
Contributor

SuperQ commented Aug 4, 2023

No, we want our Ansible roles to be idempotent. We need to maintain all configurations in order for this policy to be maintained.

ProtectHome is a different issue, we can add a check like we do for mounts to not enable ProtectHome if the textfile path contains /home.

SuperQ added a commit that referenced this issue Aug 4, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
@bluppfisk
Copy link
Author

I'll still need to use a fork for my own purposes, but I understand that this may not work for all. The latter part is a sweet and rapid improvement :) cheers

SuperQ added a commit that referenced this issue Aug 4, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 4, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 16, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
gardar pushed a commit that referenced this issue Aug 16, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
gardar pushed a commit that referenced this issue Aug 16, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
gardar pushed a commit that referenced this issue Aug 17, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 21, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 22, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
SuperQ added a commit that referenced this issue Aug 22, 2023
Set the node_exporter `ProtectHome=read-only` when the textfile dir is
in `/home`.

Fixes: #183

Signed-off-by: SuperQ <[email protected]>
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 a pull request may close this issue.

2 participants