Support getting uid/gid from rootfs path #2009
Conversation
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
oci/spec_opts_unix.go
Outdated
| return errors.Errorf("rootfs snapshot not created for container") | ||
| if c.Snapshotter == "" && c.SnapshotKey == "" { | ||
| if s.Root.Path == "" { | ||
| if c.Snapshotter == "" { |
There was a problem hiding this comment.
This if (and the following one) seem redundant because the previous line (260) already checked this condition. I think this could error immediately with "no shapshotter and rootfs set for container".
Or is 260 supposed to be || ? With && it means that if one of them is the empty string the "else" block below could fail in an unexpected way.
There was a problem hiding this comment.
These errors are only returned when all 3 are empty. And there are separate error messages for each missing field. I think i retained that same logic with the change.
There was a problem hiding this comment.
The second missing field block (line 265) would never be hit, it's dead code, and there are unhandled error cases:
c.Snapshotter |
c.SnapshotKey |
s.Root.Path |
result |
|---|---|---|---|
| "" | "" | "" | errors.Errorf("no snapshotter set for container") |
| "overlay" | "" | n/a | (goes to line 282 and may error because snapshotkey is "") |
| "" | "foo" | n/a | (goes to line 282 and may error because Snapshotter is "") |
| "overlay" | "foo" | n/a | (goes to line 282 and shouldn't error) |
| "" | "" | /something |
(goes to line 269 and shouldn't error) |
right?
There was a problem hiding this comment.
I think you are reading it wrong on your second line in the table. It won't go to 282, it goes to 265 and errors.
There was a problem hiding this comment.
same for the third line
oci/spec_opts_unix.go
Outdated
| return errors.Errorf("rootfs snapshot not created for container") | ||
| if c.Snapshotter == "" && c.SnapshotKey == "" { | ||
| if s.Root.Path == "" { | ||
| if c.Snapshotter == "" { |
crosbymichael
force-pushed
the
user-path
branch
3 times, most recently
from
4a915f8
to
aa6804a
Compare
oci/spec_opts_unix.go
Outdated
| } | ||
|
|
||
| func isRootfsAbs(root string) bool { | ||
| if root != "" { |
There was a problem hiding this comment.
nit: is this required? filepath.IsAbs("") seems to return false on linux, but maybe it's not guaranteed to be on all platforms?
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
crosbymichael
force-pushed
the
user-path
branch
from
aa6804a
to
1f5ce14
Compare
Codecov Report
@@ Coverage Diff @@
## master #2009 +/- ##
=========================================
- Coverage 45.57% 45.48% -0.1%
=========================================
Files 94 94
Lines 9296 9315 +19
=========================================
Hits 4237 4237
- Misses 4351 4370 +19
Partials 708 708
Continue to review full report at Codecov.
|
This allows the spec opts for getting the uid/gid from
/etc/passwdfrom either a container with a snapshot or with a hardcoded rootfs path.Closes #1995