image: avoid panic on nil fields when converting #157
Conversation
zhouhao3
force-pushed
the
image-test-fix
branch
2 times, most recently
from
b7b5597 to
7ca85c1
Compare
|
@opencontainers/image-tools-maintainers PTAL |
zhouhao3
force-pushed
the
image-test-fix
branch
2 times, most recently
from
18b4105 to
1747903
Compare
zhouhao3
changed the title
|
What was the error? Can you include it in the commit message that fixes said error? |
|
The following error occurs when the
Because there is no non-empty check on the optional fields in config.go. |
image/config.go
Outdated
| if err != nil { | ||
| return nil, errors.New("config.User: unsupported gid format") | ||
| if len(s.Process.Args) == 0 { | ||
| s.Process.Args = append(s.Process.Args, "sh") |
There was a problem hiding this comment.
Do we make the assumption that sh is always available in a container?
There was a problem hiding this comment.
Yes, Args has at least one value.
There was a problem hiding this comment.
That is not what I asked.
Not all containers are going to have a sh binary present. Adding this, when it might not even exist, will certainly result in failure. This is also never mentioned in the specification. In the case that no args are provided, it should just not be set.
There was a problem hiding this comment.
Fixing this is outside the scope of this PR. I've filed #166.
There was a problem hiding this comment.
@stevvooe If there are no other questions, can we merge this patch first and then discuss the sh problem? Because if you don't merge this, the create function won't work.
|
A good title for this PR and the commit might be "image: avoid panic on nil fields when converting". Also, please take the time to squash this into a single commit. |
|
updated, PTAL. |
zhouhao3
changed the title
image/config.go
Outdated
| s.Process.Cwd = "/" | ||
| if c.Config.WorkingDir != "" { | ||
| s.Process.Cwd = c.Config.WorkingDir | ||
| if s.Root != nil { |
There was a problem hiding this comment.
I think this is not a right fix. Default Root is nil in Spec, we should initialize it as s.Root= &specs.Root{}, also same to Process.
The following error occurs when the create command is executed：
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x48ef43]
goroutine 19 [running]:
panic(0x808060, 0xc420010130)
/home/zhouhao/go/src/runtime/panic.go:500 +0x1ae
testing.tRunner.func1(0xc4200932c0)
/home/zhouhao/go/src/testing/testing.go:579 +0x474
panic(0x808060, 0xc420010130)
/home/zhouhao/go/src/runtime/panic.go:458 +0x271
...
Because there is no non-empty check on the optional fields in config.go.
Signed-off-by: zhouhao <[email protected]>
zhouhao3
force-pushed
the
image-test-fix
branch
2 times, most recently
from
dd70fea to
bb4fdfe
Compare
|
LGTM |
1 similar comment
|
LGTM |
5 participants
When I execute the create command, there was an error, but there was no error when making the test. So need to improve the test content, and fix the error.
Signed-off-by: zhouhao [email protected]