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

Prevent log spew when username/password doesn't exist in /etc/passwd #286

Closed
AlexGraberTilton opened this issue Jan 18, 2022 · 4 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@AlexGraberTilton
Copy link
Contributor

AlexGraberTilton commented Jan 18, 2022

Desired behavior

Please stop the spew of "Error getting username: no matching password record.\n"
This occurs in NetUtils.cc

In commit 09723c4, there were changes to username retrieval

In the case of no username in the /etc/passwd, the username is set to "error-" + uuid.ToString()
However the error messages introduced in the change cause log spew when no username exists.
The spew causes the console log to be essentially useless, despite the fact that the system still works fine.

One major use case that this impacts is running with the OSRF gazebo docker containers using --user uid:gid
since /etc/passwd does not have the user in such a scenario, the error message repeats endlessly.

Alternatives considered

  1. add a method to prevent multiple error messages for the same uid
  2. remove the message entirely

Implementation suggestion

remove the error message when getpwuid_r passes but the rule is nullptr
backport to 7+

@AlexGraberTilton AlexGraberTilton added the enhancement New feature or request label Jan 18, 2022
@j-rivero j-rivero self-assigned this Jan 31, 2022
@j-rivero
Copy link
Contributor

j-rivero commented Jan 31, 2022

Reading the issue I think it makes sense not to print the error messages since there are valid use cases that triggered them.

@AlexGraberTilton would you mind sending a PR against ign-transport8 branch? We will review it, merge and merge forward to new versions.

Thanks!

AlexGraberTilton added a commit to AlexGraberTilton/ign-transport that referenced this issue Feb 4, 2022
nkoenig added a commit that referenced this issue Feb 7, 2022
@j-rivero
Copy link
Contributor

j-rivero commented Feb 7, 2022

@AlexGraberTilton would you mind sending a PR against ign-transport8 branch? We will review it, merge and merge forward to new versions.

Thanks so much for the PR, @AlexGraberTilton . The change should be propagated to newer branches when we forward port them, usually every some weeks.

@mjcarroll
Copy link
Contributor

Closed via #291

@osrf-triage
Copy link

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants