Improve JSON output for kv update (#475)

- Creates a struct for the JSON/YAML output shape for `render ea kv update` (before we were returning the full data we got back from the API)
  - Returns the updated KV under `data`.
  - Returns a field-level `diff` for updateable fields so scripting users can inspect what changed.
- Refactor: Moves IP allow-list equality into `internal/ipallowlist` so text and KV update diffing can share the same comparison.

---

Make an _explicit_ contract for JSON output shape for kv. We take these contracts seriously after all!

Unlike `get` and `resume`, `update` has some useful operation-specific metadata: the before/after diff.

That diff is helpful for humans in text output and also useful for scripts, so this PR makes it part of the JSON contract instead of leaving it as text-only formatter logic.

GROW-2587: https://linear.app/render-com/issue/GROW-2587/slim-down-kv-json-output-to-resource-only
GitOrigin-RevId: e806c2345bdd8c90f0951466bfb36f7e84e88223

Author: davidharting

cmd/kvupdate.go

@@ -6,9 +6,9 @@ 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"
+	"github.com/render-oss/cli/pkg/keyvalue"
 	"github.com/render-oss/cli/pkg/text"
 	kvtypes "github.com/render-oss/cli/pkg/types/keyvalue"
 )
@@ -89,22 +89,17 @@ Example: --ip-allow-list "cidr=203.0.113.5/32,description=office"`
 			return err
 		}
 
-		// Captured by both closures so the text formatter can render a diff
-		// while JSON/YAML output stays simple — just the new KV state, matching
-		// kv create's output shape.
-		var before *client.KeyValueDetail
-
 		_, err := command.NonInteractive(cmd,
-			func() (*client.KeyValueDetail, error) {
+			func() (*keyvalue.KeyValueUpdateOut, error) {
 				result, err := deps.KeyValueService().Update(cmd.Context(), input)
 				if err != nil {
 					return nil, err
 				}
-				before = result.Before
-				return result.After, nil
+				out := keyvalue.NewKeyValueUpdateOut(result.Before, result.After)
+				return &out, nil
 			},
-			func(after *client.KeyValueDetail) string {
-				return kvUpdateSuccessMessage(before, after)
+			func(out *keyvalue.KeyValueUpdateOut) string {
+				return kvUpdateSuccessMessage(out)
 			},
 		)
 		return err
@@ -113,9 +108,9 @@ Example: --ip-allow-list "cidr=203.0.113.5/32,description=office"`
 	return cmd
 }
 
