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

Support -q in nerdctl push #2132

Merged
merged 1 commit into from Mar 28, 2023
Merged

Conversation

djdongjin
Copy link
Member

# snd = sudo nerdctl
$ snd push xxx/alpine
INFO[0000] pushing as a reduced-platform image (application/vnd.docker.distribution.manifest.list.v2+json, sha256:b8c628ebeabe71102d6d889930d80a21ce2593c3ac17461b7a875220f2d876ab)
index-sha256:b8c628ebeabe71102d6d889930d80a21ce2593c3ac17461b7a875220f2d876ab:    done           |++++++++++++++++++++++++++++++++++++++|
manifest-sha256:93d5a28ff72d288d69b5997b8ba47396d2cbb62a72b5d87cd3351094b5d578a0: done           |++++++++++++++++++++++++++++++++++++++|
config-sha256:042a816809aac8d0f7d7cacac7965782ee2ecac3f21bcf9f24b1de1a7387b769:   done           |++++++++++++++++++++++++++++++++++++++|
elapsed: 0.2 s                                                                    total:  2.3 Ki (11.3 KiB/s)
$ snd push -q xxx/alpine
INFO[0000] pushing as a reduced-platform image (application/vnd.docker.distribution.manifest.list.v2+json, sha256:b8c628ebeabe71102d6d889930d80a21ce2593c3ac17461b7a875220f2d876ab)

As a comparison, in docker:

# d = docker
$ d push xxx/alpine
Using default tag: latest
The push refers to repository [docker.io/xxx/alpine]
7cd52847ad77: Mounted from library/alpine
latest: digest: sha256:e2e16842c9b54d985bf1ef9242a313f36b856181f188de21313820e177002501 size: 528
$ d push -q xxx/alpine
docker.io/xxx/alpine:latest

@djdongjin djdongjin marked this pull request as ready for review March 25, 2023 20:23
@AkihiroSuda
Copy link
Member

As a comparison, in docker:

Can we make the outputs same?

Signed-off-by: Jin Dong <jindon@amazon.com>
@djdongjin
Copy link
Member Author

Can we make the outputs same?

Updated to print ref iff -q is given, same as docker:

$ d push -q xxxxxx/alpine
docker.io/xxxxxx/alpine:latest

$ snd push -q xxxxxx/alpine
INFO[0000] pushing as a reduced-platform image (application/vnd.docker.distribution.manifest.list.v2+json, sha256:b8c628ebeabe71102d6d889930d80a21ce2593c3ac17461b7a875220f2d876ab)
docker.io/xxxxxx/alpine:latest

$ d push xxxxxx/alpine
Using default tag: latest
The push refers to repository [docker.io/xxxxxx/alpine]
7cd52847ad77: Layer already exists
latest: digest: sha256:e2e16842c9b54d985bf1ef9242a313f36b856181f188de21313820e177002501 size: 528

$ snd push xxxxxx/alpine
INFO[0000] pushing as a reduced-platform image (application/vnd.docker.distribution.manifest.list.v2+json, sha256:b8c628ebeabe71102d6d889930d80a21ce2593c3ac17461b7a875220f2d876ab)
index-sha256:b8c628ebeabe71102d6d889930d80a21ce2593c3ac17461b7a875220f2d876ab:    done           |++++++++++++++++++++++++++++++++++++++|
manifest-sha256:93d5a28ff72d288d69b5997b8ba47396d2cbb62a72b5d87cd3351094b5d578a0: done           |++++++++++++++++++++++++++++++++++++++|
config-sha256:042a816809aac8d0f7d7cacac7965782ee2ecac3f21bcf9f24b1de1a7387b769:   done           |++++++++++++++++++++++++++++++++++++++|
elapsed: 0.3 s

Not sure if the logs should still be generated/printed if -q is given (right not didn't change it)

Comment on lines 152 to +155
if err = signutil.Sign(rawRef, options.GOptions.Experimental, options.SignOptions); err != nil {
return err
}

Copy link
Member Author

Choose a reason for hiding this comment

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

not related to this PR. Just notice when pushing an image, sign (line 152) happens after push (line 133-150). Is this expected?

Copy link
Member

Choose a reason for hiding this comment

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

not related to this PR. Just notice when pushing an image, sign (line 152) happens after push (line 133-150). Is this expected?

IIUC signing after pushing is ok (for cosign, at least), but the current implementation is wrong anyway; the Sign() function should receive the digest from the Push() function to prohibit TOCTOU.

(I'm mentioning this bug publicly because the cosign integration is still experimental for nerdctl)

Copy link
Member

Choose a reason for hiding this comment

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

Opened an issue #2135 for further discussion

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 9d6ce8b into containerd:main Mar 28, 2023
21 checks passed
@AkihiroSuda AkihiroSuda added this to the v1.3.0 milestone Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants