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.

Sign up for GitHub

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

Jump to bottom

Panic building for multiple explicit platforms #544

Closed
imjasonh opened this issue Dec 14, 2021 · 4 comments · Fixed by #546
Closed

Panic building for multiple explicit platforms #544

imjasonh opened this issue Dec 14, 2021 · 4 comments · Fixed by #546

Comments

@imjasonh
Copy link
Member

Running from HEAD at 2ba70fc:

$ go run ./ build ./ --platform=linux/amd64,linux/arm64
...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x14a1c1f]

goroutine 93 [running]:
github.com/google/go-containerregistry/pkg/v1/partial.Descriptor({0x0, 0x0})
	/Users/jasonhall/git/ko/vendor/github.com/google/go-containerregistry/pkg/v1/partial/with.go:310 +0x7f
github.com/google/go-containerregistry/pkg/v1/mutate.computeDescriptor({{0x0, 0x0}, {{0x0, 0x0}, 0x0, {{0x0, 0x0}, {0x0, 0x0}}, {0x0, ...}, ...}})
	/Users/jasonhall/git/ko/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/index.go:30 +0x33
github.com/google/go-containerregistry/pkg/v1/mutate.(*index).compute(0xc0001c4460)
	/Users/jasonhall/git/ko/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/index.go:115 +0x265
github.com/google/go-containerregistry/pkg/v1/mutate.(*index).Digest(0xc0005a0000)
	/Users/jasonhall/git/ko/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/index.go:189 +0x1e
github.com/google/ko/pkg/publish.pushResult.func1({0x1a23a88, 0xc0002b4e70}, {0x9ced0a0, 0xc0000bf5c0})
	/Users/jasonhall/git/ko/pkg/publish/default.go:139 +0x383
github.com/sigstore/cosign/pkg/oci/walk.SignedEntity.func1({0x1a23a88, 0xc0002b4e70}, {0x9ced0a0, 0xc0000bf5c0})
	/Users/jasonhall/git/ko/vendor/github.com/sigstore/cosign/pkg/oci/walk/walk.go:35 +0x31
github.com/sigstore/cosign/pkg/oci/mutate.Map({0x1a242a0, 0xc00048b2c0}, {0x9ced0a0, 0xc0000bf5c0}, 0xc0004e9988)
	/Users/jasonhall/git/ko/vendor/github.com/sigstore/cosign/pkg/oci/mutate/map.go:48 +0x90
github.com/sigstore/cosign/pkg/oci/walk.SignedEntity({0x1a242a0, 0xc00048b2c0}, {0x9ced0a0, 0xc0000bf5c0}, 0x10ea44a)
	/Users/jasonhall/git/ko/vendor/github.com/sigstore/cosign/pkg/oci/walk/walk.go:34 +0x49
github.com/google/ko/pkg/publish.pushResult({0x1a242a0, 0xc00048b2c0}, {{{0x0, {0xc000215100, 0x6}}, {0xc000215107, 0x2c}}, {0xc000215134, 0x6}, {0xc000215100, ...}}, ...)
	/Users/jasonhall/git/ko/pkg/publish/default.go:179 +0x2a8
github.com/google/ko/pkg/publish.(*defalt).Publish(0xc0001c4070, {0x1a242a0, 0xc00048b2c0}, {0x9ced018, 0xc0000bf5c0}, {0xc0003138e0, 0x19})
	/Users/jasonhall/git/ko/pkg/publish/default.go:220 +0xfec
github.com/google/ko/pkg/publish.(*multiPublisher).Publish(0xc0005bc028, {0x1a242a0, 0xc00048b2c0}, {0x9ced018, 0xc0000bf5c0}, {0xc0003138e0, 0x19})
	/Users/jasonhall/git/ko/pkg/publish/multi.go:45 +0xff
github.com/google/ko/pkg/publish.(*caching).Publish.func1.1()
	/Users/jasonhall/git/ko/pkg/publish/shared.go:70 +0x3d
github.com/google/ko/pkg/publish.newFuture.func1()
	/Users/jasonhall/git/ko/pkg/publish/future.go:29 +0x2a
created by github.com/google/ko/pkg/publish.newFuture
	/Users/jasonhall/git/ko/pkg/publish/future.go:28 +0x89
exit status 2

--platform=all works as expected, the panic only happens when we have explicit platforms listed.

This seems to be an issue inside cosign/pkg/oci/walk/walk.go, which is a relatively new code path.

This also indicates we should have an e2e test that checks this code path.

cc @mattmoor
@imjasonh imjasonh mentioned this issue Dec 14, 2021
Merged
@mattmoor
Copy link
Collaborator

Weird, I generally do this a bunch, so lemme try it locally and see what's up.

@mattmoor
Copy link
Collaborator

Weird this doesn't repro with the version I have installed, but does at HEAD. Gonna open the code now and see what this is.

@mattmoor
Copy link
Collaborator

Ok, pretty sure I found the bug: https://github.com/google/ko/pull/527/files#diff-1122f9e78a67758fe70c136cfe87ae6ea4a93ec4dc81a422bfbe57d2321ba868R892

Going to confirm shortly

@mattmoor
Copy link
Collaborator

Yeah that was it.

mattmoor added a commit to mattmoor/ko that referenced this issue Dec 14, 2021
@mattmoor
Filter out nil addendum before constructing index.
8d67224 
In the PR where we added concurrency we used a fixed length array to store
addendum to preserve the ordering from the base image when constructing the
final index.  However, with `--platform=...` this list may be filtered, which
gives us `nil` entries in our addendum.

This filters `nil` entries prior to constructing the index.

Fixes: ko-build#544
@mattmoor mattmoor mentioned this issue Dec 14, 2021
Merged
mattmoor added a commit that referenced this issue Dec 14, 2021
@mattmoor
Filter out nil addendum before constructing index. (#546)
a56047a 
In the PR where we added concurrency we used a fixed length array to store
addendum to preserve the ordering from the base image when constructing the
final index.  However, with `--platform=...` this list may be filtered, which
gives us `nil` entries in our addendum.

This filters `nil` entries prior to constructing the index.

Fixes: #544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Filter out nil addendum before constructing index. mattmoor/ko
2 participants
@imjasonh @mattmoor