diff --git a/pkg/cmd/imagecmd.go b/pkg/cmd/imagecmd.go index 5c86df4..1db93c2 100644 --- a/pkg/cmd/imagecmd.go +++ b/pkg/cmd/imagecmd.go @@ -69,6 +69,20 @@ var imageDeleteCmd = cli.Command{ HideHelpCommand: true, } +// platformFromRaw reads the top-level "platform" field from a raw server payload, +// defaulting to "-" when absent. SDK v0.20.0's Image/Instance models predate the +// platform field, so it is only reachable via the raw JSON. +// +// TODO(sdk-bump): drop this once the SDK exposes a typed platform field and read +// it directly alongside the other columns. +func platformFromRaw(raw string) string { + platform := gjson.Get(raw, "platform").String() + if platform == "" { + return "-" + } + return platform +} + func handleImageList(ctx context.Context, cmd *cli.Command) error { client := hypeman.NewClient(getDefaultRequestOptions(cmd)...) @@ -118,8 +132,8 @@ func handleImageList(ctx context.Context, cmd *cli.Command) error { return nil } - table := NewTableWriter(os.Stdout, "NAME", "STATUS", "DIGEST", "SIZE", "CREATED") - table.TruncOrder = []int{0, 2, 4} // NAME first, then DIGEST, CREATED + table := NewTableWriter(os.Stdout, "NAME", "PLATFORM", "STATUS", "DIGEST", "SIZE", "CREATED") + table.TruncOrder = []int{0, 3, 5} // NAME first, then DIGEST, CREATED for _, img := range *images { digest := img.Digest if len(digest) > 19 { @@ -133,6 +147,7 @@ func handleImageList(ctx context.Context, cmd *cli.Command) error { table.AddRow( img.Name, + platformFromRaw(img.RawJSON()), string(img.Status), digest, size, @@ -165,6 +180,7 @@ func handleImageCreateLike(ctx context.Context, cmd *cli.Command, usageLine, out if cmd.Root().Bool("debug") { opts = append(opts, debugMiddlewareOption) } + opts = append(opts, platformRequestOptions(cmd.String("platform"))...) format := cmd.Root().String("format") transform := cmd.Root().String("transform") @@ -194,6 +210,10 @@ func imageCreateFlags() []cli.Flag { Name: "tag", Usage: "Set image tag key-value pair (KEY=VALUE, can be repeated)", }, + &cli.StringFlag{ + Name: "platform", + Usage: `Target platform as os/arch[/variant] (e.g., "linux/amd64")`, + }, } } @@ -208,6 +228,16 @@ func buildImageNewParams(name string, tagSpecs []string) (hypeman.ImageNewParams return params, malformedTags } +// platformRequestOptions sets the Docker-style platform field on an image- or +// instance-create request. SDK v0.20.0 has no typed platform field, so we set it +// via WithJSONSet. TODO(sdk-bump): pass a typed field once the SDK exposes it. +func platformRequestOptions(platform string) []option.RequestOption { + if platform == "" { + return nil + } + return []option.RequestOption{option.WithJSONSet("platform", platform)} +} + func handleImageGet(ctx context.Context, cmd *cli.Command) error { args := cmd.Args().Slice() if len(args) < 1 { diff --git a/pkg/cmd/imagecmd_test.go b/pkg/cmd/imagecmd_test.go index 710df9d..a20b77b 100644 --- a/pkg/cmd/imagecmd_test.go +++ b/pkg/cmd/imagecmd_test.go @@ -21,3 +21,13 @@ func TestBuildImageNewParams(t *testing.T) { }, params.Tags) assert.Equal(t, []string{"missing-delimiter"}, malformed) } + +// TestPlatformFromRaw guards the N4 image-list PLATFORM column: the value is read +// from the raw server payload (the SDK model predates the field) and falls back +// to "-" when absent. +func TestPlatformFromRaw(t *testing.T) { + assert.Equal(t, "linux/amd64", platformFromRaw(`{"name":"alpine","platform":"linux/amd64"}`)) + assert.Equal(t, "-", platformFromRaw(`{"name":"alpine"}`)) + assert.Equal(t, "-", platformFromRaw(`{"name":"alpine","platform":""}`)) + assert.Equal(t, "-", platformFromRaw("")) +} diff --git a/pkg/cmd/inspect.go b/pkg/cmd/inspect.go index 6941dbc..d0ee82c 100644 --- a/pkg/cmd/inspect.go +++ b/pkg/cmd/inspect.go @@ -2,13 +2,14 @@ package cmd import ( "context" - "encoding/json" "fmt" "os" + "strings" "github.com/kernel/hypeman-go" "github.com/kernel/hypeman-go/option" "github.com/tidwall/gjson" + "github.com/tidwall/sjson" "github.com/urfave/cli/v3" ) @@ -49,31 +50,56 @@ func handleInspect(ctx context.Context, cmd *cli.Command) error { return err } - if !cmd.Bool("show-env") { - instance.Env = redactEnvValues(instance.Env) + // Render from the raw server response rather than the typed struct: SDK + // v0.20.0's Instance model predates fields like `platform`, so marshaling the + // struct would silently drop them. RawJSON carries the full server payload. + raw := instance.RawJSON() + if raw == "" { + return fmt.Errorf("instance response did not include a raw payload") } - raw, err := json.Marshal(instance) - if err != nil { - return fmt.Errorf("failed to encode instance response: %w", err) + if !cmd.Bool("show-env") { + raw = redactEnvValues(raw) } format := cmd.Root().String("format") transform := cmd.Root().String("transform") - obj := gjson.ParseBytes(raw) + obj := gjson.Parse(raw) return ShowJSON(os.Stdout, "instance inspect", obj, format, transform) } -func redactEnvValues(env map[string]string) map[string]string { - if len(env) == 0 { - return env +// redactEnvValues replaces every value under the top-level "env" object with +// "[hidden]" in the raw JSON, preserving the keys. On any sjson error it falls +// back to the original payload so inspect never fails on redaction alone. +func redactEnvValues(raw string) string { + env := gjson.Get(raw, "env") + if !env.Exists() || !env.IsObject() { + return raw } - - redacted := make(map[string]string, len(env)) - for key := range env { - redacted[key] = "[hidden]" + out := raw + var setErr error + env.ForEach(func(key, _ gjson.Result) bool { + updated, err := sjson.Set(out, "env."+escapeSJSONKey(key.String()), "[hidden]") + if err != nil { + setErr = err + return false + } + out = updated + return true + }) + if setErr != nil { + return raw } + return out +} - return redacted +// escapeSJSONKey escapes characters sjson treats specially in a path (the '.' +// separator plus the '*', '?', '|', '#', and '@' wildcards/modifiers) so env var +// names containing them address the intended key. +func escapeSJSONKey(key string) string { + for _, special := range []string{".", "*", "?", "|", "#", "@"} { + key = strings.ReplaceAll(key, special, "\\"+special) + } + return key } diff --git a/pkg/cmd/inspect_test.go b/pkg/cmd/inspect_test.go index a9f3d9b..6eefabe 100644 --- a/pkg/cmd/inspect_test.go +++ b/pkg/cmd/inspect_test.go @@ -4,31 +4,56 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" ) func TestRedactEnvValues(t *testing.T) { - t.Run("returns empty map as-is", func(t *testing.T) { - var env map[string]string - assert.Nil(t, redactEnvValues(env)) - - empty := map[string]string{} - assert.Equal(t, empty, redactEnvValues(empty)) + t.Run("returns payload unchanged when env absent", func(t *testing.T) { + raw := `{"id":"i1","platform":"linux/amd64"}` + assert.Equal(t, raw, redactEnvValues(raw)) }) t.Run("redacts all values and preserves keys", func(t *testing.T) { - env := map[string]string{ - "FOO": "bar", - "BAZ": "qux", - } + raw := `{"id":"i1","env":{"FOO":"bar","BAZ":"qux"}}` + + redacted := redactEnvValues(raw) + + assert.Equal(t, "[hidden]", gjson.Get(redacted, "env.FOO").String()) + assert.Equal(t, "[hidden]", gjson.Get(redacted, "env.BAZ").String()) + assert.True(t, gjson.Get(redacted, "env.FOO").Exists()) + assert.True(t, gjson.Get(redacted, "env.BAZ").Exists()) + // Non-env fields are untouched. + assert.Equal(t, "i1", gjson.Get(redacted, "id").String()) + }) + + t.Run("handles env keys containing dots", func(t *testing.T) { + raw := `{"env":{"my.var":"secret"}}` - redacted := redactEnvValues(env) + redacted := redactEnvValues(raw) - assert.Equal(t, map[string]string{ - "FOO": "[hidden]", - "BAZ": "[hidden]", - }, redacted) - require.Equal(t, "bar", env["FOO"]) - require.Equal(t, "qux", env["BAZ"]) + assert.Equal(t, "[hidden]", gjson.Get(redacted, `env.my\.var`).String()) }) + + t.Run("handles env keys containing sjson special chars", func(t *testing.T) { + // '*', '?', '|', '#', '@' are all special to sjson path parsing. + raw := `{"env":{"A*B":"s1","C?D":"s2","E|F":"s3","G#H":"s4","I@J":"s5"}}` + + redacted := redactEnvValues(raw) + + for _, key := range []string{`env.A\*B`, `env.C\?D`, `env.E\|F`, `env.G\#H`, `env.I\@J`} { + assert.Equal(t, "[hidden]", gjson.Get(redacted, key).String(), "key %s", key) + } + }) +} + +// TestRedactEnvValuesPreservesPlatform guards the N4 fix: rendering from the raw +// server payload must keep fields (like platform) that the SDK's typed Instance +// model predates and would otherwise drop. +func TestRedactEnvValuesPreservesPlatform(t *testing.T) { + raw := `{"id":"i1","platform":"linux/amd64","env":{"FOO":"bar"}}` + + redacted := redactEnvValues(raw) + + assert.Equal(t, "linux/amd64", gjson.Get(redacted, "platform").String()) + assert.Equal(t, "[hidden]", gjson.Get(redacted, "env.FOO").String()) } diff --git a/pkg/cmd/pull.go b/pkg/cmd/pull.go index 6303355..201fa8f 100644 --- a/pkg/cmd/pull.go +++ b/pkg/cmd/pull.go @@ -38,6 +38,7 @@ func handlePull(ctx context.Context, cmd *cli.Command) error { if cmd.Root().Bool("debug") { opts = append(opts, debugMiddlewareOption) } + opts = append(opts, platformRequestOptions(cmd.String("platform"))...) format := cmd.Root().String("format") transform := cmd.Root().String("transform") diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index b5d9642..5251557 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -3,6 +3,7 @@ package cmd import ( "context" "encoding/json" + "errors" "fmt" "net/url" "os" @@ -25,6 +26,9 @@ Examples: # Basic run hypeman run myimage:latest + # Run an amd64 image on Apple Silicon via Rosetta + hypeman run --platform linux/amd64 --hypervisor vz docker.io/library/alpine:3.19 + # Run with custom resources hypeman run --cpus 4 --memory 8GB myimage:latest @@ -53,6 +57,10 @@ Examples: Name: "credentials-json", Usage: "Credential policy map as JSON (keyed by guest-visible env var)", }, + &cli.StringFlag{ + Name: "platform", + Usage: `Target platform as os/arch[/variant] (e.g., "linux/amd64")`, + }, &cli.StringFlag{ Name: "memory", Usage: `Base memory size (e.g., "1GB", "512MB")`, @@ -187,6 +195,7 @@ func handleRun(ctx context.Context, cmd *cli.Command) error { } image := args[0] + platform := cmd.String("platform") client := hypeman.NewClient(getDefaultRequestOptions(cmd)...) @@ -195,12 +204,11 @@ func handleRun(ctx context.Context, cmd *cli.Command) error { imgInfo, err := client.Images.Get(ctx, url.PathEscape(image)) if err != nil { // Image not found, try to pull it - var apiErr *hypeman.Error - if ok := isNotFoundError(err, &apiErr); ok { + if isNotFoundError(err) { fmt.Fprintf(os.Stderr, "Image not found locally. Pulling %s...\n", image) imgInfo, err = client.Images.New(ctx, hypeman.ImageNewParams{ Name: image, - }) + }, platformRequestOptions(platform)...) if err != nil { return fmt.Errorf("failed to pull image: %w", err) } @@ -240,6 +248,7 @@ func handleRun(ctx context.Context, cmd *cli.Command) error { OverlaySize: hypeman.Opt(cmd.String("overlay-size")), HotplugSize: hypeman.Opt(cmd.String("hotplug-size")), } + instanceCreateOpts := platformRequestOptions(platform) if len(env) > 0 { params.Env = env @@ -402,7 +411,7 @@ func handleRun(ctx context.Context, cmd *cli.Command) error { result, err := client.Instances.New( ctx, params, - opts..., + append(opts, instanceCreateOpts...)..., ) if err != nil { return err @@ -437,9 +446,9 @@ func buildNetworkEgress(enabled bool, enabledSet bool, mode string) (hypeman.Ins } // isNotFoundError checks if err is a 404 not found error -func isNotFoundError(err error, target **hypeman.Error) bool { - if apiErr, ok := err.(*hypeman.Error); ok { - *target = apiErr +func isNotFoundError(err error) bool { + var apiErr *hypeman.Error + if errors.As(err, &apiErr) { return apiErr.Response != nil && apiErr.Response.StatusCode == 404 } return false @@ -471,6 +480,12 @@ func waitForImageReady(ctx context.Context, client *hypeman.Client, img *hypeman case <-ticker.C: updated, err := client.Images.Get(ctx, url.PathEscape(img.Name)) if err != nil { + // A cold pull's record may briefly 404 before it is queryable. + // Treat that as "still pulling" and keep polling rather than + // surfacing a confusing not-found error for an in-flight pull. + if isNotFoundError(err) { + continue + } return fmt.Errorf("failed to check image status: %w", err) } diff --git a/pkg/cmd/run_test.go b/pkg/cmd/run_test.go index 8aea90c..d3cef9c 100644 --- a/pkg/cmd/run_test.go +++ b/pkg/cmd/run_test.go @@ -1,12 +1,49 @@ package cmd import ( + "errors" + "fmt" + "net/http" "testing" + hypeman "github.com/kernel/hypeman-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func apiErrorWithStatus(code int) *hypeman.Error { + return &hypeman.Error{Response: &http.Response{StatusCode: code}} +} + +func TestIsNotFoundError(t *testing.T) { + t.Run("404 API error is not found", func(t *testing.T) { + assert.True(t, isNotFoundError(apiErrorWithStatus(http.StatusNotFound))) + }) + + t.Run("wrapped 404 API error is not found", func(t *testing.T) { + // errors.As traverses the wrap chain, so an in-flight pull's 404 is still + // detected even when callers add context with fmt.Errorf. + err := fmt.Errorf("failed to check image status: %w", apiErrorWithStatus(http.StatusNotFound)) + assert.True(t, isNotFoundError(err)) + }) + + t.Run("non-404 API error is not not-found", func(t *testing.T) { + assert.False(t, isNotFoundError(apiErrorWithStatus(http.StatusInternalServerError))) + }) + + t.Run("API error without response is not not-found", func(t *testing.T) { + assert.False(t, isNotFoundError(&hypeman.Error{})) + }) + + t.Run("plain error is not not-found", func(t *testing.T) { + assert.False(t, isNotFoundError(errors.New("boom"))) + }) + + t.Run("nil error is not not-found", func(t *testing.T) { + assert.False(t, isNotFoundError(nil)) + }) +} + func TestBuildNetworkEgress(t *testing.T) { t.Run("defaults enabled to true when mode is set", func(t *testing.T) { egress, err := buildNetworkEgress(false, false, "all") diff --git a/pkg/cmd/snapshotcmd.go b/pkg/cmd/snapshotcmd.go index cdcc543..70b8914 100644 --- a/pkg/cmd/snapshotcmd.go +++ b/pkg/cmd/snapshotcmd.go @@ -64,9 +64,10 @@ var snapshotCreateCmd = cli.Command{ } var snapshotRestoreCmd = cli.Command{ - Name: "restore", - Usage: "Restore an instance from a snapshot", - ArgsUsage: " ", + Name: "restore", + Usage: "Restore an instance from a snapshot", + Description: "The target instance must not be Running; stop or standby it first (restoring a Running instance returns 409 invalid_state).", + ArgsUsage: " ", Flags: []cli.Flag{ &cli.StringFlag{ Name: "target-hypervisor",