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

osutils: deal with ENOENT in UserMaybeSudoUser() #11275

Merged
merged 2 commits into from
Jan 24, 2022

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Jan 19, 2022

When the sssd package is installed on a Ubuntu system the
TestUserMaybeSudoUser test will fail because then user.Lookup
returns a generic error

user: lookup username guy: no such file or directory

because in the go layer the getpw{uid,nam}_r() returned ENOENT.

This is already reported upstream as
golang/go#40334

This commit works around this upstream issue by checking for
ENOENT on the error from user.Lookup(). It's not nice but the
best I could come up to to handle this case. On my 21.10 system
sssd is installed as part of ubuntu-desktop-minimal.

When the `sssd` package is installed on a Ubuntu system the
`TestUserMaybeSudoUser` test will fail because then `user.Lookup`
returns a generic error
```
user: lookup username guy: no such file or directory
```
because in the go layer the getpw{uid,nam}_r() returned ENOENT.

This is already reported upstream as
golang/go#40334

This commit works around this upstream issue by checking for
ENOENT on the error from user.Lookup(). It's not nice but the
best I could come up to to handle this case. On my 21.10 system
sssd is installed as part of `ubuntu-desktop-minimal`.
@codecov-commenter
Copy link

Codecov Report

Merging #11275 (8bf80f8) into master (8a25942) will decrease coverage by 0.00%.
The diff coverage is 56.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11275      +/-   ##
==========================================
- Coverage   78.36%   78.35%   -0.01%     
==========================================
  Files         927      927              
  Lines      105700   105710      +10     
==========================================
- Hits        82832    82830       -2     
- Misses      17719    17728       +9     
- Partials     5149     5152       +3     
Flag Coverage Δ
unittests 78.35% <56.25%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
osutil/user.go 83.17% <56.25%> (-2.69%) ⬇️
osutil/synctree.go 76.41% <0.00%> (-2.84%) ⬇️
store/cache.go 69.23% <0.00%> (-1.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a25942...8bf80f8. Read the comment docs.

Copy link
Collaborator

@MiguelPires MiguelPires left a comment

Choose a reason for hiding this comment

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

There was a unit test failure FAIL: runner_test.go:1498: runnerSuite.TestRepairModesAndBases, but it seems unrelated to the PR

osutil/user.go Outdated Show resolved Hide resolved
osutil/user.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@MiguelPires MiguelPires left a comment

Choose a reason for hiding this comment

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

LGTM

@mvo5 mvo5 added Squash-merge Please squash this PR when merging. and removed Squash-merge Please squash this PR when merging. labels Jan 24, 2022
@mvo5 mvo5 merged commit b32d0fc into canonical:master Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants