Skip to content

fix: node_exporter preflight checks ssl key as ansible user - #821

Open
epruesse wants to merge 1 commit into
prometheus-community:mainfrom
epruesse:patch-1
Open

fix: node_exporter preflight checks ssl key as ansible user#821
epruesse wants to merge 1 commit into
prometheus-community:mainfrom
epruesse:patch-1

Conversation

@epruesse

Copy link
Copy Markdown

The node_exporter TLS certificate private key file may not be readable by the ansible user. The current preflight fails if this is the case. The PR makes the stat call run as the node_exporter_system_user.

Run stat on tls key file as node_exporter_system_user since this file may not be accessible to the regular ansible user. 

Signed-off-by: Elmar Pruesse <epruesse@users.noreply.github.com>
@epruesse

Copy link
Copy Markdown
Author

I'm not sure why the tests are failing. Is this expected, or an issue with the test system?

@gardar

gardar commented Jul 10, 2026

Copy link
Copy Markdown
Member

I'm not sure why the tests are failing. Is this expected, or an issue with the test system?

The tests are failing because the service user does not exist yet. The user is created during the installation step of the role, which runs after the preflight checks. The reason this only shows up in the alternative test scenario is that TLS support is enabled there, while the default and latest scenarios do not hit this check.

That said, I think the general idea is a good one. We should verify that the service user can actually access the certificate and key, and ideally this is something we should check consistently across all roles that support TLS.

Maybe the simplest fix here would be to run the certificate/key stat checks as root with become: true, rather than as the service user. Then we can check not only that the files exist, which we already do, but also that their ownership and permissions allow the service user to use them.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants