From 68cca70594b67f7213fdfa18e36967d092c3222d Mon Sep 17 00:00:00 2001 From: Thiago Durante Date: Wed, 23 Sep 2026 14:45:46 +0200 Subject: [PATCH] feat(server-groups): add --transfer-order to create and update `dhq server-groups create` sent only `name`, and the API refuses that with `422 transfer_order is not included in the list`, so a server group could not be created from the CLI at all. The template-level command already had the flag; the project-level one did not. `--transfer-order` accepts exactly `sequential` or `parallel` on both `create` and `update`. It is sent as `transfer_order` only when given: omitted, nothing is sent, so the backend's default applies on create and the group's current order is kept on update. Any other value, including an explicit empty one or a different case, is rejected as a user error before a project or client is resolved, so no request is made. The backend's inclusion check is case-sensitive, which is why `Parallel` is refused too. The flag help is shared with `templates server-groups create|update` so the two commands describe it alike; the template commands' behaviour is unchanged. The `server-groups` help no longer claims a deployment reaches every server in parallel: that depends on the transfer order, which it now describes. A bare `--name` create succeeds only once the backend defaults the field (the application half of the issue); until then pass the flag. Refs deployhq/deployhq#1256 (https://github.com/deployhq/deployhq/issues/1256) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01DH13sECZeMmHSuzGmyR3Wp --- CHANGELOG.md | 28 ++++ internal/commands/server_groups.go | 62 ++++++- internal/commands/server_groups_test.go | 169 ++++++++++++++++++++ internal/commands/templates_subresources.go | 4 +- pkg/sdk/server_groups_test.go | 36 +++++ pkg/sdk/types.go | 5 + skills/deployhq/references/servers.md | 9 ++ 7 files changed, 306 insertions(+), 7 deletions(-) create mode 100644 internal/commands/server_groups_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 5199e72..fa032ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 While the CLI is pre-1.0, minor versions may carry breaking changes to the public `pkg/sdk` surface; these are always called out under **Breaking (SDK)**. +## [Unreleased] + +### Added + +- **`dhq server-groups create` / `dhq server-groups update`**: `--transfer-order` + (`sequential` or `parallel`) sets how a deployment to the group moves through + its servers — each server's whole deployment as one unit, or each step + finished on every server before any server starts the next. Sent as `transfer_order` + only when given; omitted, nothing is sent, so the backend's default applies on + create and the group's current order is kept on update. Any other value is + rejected locally, before any request. A backend that does not yet default + the field (deployhq/deployhq#1256) refuses a create without it with `422 + transfer_order is not included in the list`; pass the flag there. + +- **SDK**: `ServerGroupCreateRequest.TransferOrder` and + `ServerGroupUpdateRequest.TransferOrder` (`transfer_order`, `omitempty`). + Purely additive. + +### Changed + +- **`dhq templates server-groups create` / `update`**: `--transfer-order` help + now names the two accepted values, matching the project-level command. + Behaviour is unchanged. + +- **`dhq server-groups` help** no longer claims a deployment reaches every + server in a group in parallel; it describes the group's transfer order + instead. + ## [0.21.0] - 2026-08-06 ### Added diff --git a/internal/commands/server_groups.go b/internal/commands/server_groups.go index 9a8bbc5..30c9168 100644 --- a/internal/commands/server_groups.go +++ b/internal/commands/server_groups.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "strings" "github.com/deployhq/deployhq-cli/internal/output" "github.com/deployhq/deployhq-cli/pkg/sdk" @@ -13,7 +14,9 @@ func newServerGroupsCmd() *cobra.Command { Use: "server-groups", Aliases: []string{"sg"}, Short: "Manage server groups", - Long: `Logical groupings of servers — typically by environment (production, staging) or role (web, worker). Deploying to a group fans out to every server in it in parallel. + Long: `Logical groupings of servers — typically by environment (production, staging) or role (web, worker). Deploying to a group deploys to every server in it. + +The group's transfer order decides how: sequential runs each server's whole deployment as one unit, parallel finishes each step on every server before any server starts the next step. Useful for clusters and blue-green setups where one logical "deploy" actually targets many machines.`, } @@ -29,6 +32,35 @@ Useful for clusters and blue-green setups where one logical "deploy" actually ta return cmd } +// transferOrders are the values the DeployHQ backend accepts for a server +// group's transfer_order. Anything else is refused with a 422, so the CLI +// rejects it locally first. +var transferOrders = []string{"sequential", "parallel"} + +// transferOrderUsage is the --transfer-order help shared by the project and +// template server-group commands, so the two read alike. +const transferOrderUsage = "Transfer order: sequential (each server's whole deployment runs as one unit) " + + "or parallel (each step finishes on every server before any server starts the next step)" + +// validateTransferOrder rejects a supplied --transfer-order the backend would +// refuse. An omitted flag passes: nothing is sent, so the backend's default +// applies. It runs before any project or client resolution, so a malformed +// invocation fails with no network access. +func validateTransferOrder(cmd *cobra.Command, value string) error { + if !cmd.Flags().Changed("transfer-order") { + return nil + } + for _, v := range transferOrders { + if value == v { + return nil + } + } + return &output.UserError{ + Message: fmt.Sprintf("Invalid --transfer-order %q", value), + Hint: "Use one of: " + strings.Join(transferOrders, ", "), + } +} + func newServerGroupsListCmd() *cobra.Command { var page, perPage int @@ -136,15 +168,23 @@ func newServerGroupsShowCmd() *cobra.Command { } func newServerGroupsCreateCmd() *cobra.Command { - var name string + var name, transferOrder string cmd := &cobra.Command{ Use: "create", Short: "Create a server group", + Long: `Create a server group in a project. + +Omit --transfer-order to take DeployHQ's default (sequential).`, + Example: ` dhq server-groups create -p my-app --name "Web" + dhq server-groups create -p my-app --name "Web" --transfer-order parallel`, RunE: func(cmd *cobra.Command, args []string) error { if name == "" { return &output.UserError{Message: "Name is required", Hint: "Use --name flag"} } + if err := validateTransferOrder(cmd, transferOrder); err != nil { + return err + } projectID, err := cliCtx.RequireProject() if err != nil { @@ -156,7 +196,10 @@ func newServerGroupsCreateCmd() *cobra.Command { return err } - group, err := client.CreateServerGroup(cliCtx.Background(), projectID, sdk.ServerGroupCreateRequest{Name: name}) + group, err := client.CreateServerGroup(cliCtx.Background(), projectID, sdk.ServerGroupCreateRequest{ + Name: name, + TransferOrder: transferOrder, + }) if err != nil { return err } @@ -171,17 +214,22 @@ func newServerGroupsCreateCmd() *cobra.Command { } cmd.Flags().StringVar(&name, "name", "", "Server group name (required)") + cmd.Flags().StringVar(&transferOrder, "transfer-order", "", transferOrderUsage) return cmd } func newServerGroupsUpdateCmd() *cobra.Command { - var name string + var name, transferOrder string cmd := &cobra.Command{ Use: "update ", Short: "Update a server group", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if err := validateTransferOrder(cmd, transferOrder); err != nil { + return err + } + projectID, err := cliCtx.RequireProject() if err != nil { return err @@ -192,7 +240,10 @@ func newServerGroupsUpdateCmd() *cobra.Command { return err } - group, err := client.UpdateServerGroup(cliCtx.Background(), projectID, args[0], sdk.ServerGroupUpdateRequest{Name: name}) + group, err := client.UpdateServerGroup(cliCtx.Background(), projectID, args[0], sdk.ServerGroupUpdateRequest{ + Name: name, + TransferOrder: transferOrder, + }) if err != nil { return err } @@ -207,6 +258,7 @@ func newServerGroupsUpdateCmd() *cobra.Command { } cmd.Flags().StringVar(&name, "name", "", "New name") + cmd.Flags().StringVar(&transferOrder, "transfer-order", "", transferOrderUsage) return cmd } diff --git a/internal/commands/server_groups_test.go b/internal/commands/server_groups_test.go new file mode 100644 index 0000000..a681b69 --- /dev/null +++ b/internal/commands/server_groups_test.go @@ -0,0 +1,169 @@ +package commands + +import ( + "bytes" + "errors" + "net/http" + "strings" + "testing" + + "github.com/deployhq/deployhq-cli/internal/output" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// serverGroup returns the decoded `server_group` object from the captured body. +func (c *capturedRequest) serverGroup(t *testing.T) map[string]any { + t.Helper() + c.mu.Lock() + defer c.mu.Unlock() + require.Equal(t, 1, c.count, "expected exactly one DeployHQ API request") + sg, ok := c.body["server_group"].(map[string]any) + require.True(t, ok, "request body must wrap a server_group object, got: %v", c.body) + return sg +} + +// ── --transfer-order on the wire ────────────────────────────────────────────── +// +// The backend validates transfer_order against sequential/parallel. The CLI +// sends it only when the operator names one, so an omitted flag leaves the +// choice to the backend's default rather than to a value the CLI invented. + +func TestServerGroupsCreate_SendsTransferOrderWhenGiven(t *testing.T) { + for _, order := range []string{"parallel", "sequential"} { + t.Run(order, func(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "server-groups", "create", + "--name", "web", "--transfer-order", order) + require.NoError(t, err) + + sg := cap.serverGroup(t) + assert.Equal(t, http.MethodPost, cap.method) + assert.Equal(t, "/projects/my-app/server_groups", cap.path) + assert.Equal(t, "web", sg["name"]) + assert.Equal(t, order, sg["transfer_order"]) + }) + } +} + +func TestServerGroupsCreate_OmitsTransferOrderWhenNotGiven(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "server-groups", "create", "--name", "web") + require.NoError(t, err) + + sg := cap.serverGroup(t) + assert.Equal(t, "web", sg["name"]) + assert.NotContains(t, sg, "transfer_order", + "an omitted --transfer-order must not be sent, so the backend default applies") +} + +func TestServerGroupsUpdate_SendsTransferOrderWhenGiven(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "server-groups", "update", "sg-1", "--transfer-order", "parallel") + require.NoError(t, err) + + sg := cap.serverGroup(t) + assert.Equal(t, http.MethodPut, cap.method) + assert.Equal(t, "/projects/my-app/server_groups/sg-1", cap.path) + assert.Equal(t, "parallel", sg["transfer_order"]) + assert.NotContains(t, sg, "name", "an update must not send a name the operator did not give") +} + +func TestServerGroupsUpdate_OmitsTransferOrderWhenNotGiven(t *testing.T) { + withResolvableContext(t) + cap := captureRequest(t) + + err := runServersCmd(t, "server-groups", "update", "sg-1", "--name", "Web EU") + require.NoError(t, err) + + sg := cap.serverGroup(t) + assert.Equal(t, "Web EU", sg["name"]) + assert.NotContains(t, sg, "transfer_order", + "an update must not disturb the transfer order when the flag was omitted") +} + +// ── local validation: fails before any network access ───────────────────────── + +func TestServerGroupsCreate_RejectsUnknownTransferOrder_NoHTTP(t *testing.T) { + for _, tc := range []struct{ name, order string }{ + {"unknown", "serial"}, + {"wrong case", "Parallel"}, // the backend's inclusion check is case-sensitive + {"explicit empty", ""}, // a mistake, not a request for the default + } { + t.Run(tc.name, func(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "server-groups", "create", + "--name", "web", "--transfer-order", tc.order) + + require.Error(t, err) + var userErr *output.UserError + require.True(t, errors.As(err, &userErr), "must be a user error, got %T: %v", err, err) + assert.Contains(t, userErr.Message, "Invalid --transfer-order") + assert.Contains(t, userErr.Hint, "sequential") + assert.Contains(t, userErr.Hint, "parallel") + assert.False(t, *called, "validation must fail before any HTTP request") + }) + } +} + +func TestServerGroupsUpdate_RejectsUnknownTransferOrder_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "server-groups", "update", "sg-1", "--transfer-order", "serial") + + require.Error(t, err) + var userErr *output.UserError + require.True(t, errors.As(err, &userErr), "must be a user error, got %T: %v", err, err) + assert.Contains(t, userErr.Message, "Invalid --transfer-order") + assert.False(t, *called, "validation must fail before any HTTP request") +} + +// A missing name is still reported first, in the same style as before. +func TestServerGroupsCreate_MissingNameStillRejected_NoHTTP(t *testing.T) { + withResolvableContext(t) + called := blockNetwork(t) + + err := runServersCmd(t, "server-groups", "create", "--transfer-order", "parallel") + + require.Error(t, err) + var userErr *output.UserError + require.True(t, errors.As(err, &userErr), "must be a user error, got %T: %v", err, err) + assert.Equal(t, "Name is required", userErr.Message) + assert.False(t, *called, "validation must fail before any HTTP request") +} + +// ── help ────────────────────────────────────────────────────────────────────── + +// The project and template server-group commands take the same flag and must +// describe it the same way: both name the two accepted values. +func TestServerGroupsHelp_ListsTransferOrderValues(t *testing.T) { + for _, path := range [][]string{ + {"server-groups", "create"}, + {"server-groups", "update"}, + {"templates", "server-groups", "create"}, + {"templates", "server-groups", "update"}, + } { + t.Run(strings.Join(path, " "), func(t *testing.T) { + root := NewRootCmd("test") + var stdout bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stdout) + root.SetArgs(append(append([]string{}, path...), "--help")) + require.NoError(t, root.Execute()) + + help := stdout.String() + assert.Contains(t, help, "--transfer-order") + assert.Contains(t, help, "sequential") + assert.Contains(t, help, "parallel") + }) + } +} diff --git a/internal/commands/templates_subresources.go b/internal/commands/templates_subresources.go index 306202d..a369fa5 100644 --- a/internal/commands/templates_subresources.go +++ b/internal/commands/templates_subresources.go @@ -1516,7 +1516,7 @@ func tsgCreateCmd() *cobra.Command { addTemplateFlag(cmd, &tmpl) cmd.Flags().StringVar(&name, "name", "", "Server group name (required)") cmd.Flags().StringVar(&environment, "environment", "", "Environment") - cmd.Flags().StringVar(&transferOrder, "transfer-order", "", "Transfer order") + cmd.Flags().StringVar(&transferOrder, "transfer-order", "", transferOrderUsage) cmd.Flags().StringVar(&emailNotifyOn, "email-notify-on", "", "Email notify on (event)") cmd.Flags().StringVar(¬ificationEmail, "notification-email", "", "Notification email") cmd.Flags().BoolVar(&autoDeploy, "auto-deploy", false, "Enable auto-deploy") @@ -1562,7 +1562,7 @@ func tsgUpdateCmd() *cobra.Command { addTemplateFlag(cmd, &tmpl) cmd.Flags().StringVar(&name, "name", "", "Server group name") cmd.Flags().StringVar(&environment, "environment", "", "Environment") - cmd.Flags().StringVar(&transferOrder, "transfer-order", "", "Transfer order") + cmd.Flags().StringVar(&transferOrder, "transfer-order", "", transferOrderUsage) cmd.Flags().StringVar(&emailNotifyOn, "email-notify-on", "", "Email notify on (event)") cmd.Flags().StringVar(¬ificationEmail, "notification-email", "", "Notification email") cmd.Flags().BoolVar(&autoDeploy, "auto-deploy", false, "Enable auto-deploy") diff --git a/pkg/sdk/server_groups_test.go b/pkg/sdk/server_groups_test.go index b10d5ef..eb46f7d 100644 --- a/pkg/sdk/server_groups_test.go +++ b/pkg/sdk/server_groups_test.go @@ -68,6 +68,42 @@ func TestCreateServerGroup(t *testing.T) { assert.Equal(t, "Staging", group.Name) } +// transfer_order is sent only when set: an empty value must be absent from the +// body, not sent as "", so the backend's default applies. +func TestCreateServerGroup_TransferOrder(t *testing.T) { + for _, tc := range []struct { + name string + order string + present bool + }{ + {"set", "parallel", true}, + {"unset", "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + ServerGroup map[string]any `json:"server_group"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + got, present := body.ServerGroup["transfer_order"] + assert.Equal(t, tc.present, present, "transfer_order presence in %v", body.ServerGroup) + if tc.present { + assert.Equal(t, tc.order, got) + } + + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(ServerGroup{Identifier: "sg-new", Name: "Web"}) + })) + defer server.Close() + + c := newTestClient(t, server) + _, err := c.CreateServerGroup(context.Background(), "my-app", + ServerGroupCreateRequest{Name: "Web", TransferOrder: tc.order}) + require.NoError(t, err) + }) + } +} + func TestDeleteServerGroup(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodDelete, r.Method) diff --git a/pkg/sdk/types.go b/pkg/sdk/types.go index 63fb451..ef380fe 100644 --- a/pkg/sdk/types.go +++ b/pkg/sdk/types.go @@ -257,11 +257,16 @@ type ServerGroup struct { // ServerGroupCreateRequest is the payload for creating a server group. type ServerGroupCreateRequest struct { Name string `json:"name"` + // TransferOrder is "sequential" or "parallel". Left empty it is omitted + // from the request, so the backend's default applies. + TransferOrder string `json:"transfer_order,omitempty"` } // ServerGroupUpdateRequest is the payload for updating a server group. type ServerGroupUpdateRequest struct { Name string `json:"name,omitempty"` + // TransferOrder is "sequential" or "parallel"; empty leaves it unchanged. + TransferOrder string `json:"transfer_order,omitempty"` } // ServerAgent is the embedded agent object within a server response. diff --git a/skills/deployhq/references/servers.md b/skills/deployhq/references/servers.md index dc10275..f17175e 100644 --- a/skills/deployhq/references/servers.md +++ b/skills/deployhq/references/servers.md @@ -325,13 +325,22 @@ dhq server-groups list -p my-app --json ``` ### `dhq server-groups create` +`--transfer-order` is `sequential` (each server's whole deployment runs as one +unit) or `parallel` (each step finishes on every server before any server starts +the next step). Omit it to take DeployHQ's default, `sequential`; any other value is +rejected before a request is made. + ```bash dhq server-groups create -p my-app --name "US Servers" --json +dhq server-groups create -p my-app --name "US Servers" --transfer-order parallel --json ``` ### `dhq server-groups update ` +Takes the same `--transfer-order` values; omitted, the group's order is left as it is. + ```bash dhq server-groups update grp-001 -p my-app --name "EU Servers" --json +dhq server-groups update grp-001 -p my-app --transfer-order sequential --json ``` ### `dhq server-groups delete `