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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions cmd/nerdctl/image_push.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ func newPushCommand() *cobra.Command {
pushCommand.Flags().String("notation-key-name", "", "Signing key name for a key previously added to notation's key list for --sign=notation")
// #endregion

pushCommand.Flags().BoolP("quiet", "q", false, "Suppress verbose output")

pushCommand.Flags().Bool(allowNonDistFlag, false, "Allow pushing images with non-distributable blobs")

return pushCommand
Expand Down Expand Up @@ -87,6 +89,10 @@ func processImagePushOptions(cmd *cobra.Command) (types.ImagePushOptions, error)
if err != nil {
return types.ImagePushOptions{}, err
}
quiet, err := cmd.Flags().GetBool("quiet")
if err != nil {
return types.ImagePushOptions{}, err
}
allowNonDist, err := cmd.Flags().GetBool(allowNonDistFlag)
if err != nil {
return types.ImagePushOptions{}, err
Expand All @@ -103,6 +109,7 @@ func processImagePushOptions(cmd *cobra.Command) (types.ImagePushOptions, error)
Estargz: estargz,
IpfsEnsureImage: ipfsEnsureImage,
IpfsAddress: ipfsAddress,
Quiet: quiet,
AllowNondistributableArtifacts: allowNonDist,
Stdout: cmd.OutOrStdout(),
}, nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/types/image_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ type ImagePushOptions struct {
IpfsEnsureImage bool
// IpfsAddress multiaddr of IPFS API (default uses $IPFS_PATH env variable if defined or local directory ~/.ipfs)
IpfsAddress string
// Suppress verbose output
Quiet bool
// AllowNondistributableArtifacts allow pushing non-distributable artifacts
AllowNondistributableArtifacts bool
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/cmd/image/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import (
"github.com/sirupsen/logrus"
)

// Push pushes an image specified by `rawRef`.
func Push(ctx context.Context, client *containerd.Client, rawRef string, options types.ImagePushOptions) error {
if scheme, ref, err := referenceutil.ParseIPFSRefWithScheme(rawRef); err == nil {
if scheme != "ipfs" {
Expand Down Expand Up @@ -116,7 +117,7 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options
}

pushFunc := func(r remotes.Resolver) error {
return push.Push(ctx, client, r, options.Stdout, pushRef, ref, platMC, options.AllowNondistributableArtifacts)
return push.Push(ctx, client, r, options.Stdout, pushRef, ref, platMC, options.AllowNondistributableArtifacts, options.Quiet)
}

var dOpts []dockerconfigresolver.Opt
Expand Down Expand Up @@ -151,6 +152,10 @@ func Push(ctx context.Context, client *containerd.Client, rawRef string, options
if err = signutil.Sign(rawRef, options.GOptions.Experimental, options.SignOptions); err != nil {
return err
}

Comment on lines 152 to +155
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

if options.Quiet {
fmt.Fprintln(options.Stdout, ref)
}
return nil
}

Expand Down
53 changes: 28 additions & 25 deletions pkg/imgutil/push/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ import (
"golang.org/x/sync/errgroup"
)

// Push pushes an image to a remote registry.
func Push(ctx context.Context, client *containerd.Client, resolver remotes.Resolver, stdout io.Writer,
localRef, remoteRef string, platform platforms.MatchComparer, allowNonDist bool) error {
localRef, remoteRef string, platform platforms.MatchComparer, allowNonDist, quiet bool) error {
img, err := client.ImageService().Get(ctx, localRef)
if err != nil {
return fmt.Errorf("unable to resolve image to manifest: %w", err)
Expand Down Expand Up @@ -77,37 +78,39 @@ func Push(ctx context.Context, client *containerd.Client, resolver remotes.Resol
)
})

eg.Go(func() error {
var (
ticker = time.NewTicker(100 * time.Millisecond)
fw = progress.NewWriter(stdout)
start = time.Now()
done bool
)
if !quiet {
eg.Go(func() error {
var (
ticker = time.NewTicker(100 * time.Millisecond)
fw = progress.NewWriter(stdout)
start = time.Now()
done bool
)

defer ticker.Stop()
defer ticker.Stop()

for {
select {
case <-ticker.C:
fw.Flush()
for {
select {
case <-ticker.C:
fw.Flush()

tw := tabwriter.NewWriter(fw, 1, 8, 1, ' ', 0)
tw := tabwriter.NewWriter(fw, 1, 8, 1, ' ', 0)

jobs.Display(tw, ongoing.status(), start)
tw.Flush()
jobs.Display(tw, ongoing.status(), start)
tw.Flush()

if done {
fw.Flush()
return nil
if done {
fw.Flush()
return nil
}
case <-doneCh:
done = true
case <-ctx.Done():
done = true // allow ui to update once more
}
case <-doneCh:
done = true
case <-ctx.Done():
done = true // allow ui to update once more
}
}
})
})
}
return eg.Wait()
}

Expand Down