-func kvUpdateSuccessMessage(before, after *client.KeyValueDetail) string {
-	details := "Full details:\n  " + strings.ReplaceAll(text.KeyValueAPIDetail(after), "\n", "\n  ")
-	diff := text.KeyValueUpdateDiff(before, after)
+func kvUpdateSuccessMessage(out *keyvalue.KeyValueUpdateOut) string {
+	details := "Full details:\n  " + strings.ReplaceAll(text.KeyValueDetail(&out.Data), "\n", "\n  ")
+	diff := text.KeyValueUpdateDiff(out.Diff)
 	if diff == "" {
 		return fmt.Sprintf("No changes applied to Key Value\n\n%s\n", details)
 	}

cmd/kvupdate_test.go

@@ -180,19 +180,40 @@ func TestKVUpdate_IPAllowListFlags_MutuallyExclusive(t *testing.T) {
 // --- Output formats and API errors ---
 
 // TestKVUpdate_OutputJSON proves --output json produces valid JSON of the
-// post-update KV (matching the shape `kv create` returns), not the pre/post
-// diff structure that text output renders.
+// post-update KV plus the public diff contract for fields update can mutate.
 func TestKVUpdate_OutputJSON(t *testing.T) {
 	server := renderapi.NewServer(t)
-	kv := seedKV(server, "before-name")
+	project := server.CreateProject(
+		renderapi.ProjectAttrs{Name: "My Project", OwnerId: kvTestWorkspaceID},
+		renderapi.EnvAttrs{Name: "production"},
+	)
+	env := project.Env("production")
+	kv := seedKVInEnv(server, "before-name", env.Id)
 
 	result, err := executeKVUpdate(t, server, kv.Id, "--name", "after-name", "--output", "json")
 	require.NoError(t, err)
 
-	var body map[string]interface{}
+	var body map[string]any
 	require.NoError(t, json.Unmarshal([]byte(result.Stdout), &body),
 		"expected valid JSON, got: %s", result.Stdout)
-	assert.Equal(t, "after-name", body["name"])
+
+	data := requireSubMap(t, body, "data")
+	assert.Equal(t, "after-name", data["name"])
+	assert.Equal(t, kv.Id, data["id"])
+	assert.Equal(t, kvTestWorkspaceID, data["ownerId"])
+	assert.Equal(t, project.Project.Id, data["projectId"])
+	assert.Equal(t, env.Id, data["environmentId"])
+
+	diff := requireSubMap(t, body, "diff")
+	nameDiff := requireSubMap(t, diff, "name")
+	assert.Equal(t, "before-name", nameDiff["before"])
+	assert.Equal(t, "after-name", nameDiff["after"])
+	assert.NotContains(t, diff, "plan")
+	assert.NotContains(t, diff, "maxmemoryPolicy")
+	assert.NotContains(t, diff, "ipAllowList")
+
+	assert.NotContains(t, body, "before")
+	assert.NotContains(t, body, "after")
 }
 
 // TestKVUpdate_APIError_Propagates proves a 5xx from the PATCH surfaces as

internal/ipallowlist/ipallowlist.go

@@ -0,0 +1,28 @@
+package ipallowlist
+
+import (
+	"cmp"
+	"slices"
+
+	"github.com/render-oss/cli/pkg/client"
+)
+
+// Equal reports whether two allow-lists contain the same entries.
+// An allow-list is conceptually a set, so the comparison is order-insensitive:
+// it sorts copies before comparing, never the caller's live slices.
+func Equal(a, b []client.CidrBlockAndDescription) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	as := slices.Clone(a)
+	bs := slices.Clone(b)
+	byKey := func(x, y client.CidrBlockAndDescription) int {
+		return cmp.Or(
+			cmp.Compare(x.CidrBlock, y.CidrBlock),
+			cmp.Compare(x.Description, y.Description),
+		)
+	}
+	slices.SortFunc(as, byKey)
+	slices.SortFunc(bs, byKey)
+	return slices.Equal(as, bs)
+}

pkg/text/ipallowlist_test.go internal/ipallowlist/ipallowlist_test.gopkg/text/ipallowlist_test.go renamed to internal/ipallowlist/ipallowlist_test.go

@@ -1,4 +1,4 @@
-package text
+package ipallowlist
 
 import (
 	"testing"
@@ -7,38 +7,38 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestIPAllowListEqual(t *testing.T) {
+func TestEqual(t *testing.T) {
 	office := client.CidrBlockAndDescription{CidrBlock: "203.0.113.5/32", Description: "office"}
 	internal := client.CidrBlockAndDescription{CidrBlock: "10.0.0.0/8", Description: "internal"}
 
 	t.Run("same entries, same order", func(t *testing.T) {
 		a := []client.CidrBlockAndDescription{office, internal}
 		b := []client.CidrBlockAndDescription{office, internal}
-		assert.True(t, ipAllowListEqual(a, b))
+		assert.True(t, Equal(a, b))
 	})
 
 	t.Run("same entries, different order", func(t *testing.T) {
 		a := []client.CidrBlockAndDescription{office, internal}
 		b := []client.CidrBlockAndDescription{internal, office}
-		assert.True(t, ipAllowListEqual(a, b))
+		assert.True(t, Equal(a, b))
 	})
 
 	t.Run("different entries", func(t *testing.T) {
 		a := []client.CidrBlockAndDescription{office}
 		b := []client.CidrBlockAndDescription{internal}
-		assert.False(t, ipAllowListEqual(a, b))
+		assert.False(t, Equal(a, b))
 	})
 
 	t.Run("different lengths", func(t *testing.T) {
 		a := []client.CidrBlockAndDescription{office, internal}
 		b := []client.CidrBlockAndDescription{office}
-		assert.False(t, ipAllowListEqual(a, b))
+		assert.False(t, Equal(a, b))
 	})
 
 	t.Run("does not mutate the caller's slices", func(t *testing.T) {
 		a := []client.CidrBlockAndDescription{office, internal}
 		b := []client.CidrBlockAndDescription{internal, office}
-		ipAllowListEqual(a, b)
+		Equal(a, b)
 		assert.Equal(t, []client.CidrBlockAndDescription{office, internal}, a)
 		assert.Equal(t, []client.CidrBlockAndDescription{internal, office}, b)
 	})

pkg/keyvalue/output.go

@@ -47,6 +47,23 @@ type SuspendOutMeta struct {
 	Message   string `json:"message,omitempty"`
 }
 
+type KeyValueUpdateOut struct {
+	Data KeyValueOut        `json:"data"`
+	Diff KeyValueUpdateDiff `json:"diff"`
+}
+
+type KeyValueUpdateDiff struct {
+	Name            *KeyValueFieldDiff[string]                           `json:"name,omitempty"`
+	Plan            *KeyValueFieldDiff[client.KeyValuePlan]              `json:"plan,omitempty"`
+	MaxmemoryPolicy *KeyValueFieldDiff[*string]                          `json:"maxmemoryPolicy,omitempty"`
+	IPAllowList     *KeyValueFieldDiff[[]client.CidrBlockAndDescription] `json:"ipAllowList,omitempty"`
+}
+
+type KeyValueFieldDiff[T any] struct {
+	Before T `json:"before"`
+	After  T `json:"after"`
+}
+
 func NewKeyValueOut(resolved *ResolvedKeyValue) KeyValueOut {
 	if resolved == nil || resolved.KeyValue == nil {
 		return KeyValueOut{}

pkg/keyvalue/service.go

@@ -104,7 +104,7 @@ func (s *Service) Resume(ctx context.Context, id string) error {
 	return s.repo.ResumeKeyValue(ctx, id)
 }
 
-func (s *Service) Update(ctx context.Context, input kvtypes.KeyValueUpdateInput) (*UpdateResult, error) {
+func (s *Service) Update(ctx context.Context, input kvtypes.KeyValueUpdateInput) (*UpdateOutcome, error) {
 	normalized, err := kvtypes.NormalizeAndValidateUpdateInput(input)
 	if err != nil {
 		return nil, err
@@ -127,7 +127,14 @@ func (s *Service) Update(ctx context.Context, input kvtypes.KeyValueUpdateInput)
 	if err != nil {
 		return nil, err
 	}
-	return &UpdateResult{Before: before.KeyValue, After: after}, nil
+	return &UpdateOutcome{
+		Before: before.KeyValue,
+		After: &ResolvedKeyValue{
+			KeyValue:    after,
+			Project:     before.Project,
+			Environment: before.Environment,
+		},
+	}, nil
 }
 
 func (s *Service) hydrateKeyValueModel(ctx context.Context, kv *client.KeyValue, projects []*client.Project) (*Model, error) {

pkg/keyvalue/update.go

@@ -1,17 +1,56 @@
 package keyvalue
 
 import (
+	"github.com/render-oss/cli/internal/ipallowlist"
 	"github.com/render-oss/cli/pkg/client"
+	"github.com/render-oss/cli/pkg/pointers"
 	"github.com/render-oss/cli/pkg/types"
 	kvtypes "github.com/render-oss/cli/pkg/types/keyvalue"
 )
 
-// UpdateResult captures both the pre- and post-update KV state so callers can
-// show users a diff of what changed. JSON/YAML consumers also get strictly
-// more information than just the new state.
-type UpdateResult struct {
-	Before *client.KeyValueDetail `json:"before"`
-	After  *client.KeyValueDetail `json:"after"`
+// UpdateOutcome captures both the pre-update API detail and post-update
+// resolved state needed by command output.
+type UpdateOutcome struct {
+	Before *client.KeyValueDetail
+	After  *ResolvedKeyValue
+}
+
+func NewKeyValueUpdateOut(before *client.KeyValueDetail, after *ResolvedKeyValue) KeyValueUpdateOut {
+	out := KeyValueUpdateOut{
+		Data: NewKeyValueOut(after),
+	}
+	if before == nil {
+		return out
+	}
+	out.Diff = NewKeyValueUpdateDiff(before, &out.Data)
+	return out
+}
+
+func NewKeyValueUpdateDiff(before *client.KeyValueDetail, after *KeyValueOut) KeyValueUpdateDiff {
+	var diff KeyValueUpdateDiff
+	if before.Name != after.Name {
+		diff.Name = newKeyValueFieldDiff(before.Name, after.Name)
+	}
+	if before.Plan != after.Plan {
+		diff.Plan = newKeyValueFieldDiff(before.Plan, after.Plan)
+	}
+	if !pointers.Equal(before.Options.MaxmemoryPolicy, after.MaxmemoryPolicy) {
+		diff.MaxmemoryPolicy = newKeyValueFieldDiff(
+			before.Options.MaxmemoryPolicy,
+			after.MaxmemoryPolicy,
+		)
+	}
+	if !ipallowlist.Equal(before.IpAllowList, after.IPAllowList) {
+		diff.IPAllowList = newKeyValueFieldDiff(before.IpAllowList, after.IPAllowList)
+	}
+	return diff
+}
+
+func newKeyValueFieldDiff[T any](before, after T) *KeyValueFieldDiff[T] {
+	return &KeyValueFieldDiff[T]{
+		Before: before,
+		After:  after,
+	}
 }
 
 // BuildUpdateRequest converts a normalized KeyValueUpdateInput into the API

pkg/pointers/pointers.go

@@ -21,6 +21,16 @@ func ValueOrDefault[T any](x *T, def T) T {
 	return *x
 }
 
+// Equal reports whether two pointers are both nil or point to equal values.
+// It compares the pointed-to values, not the pointer addresses. T must be
+// comparable because Equal uses == after dereferencing non-nil pointers.
+func Equal[T comparable](a, b *T) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+	return *a == *b
+}
+
 func PointerValueIfNotEmptyString(s string) *string {
 	if s == "" {
 		return nil

pkg/pointers/pointers_test.go

@@ -0,0 +1,32 @@
+package pointers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestEqual(t *testing.T) {
+	value := "value"
+	same := "value"
+	other := "other"
+
+	tests := []struct {
+		name string
+		a    *string
+		b    *string
+		want bool
+	}{
+		{name: "both nil", want: true},
+		{name: "left nil", b: &value, want: false},
+		{name: "right nil", a: &value, want: false},
+		{name: "same value", a: &value, b: &same, want: true},
+		{name: "different value", a: &value, b: &other, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.want, Equal(tt.a, tt.b))
+		})
+	}
+}

pkg/text/ipallowlist.go

@@ -1,9 +1,7 @@
 package text
 
 import (
-	"cmp"
 	"fmt"
-	"slices"
 	"strings"
 
 	"github.com/render-oss/cli/pkg/client"
@@ -42,24 +40,3 @@ func ipAllowListLabel(entries []client.CidrBlockAndDescription) string {
 		return fmt.Sprintf("%d entries", len(entries))
 	}
 }
-
-// ipAllowListEqual reports whether two allow-lists contain the same entries.
-// An allow-list is conceptually a set, so the comparison is order-insensitive:
-// it sorts copies (never the caller's slices, which are live server data)
-// before comparing, so a reordered list is not reported as a change.
-func ipAllowListEqual(a, b []client.CidrBlockAndDescription) bool {
-	if len(a) != len(b) {
-		return false
-	}
-	as := slices.Clone(a)
-	bs := slices.Clone(b)
-	byKey := func(x, y client.CidrBlockAndDescription) int {
-		return cmp.Or(
-			cmp.Compare(x.CidrBlock, y.CidrBlock),
-			cmp.Compare(x.Description, y.Description),
-		)
-	}
-	slices.SortFunc(as, byKey)
-	slices.SortFunc(bs, byKey)
-	return slices.Equal(as, bs)
-}