Implement render services update (#493)

Builds on prior PRs in this stack to implement `render services update`. Most of the logic already exists. But before this change, we were not actually building a PATCH request body, so the command was a no-op. Now it actually PATCHes!

- For now, we accept the limitation that you cannot _change_ runtimes (e.g., from Python -> Docker) via the CLI.
- `--num-instances` is not allowed. I will remove that in a follow-up PR. I backlogged a ticket for implementing `render services scale`.

GROW-2375

GitOrigin-RevId: 718a6071051e1b6533c5761ae33720d3cd12c76c

Author: davidharting

cmd/serviceupdate.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/spf13/cobra"
 
-	"github.com/render-oss/cli/pkg/client"
 	"github.com/render-oss/cli/pkg/command"
 	"github.com/render-oss/cli/pkg/dependencies"
 	servicepkg "github.com/render-oss/cli/pkg/service"
@@ -38,8 +37,8 @@ Provide configuration updates with flags.`,
 		// Interactive mode is not implemented yet; force non-interactive behavior for now.
 		command.DefaultFormatNonInteractive(cmd)
 
-		cliInput = servicetypes.NormalizeServiceUpdateCLIInput(cliInput)
-		if err := cliInput.ValidateUpdate(); err != nil {
+		cliInput, err := servicetypes.NormalizeAndValidateUpdateInput(cliInput)
+		if err != nil {
 			return err
 		}
 
@@ -123,9 +122,19 @@ func updateServiceNonInteractive(ctx context.Context, deps *dependencies.Depende
 		return nil, fmt.Errorf("failed to resolve service %q: %w", idOrName, err)
 	}
 
-	// TODO: BuildUpdateRequest to construct the PATCH body
-	// For now, use an empty UpdateServiceJSONRequestBody as a placeholder
-	body := client.UpdateServiceJSONRequestBody{}
+	before, err := serviceRepo.GetService(ctx, serviceID)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := cliInput.ValidateForServiceType(servicetypes.ServiceType(before.Type)); err != nil {
+		return nil, err
+	}
+
+	body, err := servicepkg.BuildUpdateRequest(*before, cliInput)
+	if err != nil {
+		return nil, err
+	}
 
 	if _, err := serviceRepo.UpdateService(ctx, serviceID, body); err != nil {
 		return nil, err

cmd/serviceupdate_test.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"bytes"
+	"net/http"
 	"testing"
 
 	renderapi "github.com/render-oss/cli/internal/fakes/renderapi"
@@ -11,6 +12,8 @@ import (
 	"github.com/render-oss/cli/pkg/command"
 	"github.com/render-oss/cli/pkg/config"
 	"github.com/render-oss/cli/pkg/dependencies"
+	"github.com/render-oss/cli/pkg/pointers"
+	servicetypes "github.com/render-oss/cli/pkg/types/service"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -194,7 +197,7 @@ func (h serviceUpdateHarness) setupCmd() (*cobra.Command, *bytes.Buffer, *bytes.
 	return root, &stdout, &stderr
 }
 
-func TestServiceUpdate_JSONOutput_ReturnsServiceOutEnvelope(t *testing.T) {
+func TestServiceUpdate_JSONOutput_UpdatesTopLevelFieldsAndReturnsEnvelope(t *testing.T) {
 	harness := newServiceUpdateHarness(t)
 	project := harness.server.CreateProject(
 		renderapi.ProjectAttrs{Name: "Website", OwnerId: serviceTestWorkspaceID},
@@ -209,19 +212,22 @@ func TestServiceUpdate_JSONOutput_ReturnsServiceOutEnvelope(t *testing.T) {
 		},
 	}))
 
+	// Per-flag PATCH-body wiring is covered by TestServiceUpdate_FlagWiresIntoPatchBody.
+	// This test owns the JSON output envelope shape, so it sets a single flag.
 	result, err := harness.execute(svc.Id,
 		"--name", "json-service-renamed",
 		"--output", "json",
 	)
 	require.NoError(t, err)
 
 	assert.True(t, harness.server.HasRequest("PATCH", "/services/"+svc.Id))
+	assert.Equal(t, "json-service-renamed", svc.Name)
 
 	body := testrequire.ParseJSONMap(t, result.Stdout)
 	testassert.MapContains(t, body, map[string]any{
 		"data": map[string]any{
 			"id":            svc.Id,
-			"name":          "json-service",
+			"name":          "json-service-renamed",
 			"type":          string(client.WebService),
 			"projectId":     project.Project.Id,
 			"environmentId": envID,
@@ -233,3 +239,215 @@ func TestServiceUpdate_JSONOutput_ReturnsServiceOutEnvelope(t *testing.T) {
 	})
 	assert.NotContains(t, body, "id")
 }
+
+// addWebService seeds a web service under a new project and returns it.
+func (h serviceUpdateHarness) addWebService(name string) *client.Service {
+	h.t.Helper()
+
+	project := h.server.CreateProject(
+		renderapi.ProjectAttrs{Name: "Website", OwnerId: serviceTestWorkspaceID},
+		renderapi.EnvAttrs{Name: "production"},
+	)
+	return h.server.Services.Add(renderapi.NewWebService(renderapi.WebServiceAttrs{
+		Service: renderapi.CommonServiceAttrs{
+			Name:          name,
+			OwnerID:       serviceTestWorkspaceID,
+			EnvironmentID: project.Env("production").Id,
+		},
+	}))
+}
+
+// TestServiceUpdate_ParseCommand_HappyPath verifies that every flag the update command
+// registers binds to the expected ServiceUpdateInput field with the expected
+// type. It drives the real command's flag set through ParseCommand in memory —
+// but does not invoke the command.
+// It exhaustively covers the CLI flag input <-> `cli:` tag seam that the e2e tests would only exercise indirectly.
+// The input->PATCH-body transform is covered separately in pkg/service/update_test.go.
+func TestServiceUpdate_ParseCommand_HappyPath(t *testing.T) {
+	cmd := newServiceUpdateTestCmd()
+	require.NoError(t, cmd.ParseFlags([]string{
+		"--name", "svc-name",
+		"--repo", "https://github.com/example/repo",
+		"--branch", "main",
+		"--image", "docker.io/library/nginx:alpine",
+		"--plan", "starter",
+		"--runtime", string(servicetypes.ServiceRuntimeGo),
+		"--root-directory", "app",
+		"--build-command", "make build",
+		"--start-command", "./run",
+		"--pre-deploy-command", "bin/migrate",
+		"--health-check-path", "/ready",
+		"--publish-directory", "public",
+		"--cron-command", "echo hi",
+		"--cron-schedule", "0 12 * * *",
+		"--registry-credential", "rc-123",
+		"--auto-deploy=false",
+		"--build-filter-path", "src/**",
+		"--build-filter-path", "go.mod",
+		"--build-filter-ignored-path", "docs/**",
+		"--num-instances", "3",
+		"--max-shutdown-delay", "42",
+		"--previews", string(servicetypes.PreviewsGenerationManual),
+		"--maintenance-mode=true",
+		"--maintenance-mode-uri", "https://status.example.com",
+		"--ip-allow-list", "cidr=203.0.113.5/32,description=office",
+	}))
+
+	var in servicetypes.ServiceUpdateInput
+	require.NoError(t, command.ParseCommand(cmd, []string{"srv-abc123"}, &in))
+
+	previews := servicetypes.PreviewsGenerationManual
+	runtime := servicetypes.ServiceRuntimeGo
+	assert.Equal(t, servicetypes.ServiceUpdateInput{
+		Name:                    "svc-name",
+		Repo:                    pointers.From("https://github.com/example/repo"),
+		Branch:                  pointers.From("main"),
+		Image:                   pointers.From("docker.io/library/nginx:alpine"),
+		Plan:                    pointers.From("starter"),
+		Runtime:                 &runtime,
+		RootDirectory:           pointers.From("app"),
+		BuildCommand:            pointers.From("make build"),
+		StartCommand:            pointers.From("./run"),
+		PreDeployCommand:        pointers.From("bin/migrate"),
+		HealthCheckPath:         pointers.From("/ready"),
+		PublishDirectory:        pointers.From("public"),
+		CronCommand:             pointers.From("echo hi"),
+		CronSchedule:            pointers.From("0 12 * * *"),
+		RegistryCredential:      pointers.From("rc-123"),
+		AutoDeploy:              pointers.From(false),
+		BuildFilterPaths:        []string{"src/**", "go.mod"},
+		BuildFilterIgnoredPaths: []string{"docs/**"},
+		NumInstances:            pointers.From(3),
+		MaxShutdownDelay:        pointers.From(42),
+		Previews:                &previews,
+		MaintenanceMode:         pointers.From(true),
+		MaintenanceModeURI:      pointers.From("https://status.example.com"),
+		IPAllowList:             []string{"cidr=203.0.113.5/32,description=office"},
+		ServiceIDOrName:         "srv-abc123",
+	}, in)
+}
+
+// TestServiceUpdate_FlagWiresIntoPatchBody verifies end-to-end that flags reach
+// the serialized PATCH body and that nothing unspecified comes along for the
+// ride. Each case asserts the *entire* decoded body by equality, so an extra or
+// misplaced field fails the test — stronger than checking individual keys.
+// Cases cover a single top-level field, a single serviceDetails field, a mix of
+// top-level and serviceDetails fields, and multiple serviceDetails fields.
+func TestServiceUpdate_FlagWiresIntoPatchBody(t *testing.T) {
+	tests := []struct {
+		name     string
+		args     []string
+		wantBody map[string]any
+	}{
+		{
+			name:     "name maps to top-level name only",
+			args:     []string{"--name", "renamed-service"},
+			wantBody: map[string]any{"name": "renamed-service"},
+		},
+		{
+			name: "plan maps into serviceDetails only",
+			args: []string{"--plan", "starter"},
+			wantBody: map[string]any{
+				"serviceDetails": map[string]any{"plan": "starter"},
+			},
+		},
+		{
+			name: "top-level and detail fields coexist without leaking",
+			args: []string{"--name", "renamed-service", "--health-check-path", "/ready"},
+			wantBody: map[string]any{
+				"name":           "renamed-service",
+				"serviceDetails": map[string]any{"healthCheckPath": "/ready"},
+			},
+		},
+		{
+			name: "multiple serviceDetails fields land together",
+			args: []string{"--health-check-path", "/ready", "--plan", "starter"},
+			wantBody: map[string]any{
+				"serviceDetails": map[string]any{
+					"healthCheckPath": "/ready",
+					"plan":            "starter",
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			harness := newServiceUpdateHarness(t)
+			svc := harness.addWebService("wire-service")
+
+			args := append([]string{svc.Id}, tt.args...)
+			args = append(args, "--output", "json")
+			_, err := harness.execute(args...)
+			require.NoError(t, err)
+
+			rec, ok := harness.server.LastRequest("PATCH", "/services/"+svc.Id)
+			require.True(t, ok, "expected a PATCH request to be recorded")
+			body := testrequire.ParseJSONMap(t, string(rec.Body))
+			assert.Equal(t, tt.wantBody, body)
+		})
+	}
+}
+
+func TestServiceUpdate_TypeIncompatibleFlag_FailsBeforePatch(t *testing.T) {
+	harness := newServiceUpdateHarness(t)
+	svc := harness.addWebService("web-service")
+
+	// --cron-schedule is rejected only by ValidateForServiceType, which runs
+	// against the fetched service type. The command must fail before writing.
+	_, err := harness.execute(svc.Id, "--cron-schedule", "0 12 * * *", "--output", "json")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "--cron-schedule is not supported for web_service")
+	assert.False(t, harness.server.HasRequest("PATCH", "/services/"+svc.Id))
+}
+
+func TestServiceUpdate_DefaultTextOutput(t *testing.T) {
+	harness := newServiceUpdateHarness(t)
+	svc := harness.addWebService("text-service")
+
+	result, err := harness.execute(svc.Id, "--name", "text-service-renamed")
+	require.NoError(t, err)
+
+	assert.True(t, harness.server.HasRequest("PATCH", "/services/"+svc.Id))
+	assert.Contains(t, result.Stdout, "Updated this service")
+	assert.Contains(t, result.Stdout, "text-service-renamed")
+}
+
+func TestServiceUpdate_ServiceDetailsPatchReachesAPI(t *testing.T) {
+	harness := newServiceUpdateHarness(t)
+	svc := harness.addWebService("plan-service")
+
+	// --plan flows into the ServiceDetails union; this exercises that the union
+	// serializes and is accepted end-to-end (the fake decodes the PATCH body).
+	_, err := harness.execute(svc.Id, "--plan", "starter", "--output", "json")
+	require.NoError(t, err)
+
+	assert.True(t, harness.server.HasRequest("PATCH", "/services/"+svc.Id))
+}
+
+func TestServiceUpdate_UnknownService_FailsToResolveBeforePatch(t *testing.T) {
+	harness := newServiceUpdateHarness(t)
+
+	// No service is seeded, so resolving the name finds nothing. The command
+	// must surface a resolve error and never attempt a PATCH.
+	_, err := harness.execute("does-not-exist", "--name", "renamed", "--output", "json")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `failed to resolve service "does-not-exist"`)
+	assert.False(t, harness.server.HasRequest("PATCH", "/services/"))
+}
+
+func TestServiceUpdate_APIError_SurfacesWithoutWriting(t *testing.T) {
+	harness := newServiceUpdateHarness(t)
+	svc := harness.addWebService("api-error-service")
+
+	// The fake drains its error queue FIFO across all service requests, so a
+	// single queued error lands on the first GET the command issues to resolve
+	// and load the service — before any PATCH. This asserts the command
+	// propagates an unexpected API error instead of swallowing it, and does not
+	// write when a pre-PATCH step fails.
+	harness.server.Services.RespondWith(http.StatusInternalServerError)
+
+	_, err := harness.execute(svc.Id, "--name", "renamed", "--output", "json")
+	require.Error(t, err)
+	assert.False(t, harness.server.HasRequest("PATCH", "/services/"+svc.Id))
+}

internal/fakes/renderapi/server.go

@@ -1,8 +1,10 @@
 package renderapi
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"slices"
@@ -52,6 +54,9 @@ func writeJSON(w http.ResponseWriter, status int, v any) {
 type RecordedRequest struct {
 	Method string
 	URI    string
+	// Body is the raw request body bytes, captured before the route handler
+	// decodes them. Nil when the request had no body.
+	Body []byte
 }
 
 // Resource is a generic in-memory store for any fake API resource type.
@@ -281,7 +286,7 @@ type Server struct {
 	Environments *Resource[*client.Environment]
 	KV           *KVResource
 	Postgres     *PostgresResource
-	Services *ServiceResource
+	Services     *ServiceResource
 }
 
 // ownerByID returns the Owner with the given ID from the seeded owners. The
@@ -317,6 +322,20 @@ func (s *Server) HasRequest(method, uriSubstring string) bool {
 	return false
 }
 
+// LastRequest returns the most recent recorded request matching the given
+// method and URI substring, and whether one was found. Returning the last match
+// is robust when a command issues several requests (e.g. GETs to resolve a
+// service before the PATCH).
+func (s *Server) LastRequest(method, uriSubstring string) (RecordedRequest, bool) {
+	for i := len(s.Requests) - 1; i >= 0; i-- {
+		r := s.Requests[i]
+		if r.Method == method && strings.Contains(r.URI, uriSubstring) {
+			return r, true
+		}
+	}
+	return RecordedRequest{}, false
+}
+
 // HasDeleteRequest returns true if any recorded request used the DELETE method.
 func (s *Server) HasDeleteRequest() bool {
 	for _, r := range s.Requests {
@@ -344,7 +363,15 @@ func NewServer(t *testing.T) *Server {
 	mux := http.NewServeMux()
 
 	record := func(r *http.Request) {
-		s.Requests = append(s.Requests, RecordedRequest{Method: r.Method, URI: r.URL.RequestURI()})
+		rec := RecordedRequest{Method: r.Method, URI: r.URL.RequestURI()}
+		if r.Body != nil {
+			if body, err := io.ReadAll(r.Body); err == nil {
+				rec.Body = body
+				// Restore the body so route handlers can still decode it.
+				r.Body = io.NopCloser(bytes.NewReader(body))
+			}
+		}
+		s.Requests = append(s.Requests, rec)
 	}
 
 	// GET /users - retrieve the authenticated user.

pkg/service/repo.go

@@ -96,13 +96,11 @@ func (s *Repo) CreateService(ctx context.Context, data client.CreateServiceJSONR
 	return resp.JSON201.Service, nil
 }
 
+// UpdateService PATCHes the given service. It does not re-fetch the service to
+// validate workspace ownership, so the caller must pass an id already resolved
+// against the active workspace (GetService and ResolveServiceIDFromNameOrID
+// both validate ownership and return such an id).
 func (s *Repo) UpdateService(ctx context.Context, id string, data client.UpdateServiceJSONRequestBody) (*client.Service, error) {
-	// we get the Service to ensure the workspace matches. Since GetService checks the workspace, we just check
-	// if an error was returned
-	if _, err := s.GetService(ctx, id); err != nil {
-		return nil, err
-	}
-
 	resp, err := s.client.UpdateServiceWithResponse(ctx, id, data)
 	if err != nil {
 		return nil, err