From ecc6c8d3af31749566ee1eaf3b9f3836c16033f9 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:09:19 +0000 Subject: [PATCH 1/4] Improve Docker-like push output --- pkg/cmd/push.go | 80 +++++++++++++++++++++++++++++++++++++++----- pkg/cmd/push_test.go | 33 ++++++++++++++++++ 2 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 pkg/cmd/push_test.go diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 5476beb..9278939 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -3,6 +3,7 @@ package cmd import ( "context" "fmt" + "io" "net/http" "net/url" "os" @@ -10,17 +11,23 @@ import ( "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/daemon" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/urfave/cli/v3" + "golang.org/x/term" ) var pushCmd = cli.Command{ Name: "push", Aliases: []string{"pushes"}, - Usage: "Push a local Docker image to hypeman", - ArgsUsage: " [target-name]", - Description: `Push a local Docker image into the hypeman image cache. + Usage: "Push an image to hypeman", + ArgsUsage: "NAME[:TAG] [TARGET]", + Description: `Push an image from the local Docker daemon into the hypeman image cache. + +The command follows Docker's push flow: the source image is read from the +local daemon, uploaded to hypeman, and reported with its manifest digest. If +TARGET is omitted, the source name and tag are used. Subcommands manage outbound pushes, which export a cached hypeman image to a remote registry (e.g. AWS ECR, Docker Hub): @@ -29,9 +36,12 @@ remote registry (e.g. AWS ECR, Docker Hub): hypeman push get Get push details Examples: - # Push a local Docker image into hypeman + # Push the local nginx:latest image hypeman push nginx:latest + # Push using a different repository or tag + hypeman push nginx:latest myapp/nginx:v1 + # Export a cached hypeman image to a remote registry hypeman push create nginx:latest registry.example.com/nginx:latest`, Commands: []*cli.Command{ @@ -61,6 +71,12 @@ func handlePush(ctx context.Context, cmd *cli.Command) error { if err != nil { return fmt.Errorf("invalid base URL: %w", err) } + if parsedURL.Host == "" { + return fmt.Errorf("invalid base URL %q: missing host", baseURL) + } + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return fmt.Errorf("invalid base URL %q: scheme must be http or https", baseURL) + } registryHost := parsedURL.Host @@ -76,15 +92,21 @@ func handlePush(ctx context.Context, cmd *cli.Command) error { return fmt.Errorf("load image: %w", err) } - // Build target reference - server computes digest from manifest + // Build the target reference. The server computes the image digest from + // the manifest, while the tag keeps the image addressable with Docker-like + // image names after the push. targetRef := registryHost + "/" + strings.TrimPrefix(targetName, "/") - fmt.Fprintf(os.Stderr, "Pushing to %s...\n", targetRef) - - dstRef, err := name.ParseReference(targetRef, name.Insecure) + parseOptions := []name.Option(nil) + if parsedURL.Scheme == "http" { + parseOptions = append(parseOptions, name.Insecure) + } + dstRef, err := name.ParseReference(targetRef, parseOptions...) if err != nil { return fmt.Errorf("invalid target: %w", err) } + fmt.Fprintf(os.Stderr, "The push refers to repository [%s]\n", dstRef.Context().Name()) + token := resolveAPIKey() // Use custom transport that always sends Basic auth header @@ -93,19 +115,59 @@ func handlePush(ctx context.Context, cmd *cli.Command) error { token: token, } + progress := make(chan v1.Update, 32) + progressDone := make(chan struct{}) + go func() { + defer close(progressDone) + renderPushProgress(progress, os.Stderr, term.IsTerminal(int(os.Stderr.Fd()))) + }() + err = remote.Write(dstRef, img, remote.WithContext(ctx), remote.WithAuth(authn.Anonymous), remote.WithTransport(transport), + remote.WithProgress(progress), ) + <-progressDone if err != nil { return fmt.Errorf("push failed: %w", err) } - fmt.Fprintf(os.Stderr, "Pushed %s\n", targetRef) + digest, err := img.Digest() + if err != nil { + return fmt.Errorf("read pushed image digest: %w", err) + } + rawManifest, err := img.RawManifest() + if err != nil { + return fmt.Errorf("read pushed image manifest: %w", err) + } + + fmt.Fprintf(os.Stderr, "%s: digest: %s size: %d\n", dstRef.Identifier(), digest, len(rawManifest)) return nil } +// renderPushProgress consumes go-containerregistry's aggregate byte updates. +// Keep progress on stderr so stdout remains available for shell pipelines. +func renderPushProgress(updates <-chan v1.Update, output io.Writer, interactive bool) { + if !interactive { + for range updates { + } + return + } + + printed := false + for update := range updates { + if update.Error != nil || update.Total <= 0 { + continue + } + fmt.Fprintf(output, "\r%s / %s", formatBytes(update.Complete), formatBytes(update.Total)) + printed = true + } + if printed { + fmt.Fprintln(output) + } +} + // authTransport adds Basic auth header to all requests type authTransport struct { base http.RoundTripper diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go new file mode 100644 index 0000000..ad58f6b --- /dev/null +++ b/pkg/cmd/push_test.go @@ -0,0 +1,33 @@ +package cmd + +import ( + "bytes" + "testing" + + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/stretchr/testify/assert" +) + +func TestRenderPushProgress(t *testing.T) { + updates := make(chan v1.Update, 3) + updates <- v1.Update{Total: 2048, Complete: 1024} + updates <- v1.Update{Total: 2048, Complete: 2048} + updates <- v1.Update{Error: assert.AnError} + close(updates) + + var output bytes.Buffer + renderPushProgress(updates, &output, true) + + assert.Equal(t, "\r1.0 KB / 2.0 KB\r2.0 KB / 2.0 KB\n", output.String()) +} + +func TestRenderPushProgressNonInteractive(t *testing.T) { + updates := make(chan v1.Update, 1) + updates <- v1.Update{Total: 1024, Complete: 1024} + close(updates) + + var output bytes.Buffer + renderPushProgress(updates, &output, false) + + assert.Empty(t, output.String()) +} From d9a2aabb9bff07fa9e077b1040c002fc63f91e8b Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 14 Aug 2026 15:05:22 +0000 Subject: [PATCH 2/4] Make remote push the primary flow --- pkg/cmd/push.go | 71 +++++++++++++------- pkg/cmd/pushcmd.go | 145 ++++++++++++++++++++++++++++++++-------- pkg/cmd/pushcmd_test.go | 11 ++- 3 files changed, 170 insertions(+), 57 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 9278939..05c009f 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -21,42 +21,61 @@ import ( var pushCmd = cli.Command{ Name: "push", Aliases: []string{"pushes"}, - Usage: "Push an image to hypeman", - ArgsUsage: "NAME[:TAG] [TARGET]", - Description: `Push an image from the local Docker daemon into the hypeman image cache. + Usage: "Push an image to a registry", + ArgsUsage: "SOURCE [TARGET]", + Description: `Push an image from Hypeman to a remote registry. -The command follows Docker's push flow: the source image is read from the -local daemon, uploaded to hypeman, and reported with its manifest digest. If -TARGET is omitted, the source name and tag are used. +The source image must already exist in Hypeman. TARGET is the remote registry +reference, matching Docker's push syntax as closely as possible. -Subcommands manage outbound pushes, which export a cached hypeman image to a -remote registry (e.g. AWS ECR, Docker Hub): - hypeman push create Push a hypeman image to a remote registry - hypeman push list List outbound image push jobs - hypeman push get Get push details +Local Docker-daemon uploads remain available explicitly with "push local": + hypeman push local IMAGE [TARGET] + +Push jobs can be inspected while they run: + hypeman push ls + hypeman push inspect Examples: - # Push the local nginx:latest image - hypeman push nginx:latest - - # Push using a different repository or tag - hypeman push nginx:latest myapp/nginx:v1 - - # Export a cached hypeman image to a remote registry - hypeman push create nginx:latest registry.example.com/nginx:latest`, - Commands: []*cli.Command{ - &pushCreateCmd, - &pushListCmd, - &pushGetCmd, - }, + # Push a cached image to ECR + hypeman push alpine:latest 123456789.dkr.ecr.us-east-1.amazonaws.com/myapp:v1 + + # Push with credentials read from stdin + echo "$ECR_PASSWORD" | hypeman push alpine:latest registry.example.com/app:v1 \ + --username AWS --password-stdin + + # Upload a local Docker image into Hypeman + hypeman push local nginx:latest`, + Flags: pushRemoteFlags(), + Commands: []*cli.Command{&pushLocalCmd, &pushCreateCmd, &pushListCmd, &pushGetCmd}, Action: handlePush, HideHelpCommand: true, } +var pushLocalCmd = cli.Command{ + Name: "local", + Usage: "Upload a local Docker image to Hypeman", + ArgsUsage: "IMAGE [TARGET]", + Action: handleLocalPush, + HideHelpCommand: true, +} + func handlePush(ctx context.Context, cmd *cli.Command) error { args := cmd.Args().Slice() - if len(args) < 1 { - return fmt.Errorf("image reference required\nUsage: hypeman push ") + switch len(args) { + case 1: + // Keep the old one-argument form working for existing scripts. + return handleLocalPush(ctx, cmd) + case 2: + return runRemotePush(ctx, cmd, args[0], args[1]) + default: + return fmt.Errorf("source image and target required\nUsage: hypeman push ") + } +} + +func handleLocalPush(ctx context.Context, cmd *cli.Command) error { + args := cmd.Args().Slice() + if len(args) < 1 || len(args) > 2 { + return fmt.Errorf("image reference required\nUsage: hypeman push local [target]") } sourceImage := args[0] diff --git a/pkg/cmd/pushcmd.go b/pkg/cmd/pushcmd.go index 64af10a..ee3b311 100644 --- a/pkg/cmd/pushcmd.go +++ b/pkg/cmd/pushcmd.go @@ -3,30 +3,20 @@ package cmd import ( "context" "fmt" + "io" "os" + "strings" + "time" + "github.com/google/go-containerregistry/pkg/name" "github.com/kernel/hypeman-go" "github.com/kernel/hypeman-go/option" "github.com/tidwall/gjson" "github.com/urfave/cli/v3" ) -var pushCreateCmd = cli.Command{ - Name: "create", - Usage: "Push a hypeman image to a remote registry", - ArgsUsage: " ", - Description: `Create a push job that exports a hypeman image to a remote registry. - -Only images in the ready state can be pushed. The push runs asynchronously; -use "hypeman push get " to poll its progress. - -Examples: - # Push a cached image to ECR using the server's registry credentials - hypeman push create alpine:latest 123456789.dkr.ecr.us-east-1.amazonaws.com/myapp:v1 - - # Push with credentials borrowed for this push only - hypeman push create alpine:latest registry.example.com/myapp:v1 --username alice --password s3cret`, - Flags: []cli.Flag{ +func pushRemoteFlags() []cli.Flag { + return []cli.Flag{ &cli.BoolFlag{ Name: "insecure", Usage: "Allow pushing to plain-HTTP registries", @@ -39,18 +29,40 @@ Examples: Name: "password", Usage: "Registry password or access token", }, + &cli.BoolFlag{ + Name: "password-stdin", + Usage: "Read the registry password from stdin", + }, &cli.StringFlag{ Name: "registry-token", Usage: "Bearer token for an Authorization header", }, - }, + &cli.BoolFlag{ + Name: "detach", + Aliases: []string{"d"}, + Usage: "Return after queueing the push", + }, + } +} + +var pushCreateCmd = cli.Command{ + Name: "create", + Aliases: []string{"remote"}, + Usage: "Create a remote push job (deprecated; use push SOURCE TARGET)", + ArgsUsage: " ", + Flags: pushRemoteFlags(), + Description: `Create a remote push job without waiting for it to finish. + +Use "hypeman push SOURCE TARGET" for the Docker-like flow. This command is +kept as a compatibility alias for existing scripts.`, Action: handlePushCreate, HideHelpCommand: true, } var pushListCmd = cli.Command{ - Name: "list", - Usage: "List outbound image push jobs", + Name: "list", + Aliases: []string{"ls"}, + Usage: "List outbound image push jobs", Flags: []cli.Flag{ &cli.BoolFlag{ Name: "quiet", @@ -64,6 +76,7 @@ var pushListCmd = cli.Command{ var pushGetCmd = cli.Command{ Name: "get", + Aliases: []string{"inspect"}, Usage: "Get push details", ArgsUsage: "", Action: handlePushGet, @@ -72,16 +85,24 @@ var pushGetCmd = cli.Command{ func handlePushCreate(ctx context.Context, cmd *cli.Command) error { args := cmd.Args().Slice() - if len(args) < 2 { + if len(args) != 2 { return fmt.Errorf("image and target required\nUsage: hypeman push create ") } + return runRemotePush(ctx, cmd, args[0], args[1]) +} + +func runRemotePush(ctx context.Context, cmd *cli.Command, image, target string) error { + password, err := pushPassword(cmd) + if err != nil { + return err + } params := buildPushNewParams( - args[0], - args[1], + image, + target, cmd.Bool("insecure"), cmd.String("username"), - cmd.String("password"), + password, cmd.String("registry-token"), ) @@ -94,7 +115,6 @@ func handlePushCreate(ctx context.Context, cmd *cli.Command) error { format := cmd.Root().String("format") transform := cmd.Root().String("transform") - if format != "auto" { var res []byte opts = append(opts, option.WithResponseBodyInto(&res)) @@ -102,8 +122,7 @@ func handlePushCreate(ctx context.Context, cmd *cli.Command) error { if err != nil { return err } - obj := gjson.ParseBytes(res) - return ShowJSON(os.Stdout, "push create", obj, format, transform) + return ShowJSON(os.Stdout, "push", gjson.ParseBytes(res), format, transform) } push, err := client.Pushes.New(ctx, params, opts...) @@ -111,9 +130,77 @@ func handlePushCreate(ctx context.Context, cmd *cli.Command) error { return err } - fmt.Fprintf(os.Stderr, "Pushing %s to %s...\n", push.Image, push.Target) - fmt.Println(push.ID) - return nil + if cmd.Bool("detach") || cmd.Name == "create" || cmd.Name == "remote" { + fmt.Fprintf(os.Stderr, "push queued: %s\n", push.ID) + fmt.Println(push.ID) + return nil + } + + return waitForPush(ctx, &client, push, opts) +} + +func pushPassword(cmd *cli.Command) (string, error) { + password := cmd.String("password") + if !cmd.Bool("password-stdin") { + return password, nil + } + if password != "" { + return "", fmt.Errorf("--password and --password-stdin cannot be used together") + } + + data, err := io.ReadAll(os.Stdin) + if err != nil { + return "", fmt.Errorf("read registry password from stdin: %w", err) + } + return strings.TrimRight(string(data), "\r\n"), nil +} + +func waitForPush(ctx context.Context, client *hypeman.Client, push *hypeman.Push, opts []option.RequestOption) error { + fmt.Fprintf(os.Stderr, "The push refers to repository [%s]\n", pushRepository(push.Target)) + fmt.Fprintln(os.Stderr, "queued") + + ticker := time.NewTicker(time.Second) + defer ticker.Stop() + lastStatus := push.Status + var lastBytes int64 + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + current, err := client.Pushes.Get(ctx, push.ID, opts...) + if err != nil { + return fmt.Errorf("check push %s: %w", push.ID, err) + } + if current.Status != lastStatus { + fmt.Fprintln(os.Stderr, string(current.Status)) + lastStatus = current.Status + } + if current.Status == hypeman.PushStatusPushing && current.Bytes > lastBytes { + fmt.Fprintf(os.Stderr, "pushing %s (%d layers)\n", formatBytes(current.Bytes), current.Layers) + lastBytes = current.Bytes + } + + switch current.Status { + case hypeman.PushStatusPushed: + fmt.Fprintf(os.Stderr, "digest: %s\n", current.Digest) + return nil + case hypeman.PushStatusFailed: + if current.Error != "" { + return fmt.Errorf("push failed: %s", current.Error) + } + return fmt.Errorf("push failed") + } + } + } +} + +func pushRepository(target string) string { + ref, err := name.ParseReference(target) + if err != nil { + return target + } + return ref.Context().Name() } // buildPushNewParams assembles the outbound push request. Credentials are only diff --git a/pkg/cmd/pushcmd_test.go b/pkg/cmd/pushcmd_test.go index 699337b..cfc1fe0 100644 --- a/pkg/cmd/pushcmd_test.go +++ b/pkg/cmd/pushcmd_test.go @@ -13,15 +13,22 @@ func TestPushCommandStructure(t *testing.T) { subcommandNames = append(subcommandNames, sub.Name) } + assert.Contains(t, subcommandNames, "local") assert.Contains(t, subcommandNames, "create") assert.Contains(t, subcommandNames, "list") assert.Contains(t, subcommandNames, "get") + assert.Contains(t, pushListCmd.Aliases, "ls") + assert.Contains(t, pushGetCmd.Aliases, "inspect") - // The parent action still pushes a local Docker image into hypeman, so it - // must stay reachable alongside the outbound push subcommands. + // The parent action remains reachable for the legacy local-upload form and + // the new direct remote-push form. assert.NotNil(t, pushCmd.Action) } +func TestPushRepository(t *testing.T) { + assert.Equal(t, "registry.example.com/app", pushRepository("registry.example.com/app:v1")) +} + func TestBuildPushNewParams(t *testing.T) { params := buildPushNewParams("alpine:latest", "registry.example.com/alpine:v1", false, "", "", "") From 0a20e7eb2c77b93f56e459743e73d5aba0911a78 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:32:13 +0000 Subject: [PATCH 3/4] Polish push output and validation --- pkg/cmd/push.go | 19 +++-- pkg/cmd/push_test.go | 23 ++++++ pkg/cmd/pushcmd.go | 163 ++++++++++++++++++++++++++++++---------- pkg/cmd/pushcmd_test.go | 7 ++ 4 files changed, 163 insertions(+), 49 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 05c009f..5f4a36e 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -99,21 +99,14 @@ func handleLocalPush(ctx context.Context, cmd *cli.Command) error { registryHost := parsedURL.Host - fmt.Fprintf(os.Stderr, "Loading image %s from Docker...\n", sourceImage) - srcRef, err := name.ParseReference(sourceImage) if err != nil { return fmt.Errorf("invalid source image: %w", err) } - img, err := daemon.Image(srcRef) - if err != nil { - return fmt.Errorf("load image: %w", err) - } - - // Build the target reference. The server computes the image digest from - // the manifest, while the tag keeps the image addressable with Docker-like - // image names after the push. + // Build and validate the target before opening the Docker daemon. The + // server computes the image digest from the manifest, while the tag keeps + // the image addressable with Docker-like image names after the push. targetRef := registryHost + "/" + strings.TrimPrefix(targetName, "/") parseOptions := []name.Option(nil) if parsedURL.Scheme == "http" { @@ -124,6 +117,12 @@ func handleLocalPush(ctx context.Context, cmd *cli.Command) error { return fmt.Errorf("invalid target: %w", err) } + fmt.Fprintf(os.Stderr, "Loading image %s from Docker...\n", sourceImage) + img, err := daemon.Image(srcRef) + if err != nil { + return fmt.Errorf("load image: %w", err) + } + fmt.Fprintf(os.Stderr, "The push refers to repository [%s]\n", dstRef.Context().Name()) token := resolveAPIKey() diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go index ad58f6b..ab2405f 100644 --- a/pkg/cmd/push_test.go +++ b/pkg/cmd/push_test.go @@ -31,3 +31,26 @@ func TestRenderPushProgressNonInteractive(t *testing.T) { assert.Empty(t, output.String()) } + +func TestPushStatusRenderer(t *testing.T) { + var output bytes.Buffer + renderer := &pushStatusRenderer{output: &output, interactive: true} + + renderer.update("queued") + renderer.update("queued") + renderer.update("pushing 1.0 MB") + renderer.finish() + + assert.Equal(t, "\r\033[Kqueued\r\033[Kpushing 1.0 MB\n", output.String()) +} + +func TestPushStatusRendererNonInteractive(t *testing.T) { + var output bytes.Buffer + renderer := &pushStatusRenderer{output: &output} + + renderer.update("queued") + renderer.update("pushing 1.0 MB") + renderer.finish() + + assert.Equal(t, "queued\npushing 1.0 MB\n", output.String()) +} diff --git a/pkg/cmd/pushcmd.go b/pkg/cmd/pushcmd.go index ee3b311..d095f3d 100644 --- a/pkg/cmd/pushcmd.go +++ b/pkg/cmd/pushcmd.go @@ -13,6 +13,7 @@ import ( "github.com/kernel/hypeman-go/option" "github.com/tidwall/gjson" "github.com/urfave/cli/v3" + "golang.org/x/term" ) func pushRemoteFlags() []cli.Flag { @@ -51,7 +52,8 @@ var pushCreateCmd = cli.Command{ Usage: "Create a remote push job (deprecated; use push SOURCE TARGET)", ArgsUsage: " ", Flags: pushRemoteFlags(), - Description: `Create a remote push job without waiting for it to finish. + Description: `Create a remote push job. The command waits by default; use --detach +when existing scripts need the push ID immediately. Use "hypeman push SOURCE TARGET" for the Docker-like flow. This command is kept as a compatibility alias for existing scripts.`, @@ -92,6 +94,10 @@ func handlePushCreate(ctx context.Context, cmd *cli.Command) error { } func runRemotePush(ctx context.Context, cmd *cli.Command, image, target string) error { + if err := validateRemotePushReferences(image, target); err != nil { + return err + } + password, err := pushPassword(cmd) if err != nil { return err @@ -115,28 +121,54 @@ func runRemotePush(ctx context.Context, cmd *cli.Command, image, target string) format := cmd.Root().String("format") transform := cmd.Root().String("transform") + var createResponse []byte + createOpts := opts if format != "auto" { - var res []byte - opts = append(opts, option.WithResponseBodyInto(&res)) - _, err := client.Pushes.New(ctx, params, opts...) - if err != nil { - return err - } - return ShowJSON(os.Stdout, "push", gjson.ParseBytes(res), format, transform) + createOpts = append(append([]option.RequestOption(nil), opts...), option.WithResponseBodyInto(&createResponse)) } - push, err := client.Pushes.New(ctx, params, opts...) + push, err := client.Pushes.New(ctx, params, createOpts...) if err != nil { return err } - if cmd.Bool("detach") || cmd.Name == "create" || cmd.Name == "remote" { + if cmd.Bool("detach") { + if format != "auto" { + return ShowJSON(os.Stdout, "push", gjson.ParseBytes(createResponse), format, transform) + } fmt.Fprintf(os.Stderr, "push queued: %s\n", push.ID) fmt.Println(push.ID) return nil } - return waitForPush(ctx, &client, push, opts) + var finalResponse []byte + final, err := waitForPush(ctx, &client, push, opts, format != "auto", &finalResponse) + if err != nil { + return err + } + if format != "auto" { + if len(finalResponse) == 0 { + finalResponse = []byte(final.RawJSON()) + } + return ShowJSON(os.Stdout, "push", gjson.ParseBytes(finalResponse), format, transform) + } + return nil +} + +func validateRemotePushReferences(image, target string) error { + if _, err := name.ParseReference(image); err != nil { + return fmt.Errorf("invalid source image %q: %w", image, err) + } + if _, err := name.ParseReference(target); err != nil { + return fmt.Errorf("invalid target %q: %w", target, err) + } + lastSlash := strings.LastIndex(target, "/") + lastColon := strings.LastIndex(target, ":") + lastAt := strings.LastIndex(target, "@") + if lastAt > lastSlash || lastColon <= lastSlash { + return fmt.Errorf("target %q must include an explicit tag", target) + } + return nil } func pushPassword(cmd *cli.Command) (string, error) { @@ -155,43 +187,96 @@ func pushPassword(cmd *cli.Command) (string, error) { return strings.TrimRight(string(data), "\r\n"), nil } -func waitForPush(ctx context.Context, client *hypeman.Client, push *hypeman.Push, opts []option.RequestOption) error { - fmt.Fprintf(os.Stderr, "The push refers to repository [%s]\n", pushRepository(push.Target)) - fmt.Fprintln(os.Stderr, "queued") +func waitForPush(ctx context.Context, client *hypeman.Client, push *hypeman.Push, opts []option.RequestOption, quiet bool, finalResponse *[]byte) (*hypeman.Push, error) { + var renderer *pushStatusRenderer + if !quiet { + fmt.Fprintf(os.Stderr, "The push refers to repository [%s]\n", pushRepository(push.Target)) + renderer = &pushStatusRenderer{ + output: os.Stderr, + interactive: term.IsTerminal(int(os.Stderr.Fd())), + } + } - ticker := time.NewTicker(time.Second) - defer ticker.Stop() - lastStatus := push.Status + current := push var lastBytes int64 for { - select { - case <-ctx.Done(): - return ctx.Err() - case <-ticker.C: - current, err := client.Pushes.Get(ctx, push.ID, opts...) - if err != nil { - return fmt.Errorf("check push %s: %w", push.ID, err) - } - if current.Status != lastStatus { - fmt.Fprintln(os.Stderr, string(current.Status)) - lastStatus = current.Status - } - if current.Status == hypeman.PushStatusPushing && current.Bytes > lastBytes { - fmt.Fprintf(os.Stderr, "pushing %s (%d layers)\n", formatBytes(current.Bytes), current.Layers) - lastBytes = current.Bytes - } - + if renderer != nil { switch current.Status { + case hypeman.PushStatusQueued: + renderer.update(fmt.Sprintf("queued · %s", current.Target)) + case hypeman.PushStatusPushing: + if current.Bytes > lastBytes { + lastBytes = current.Bytes + } + renderer.update(fmt.Sprintf("pushing %s · %d layers · %s", formatBytes(lastBytes), current.Layers, current.Target)) case hypeman.PushStatusPushed: - fmt.Fprintf(os.Stderr, "digest: %s\n", current.Digest) - return nil + renderer.update(fmt.Sprintf("pushed · digest: %s", current.Digest)) case hypeman.PushStatusFailed: - if current.Error != "" { - return fmt.Errorf("push failed: %s", current.Error) + message := current.Error + if message == "" { + message = "unknown error" } - return fmt.Errorf("push failed") + renderer.update("failed · " + message) + } + } + + switch current.Status { + case hypeman.PushStatusPushed: + if renderer != nil { + renderer.finish() } + return current, nil + case hypeman.PushStatusFailed: + if renderer != nil { + renderer.finish() + } + if current.Error != "" { + return nil, fmt.Errorf("push %s failed: %s", push.ID, current.Error) + } + return nil, fmt.Errorf("push %s failed", push.ID) + } + + ticker := time.NewTimer(time.Second) + select { + case <-ctx.Done(): + ticker.Stop() + return nil, ctx.Err() + case <-ticker.C: } + + getOpts := opts + if finalResponse != nil { + getOpts = append(append([]option.RequestOption(nil), opts...), option.WithResponseBodyInto(finalResponse)) + } + var err error + current, err = client.Pushes.Get(ctx, push.ID, getOpts...) + if err != nil { + return nil, fmt.Errorf("check push %s: %w", push.ID, err) + } + } +} + +type pushStatusRenderer struct { + output io.Writer + interactive bool + last string +} + +func (r *pushStatusRenderer) update(message string) { + if message == r.last { + return + } + r.last = message + if r.interactive { + fmt.Fprintf(r.output, "\r\033[K%s", message) + return + } + fmt.Fprintln(r.output, message) +} + +func (r *pushStatusRenderer) finish() { + if r.interactive && r.last != "" { + fmt.Fprintln(r.output) } } diff --git a/pkg/cmd/pushcmd_test.go b/pkg/cmd/pushcmd_test.go index cfc1fe0..5422068 100644 --- a/pkg/cmd/pushcmd_test.go +++ b/pkg/cmd/pushcmd_test.go @@ -29,6 +29,13 @@ func TestPushRepository(t *testing.T) { assert.Equal(t, "registry.example.com/app", pushRepository("registry.example.com/app:v1")) } +func TestValidateRemotePushReferences(t *testing.T) { + assert.NoError(t, validateRemotePushReferences("alpine:latest", "registry.example.com/app:v1")) + assert.ErrorContains(t, validateRemotePushReferences("alpine:latest", "registry.example.com/app"), "explicit tag") + assert.ErrorContains(t, validateRemotePushReferences("not valid", "registry.example.com/app:v1"), "invalid source image") + assert.Error(t, validateRemotePushReferences("alpine:latest", "registry.example.com/app@sha256:abc")) +} + func TestBuildPushNewParams(t *testing.T) { params := buildPushNewParams("alpine:latest", "registry.example.com/alpine:v1", false, "", "", "") From 6ce076898ba995f3eeb4ff1b6990f6d804309dfd Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:40:09 +0000 Subject: [PATCH 4/4] Fix push status and compatibility bugs --- pkg/cmd/push.go | 41 +++++++++++++++++++++++++---------------- pkg/cmd/push_test.go | 4 ++-- pkg/cmd/pushcmd.go | 37 ++++++++++++------------------------- 3 files changed, 39 insertions(+), 43 deletions(-) diff --git a/pkg/cmd/push.go b/pkg/cmd/push.go index 5f4a36e..e72f623 100644 --- a/pkg/cmd/push.go +++ b/pkg/cmd/push.go @@ -135,9 +135,10 @@ func handleLocalPush(ctx context.Context, cmd *cli.Command) error { progress := make(chan v1.Update, 32) progressDone := make(chan struct{}) + progressStop := make(chan struct{}) go func() { defer close(progressDone) - renderPushProgress(progress, os.Stderr, term.IsTerminal(int(os.Stderr.Fd()))) + renderPushProgress(progress, os.Stderr, term.IsTerminal(int(os.Stderr.Fd())), progressStop) }() err = remote.Write(dstRef, img, @@ -146,6 +147,7 @@ func handleLocalPush(ctx context.Context, cmd *cli.Command) error { remote.WithTransport(transport), remote.WithProgress(progress), ) + close(progressStop) <-progressDone if err != nil { return fmt.Errorf("push failed: %w", err) @@ -166,23 +168,30 @@ func handleLocalPush(ctx context.Context, cmd *cli.Command) error { // renderPushProgress consumes go-containerregistry's aggregate byte updates. // Keep progress on stderr so stdout remains available for shell pipelines. -func renderPushProgress(updates <-chan v1.Update, output io.Writer, interactive bool) { - if !interactive { - for range updates { - } - return - } - +func renderPushProgress(updates <-chan v1.Update, output io.Writer, interactive bool, stop <-chan struct{}) { printed := false - for update := range updates { - if update.Error != nil || update.Total <= 0 { - continue + for { + select { + case <-stop: + if printed && interactive { + fmt.Fprintln(output) + } + return + case update, ok := <-updates: + if !ok { + if printed && interactive { + fmt.Fprintln(output) + } + return + } + if update.Error != nil || update.Total <= 0 { + continue + } + if interactive { + fmt.Fprintf(output, "\r%s / %s", formatBytes(update.Complete), formatBytes(update.Total)) + printed = true + } } - fmt.Fprintf(output, "\r%s / %s", formatBytes(update.Complete), formatBytes(update.Total)) - printed = true - } - if printed { - fmt.Fprintln(output) } } diff --git a/pkg/cmd/push_test.go b/pkg/cmd/push_test.go index ab2405f..4085a33 100644 --- a/pkg/cmd/push_test.go +++ b/pkg/cmd/push_test.go @@ -16,7 +16,7 @@ func TestRenderPushProgress(t *testing.T) { close(updates) var output bytes.Buffer - renderPushProgress(updates, &output, true) + renderPushProgress(updates, &output, true, make(chan struct{})) assert.Equal(t, "\r1.0 KB / 2.0 KB\r2.0 KB / 2.0 KB\n", output.String()) } @@ -27,7 +27,7 @@ func TestRenderPushProgressNonInteractive(t *testing.T) { close(updates) var output bytes.Buffer - renderPushProgress(updates, &output, false) + renderPushProgress(updates, &output, false, make(chan struct{})) assert.Empty(t, output.String()) } diff --git a/pkg/cmd/pushcmd.go b/pkg/cmd/pushcmd.go index d095f3d..102d043 100644 --- a/pkg/cmd/pushcmd.go +++ b/pkg/cmd/pushcmd.go @@ -52,11 +52,11 @@ var pushCreateCmd = cli.Command{ Usage: "Create a remote push job (deprecated; use push SOURCE TARGET)", ArgsUsage: " ", Flags: pushRemoteFlags(), - Description: `Create a remote push job. The command waits by default; use --detach -when existing scripts need the push ID immediately. + Description: `Create a remote push job and return its ID immediately. -Use "hypeman push SOURCE TARGET" for the Docker-like flow. This command is -kept as a compatibility alias for existing scripts.`, +Use "hypeman push SOURCE TARGET" for the Docker-like flow, which waits by +default. This command is kept as a detached compatibility alias for existing +scripts.`, Action: handlePushCreate, HideHelpCommand: true, } @@ -121,36 +121,27 @@ func runRemotePush(ctx context.Context, cmd *cli.Command, image, target string) format := cmd.Root().String("format") transform := cmd.Root().String("transform") - var createResponse []byte - createOpts := opts - if format != "auto" { - createOpts = append(append([]option.RequestOption(nil), opts...), option.WithResponseBodyInto(&createResponse)) - } - - push, err := client.Pushes.New(ctx, params, createOpts...) + push, err := client.Pushes.New(ctx, params, opts...) if err != nil { return err } - if cmd.Bool("detach") { + legacyDetached := cmd.Name == "create" || cmd.Name == "remote" + if cmd.Bool("detach") || legacyDetached { if format != "auto" { - return ShowJSON(os.Stdout, "push", gjson.ParseBytes(createResponse), format, transform) + return ShowJSON(os.Stdout, "push", gjson.Parse(push.RawJSON()), format, transform) } fmt.Fprintf(os.Stderr, "push queued: %s\n", push.ID) fmt.Println(push.ID) return nil } - var finalResponse []byte - final, err := waitForPush(ctx, &client, push, opts, format != "auto", &finalResponse) + final, err := waitForPush(ctx, &client, push, opts, format != "auto") if err != nil { return err } if format != "auto" { - if len(finalResponse) == 0 { - finalResponse = []byte(final.RawJSON()) - } - return ShowJSON(os.Stdout, "push", gjson.ParseBytes(finalResponse), format, transform) + return ShowJSON(os.Stdout, "push", gjson.Parse(final.RawJSON()), format, transform) } return nil } @@ -187,7 +178,7 @@ func pushPassword(cmd *cli.Command) (string, error) { return strings.TrimRight(string(data), "\r\n"), nil } -func waitForPush(ctx context.Context, client *hypeman.Client, push *hypeman.Push, opts []option.RequestOption, quiet bool, finalResponse *[]byte) (*hypeman.Push, error) { +func waitForPush(ctx context.Context, client *hypeman.Client, push *hypeman.Push, opts []option.RequestOption, quiet bool) (*hypeman.Push, error) { var renderer *pushStatusRenderer if !quiet { fmt.Fprintf(os.Stderr, "The push refers to repository [%s]\n", pushRepository(push.Target)) @@ -244,12 +235,8 @@ func waitForPush(ctx context.Context, client *hypeman.Client, push *hypeman.Push case <-ticker.C: } - getOpts := opts - if finalResponse != nil { - getOpts = append(append([]option.RequestOption(nil), opts...), option.WithResponseBodyInto(finalResponse)) - } var err error - current, err = client.Pushes.Get(ctx, push.ID, getOpts...) + current, err = client.Pushes.Get(ctx, push.ID, opts...) if err != nil { return nil, fmt.Errorf("check push %s: %w", push.ID, err) }