Ignore result of EvalSymlinks on ENOENT #23167
Conversation
openshift-ci
bot
added
release-note
approved
|
Marking no new tests, we don't test this logic in CI. We probably should, but dealing with on-restart logic like this is hard in CI. |
libpod/boltdb_state_internal.go
Outdated
| } else { | ||
| // This is ENOENT. | ||
| // We can at least clean the path. | ||
| dbValue = filepath.Clean(dbValue) |
There was a problem hiding this comment.
in what cases would the dbValue path be usable if it doesn't exist?
There was a problem hiding this comment.
The one we're seeing (and probably the only one that makes sense) is volume path. It's a subdirectory of the c/storage root path that is only created the first time the user creates a named or anonymous volume, so it's possible (admittedly difficult, but certainly possible) to use Podman for months without the volume path being created.
|
Ephemeral COPR build failed. @containers/packit-build please check. |
mheon
force-pushed
the
fix_rhel_37948
branch
2 times, most recently
from
c7fc5da to
52b8dec
Compare
|
Cockpit tests failed for commit c7fc5dae487e3d88ebb69d6b718b36e5b24aa12e. @martinpitt, @jelly, @mvollmer please check. |
There was a problem hiding this comment.
Given it is basically duplicated code 4 times could you merge this into a helper function?
I am also very confused about how this is supposed to work, overall this whole logic seem to very brittle? Should we re-evaluate whenever checking this is even worth it compared to the bugs caused by this? (Of course not as part of this PR)
|
Helper function does seem sensible, but we'd have to accept the error message as an argument I think... On whether this is worth it: In actual usage, it seems quite robust (I can recall only one bug where this code hasn't been properly matching paths, resulting in the addition of the EvalSymlinks that caused this problem). It's probably not completely robust against very strange paths, but I don't think we have folks running Podman in strange root directories like that. |
When the path does not exist, filepath.EvalSymlinks returns an empty string - so we can't just ignore ENOENT, we have to discard the result if an ENOENT is returned. Should fix Jira issue RHEL-37948 Signed-off-by: Matt Heon <[email protected]>
|
@giuseppe PTAL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
stale-locking-app
bot
added
the
locked - please file new issue/PR
When the path does not exist, filepath.EvalSymlinks returns an empty string - so we can't just ignore ENOENT, we have to discard the result if an ENOENT is returned.
Should fix Jira issue RHEL-37948
Does this PR introduce a user-facing change?