feat: report availabilityZoneRebalancing on ECS services (#1055)

DescribeServices returned no availabilityZoneRebalancing field, so the terraform provider read the computed attribute as empty and planned an in-place update to DISABLED on every plan, including one taken immediately after a clean apply.

The pinned aws-sdk-go v1.55.8 is the last v1 release and its ECS shapes predate the field, so it can be neither parsed from an input nor set on an output through the SDK types. Embedding ecs.Service in a wrapper does not work either: jsonutil nests an embedded struct rather than flattening it, so the service arrives under its own key.

The field is read off the raw request body and written onto the encoded response instead. checkAZRebalancing refuses anything but DISABLED with InvalidParameterException, because nothing here rebalances tasks across zones and a silently dropped ENABLED would report a service as rebalancing when nothing does. addSDKGapFields runs from WriteJSONResponse so every service-carrying action is covered from one place, decodes with UseNumber to keep the epoch-second times jsonutil emitted, and leaves a value already present alone.

Closes mulga-t239t.

spinifex/gateway/ecs/actions.go

@@ -297,6 +297,9 @@ func CreateService(ctx context.Context, nc *nats.Conn, accountID string, body []
 	if err := unmarshalIfBody(body, input); err != nil {
 		return nil, err
 	}
+	if err := checkAZRebalancing(body); err != nil {
+		return nil, err
+	}
 	return createService(ctx, handlers_ecs.NewNATSECSService(nc), accountID, input, passRoleCheck)
 }
 
@@ -314,6 +317,9 @@ func UpdateService(ctx context.Context, nc *nats.Conn, accountID string, body []
 	if err := unmarshalIfBody(body, input); err != nil {
 		return nil, err
 	}
+	if err := checkAZRebalancing(body); err != nil {
+		return nil, err
+	}
 	return updateService(ctx, handlers_ecs.NewNATSECSService(nc), accountID, input, passRoleCheck)
 }
 

spinifex/gateway/ecs/azrebalancing.go

@@ -0,0 +1,103 @@
+package gateway_ecs
+
+import (
+	"bytes"
+	"encoding/json"
+	"log/slog"
+
+	"github.com/aws/aws-sdk-go/service/ecs"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+)
+
+// availabilityZoneRebalancing is a field the pinned aws-sdk-go's ECS shapes
+// predate: v1.55.8 is the last v1 release and carries no such member, so it can
+// be neither parsed into an input nor set on an output through the SDK types.
+// It is read off the raw request body and written onto the encoded response
+// instead.
+const availabilityZoneRebalancingField = "availabilityZoneRebalancing"
+
+// azRebalancingDisabled is the only value this platform can report honestly.
+// Nothing here rebalances tasks across availability zones, so a service always
+// reads DISABLED and a request to enable it is refused rather than stored.
+const azRebalancingDisabled = "DISABLED"
+
+// checkAZRebalancing refuses a CreateService or UpdateService asking for
+// availability-zone rebalancing. Terraform sends the attribute on every apply
+// once it is in state, so silently dropping ENABLED would report a service as
+// rebalancing when nothing does.
+func checkAZRebalancing(body []byte) error {
+	if len(body) == 0 {
+		return nil
+	}
+	var req struct {
+		AvailabilityZoneRebalancing *string `json:"availabilityZoneRebalancing"`
+	}
+	if err := json.Unmarshal(body, &req); err != nil {
+		// A body the SDK shapes will reject anyway; let that error be the one
+		// the caller sees rather than a second opinion from here.
+		return nil //nolint:nilerr // unmarshalIfBody reports the real parse error
+	}
+	if req.AvailabilityZoneRebalancing == nil || *req.AvailabilityZoneRebalancing == azRebalancingDisabled {
+		return nil
+	}
+	return awserrors.Errorf(awserrors.ErrorECSInvalidParameter,
+		"availabilityZoneRebalancing %s cannot be applied (no cross-zone rebalancer)",
+		*req.AvailabilityZoneRebalancing)
+}
+
+// addSDKGapFields re-encodes a service-carrying response with the fields the
+// pinned SDK's shapes do not define. Embedding ecs.Service in a wrapper does not
+// work: jsonutil nests an embedded struct rather than flattening it, so the
+// service would arrive under its own key. Mirroring the whole shape to carry one
+// string would duplicate forty fields, so the field is added to the encoded body.
+// A response carrying no service is returned untouched.
+func addSDKGapFields(obj any, body []byte) []byte {
+	var key string
+	switch obj.(type) {
+	case *ecs.DescribeServicesOutput:
+		key = "services"
+	case *ecs.CreateServiceOutput, *ecs.UpdateServiceOutput, *ecs.DeleteServiceOutput:
+		key = "service"
+	default:
+		return body
+	}
+
+	// UseNumber keeps the epoch-second times jsonutil emitted as they were
+	// written; decoding them into float64 would re-encode them in exponent form.
+	dec := json.NewDecoder(bytes.NewReader(body))
+	dec.UseNumber()
+	var doc map[string]any
+	if err := dec.Decode(&doc); err != nil {
+		slog.Error("ECS: failed to decode response for field injection", "err", err)
+		return body
+	}
+
+	switch v := doc[key].(type) {
+	case map[string]any:
+		setAZRebalancing(v)
+	case []any:
+		for _, item := range v {
+			if svc, ok := item.(map[string]any); ok {
+				setAZRebalancing(svc)
+			}
+		}
+	default:
+		return body
+	}
+
+	patched, err := json.Marshal(doc)
+	if err != nil {
+		slog.Error("ECS: failed to re-encode response after field injection", "err", err)
+		return body
+	}
+	return patched
+}
+
+// setAZRebalancing adds the field to one service object, leaving a value the
+// daemon already supplied alone so this never overwrites a real answer.
+func setAZRebalancing(svc map[string]any) {
+	if _, ok := svc[availabilityZoneRebalancingField]; ok {
+		return
+	}
+	svc[availabilityZoneRebalancingField] = azRebalancingDisabled
+}

spinifex/gateway/ecs/azrebalancing_test.go

@@ -0,0 +1,130 @@
+package gateway_ecs
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/private/protocol/json/jsonutil"
+	"github.com/aws/aws-sdk-go/service/ecs"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// A caller asking for nothing, or asking for the only value this platform can
+// honour, is accepted. Anything else is refused rather than dropped, since a
+// dropped ENABLED would report a service as rebalancing when nothing does.
+func TestCheckAZRebalancing(t *testing.T) {
+	cases := []struct {
+		name    string
+		body    string
+		wantErr bool
+	}{
+		{name: "empty body", body: ""},
+		{name: "field absent", body: `{"serviceName":"svc"}`},
+		{name: "disabled", body: `{"availabilityZoneRebalancing":"DISABLED"}`},
+		{name: "unparseable body defers to the SDK shapes", body: `{`},
+		{name: "enabled", body: `{"availabilityZoneRebalancing":"ENABLED"}`, wantErr: true},
+		{name: "unknown value", body: `{"availabilityZoneRebalancing":"SOMETHING"}`, wantErr: true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := checkAZRebalancing([]byte(tc.body))
+			if !tc.wantErr {
+				require.NoError(t, err)
+				return
+			}
+			require.Error(t, err)
+			code, message, ok := awserrors.ResolveErrorDetail(err)
+			require.True(t, ok)
+			assert.Equal(t, awserrors.ErrorECSInvalidParameter, code)
+			assert.Contains(t, message, "availabilityZoneRebalancing")
+		})
+	}
+}
+
+// Every service in a DescribeServices response carries the field, so a plan
+// taken straight after an apply reads the value it wrote rather than an empty
+// one and stops proposing an update.
+func TestAddSDKGapFields_DescribeServices(t *testing.T) {
+	out := &ecs.DescribeServicesOutput{Services: []*ecs.Service{
+		{ServiceName: aws.String("a")},
+		{ServiceName: aws.String("b")},
+	}}
+	body, err := jsonutil.BuildJSON(out)
+	require.NoError(t, err)
+
+	var doc struct {
+		Services []map[string]any `json:"services"`
+	}
+	require.NoError(t, json.Unmarshal(addSDKGapFields(out, body), &doc))
+	require.Len(t, doc.Services, 2)
+	for _, svc := range doc.Services {
+		assert.Equal(t, azRebalancingDisabled, svc[availabilityZoneRebalancingField])
+	}
+}
+
+// The single-service responses carry the field under "service" rather than
+// "services".
+func TestAddSDKGapFields_SingleService(t *testing.T) {
+	for _, out := range []any{
+		&ecs.CreateServiceOutput{Service: &ecs.Service{ServiceName: aws.String("a")}},
+		&ecs.UpdateServiceOutput{Service: &ecs.Service{ServiceName: aws.String("a")}},
+		&ecs.DeleteServiceOutput{Service: &ecs.Service{ServiceName: aws.String("a")}},
+	} {
+		body, err := jsonutil.BuildJSON(out)
+		require.NoError(t, err)
+
+		var doc struct {
+			Service map[string]any `json:"service"`
+		}
+		require.NoError(t, json.Unmarshal(addSDKGapFields(out, body), &doc))
+		assert.Equal(t, azRebalancingDisabled, doc.Service[availabilityZoneRebalancingField])
+	}
+}
+
+// A response carrying no service is returned byte for byte, so nothing else on
+// the ECS surface pays for this.
+func TestAddSDKGapFields_LeavesOtherResponsesAlone(t *testing.T) {
+	out := &ecs.ListServicesOutput{ServiceArns: aws.StringSlice([]string{"arn:aws:ecs:::service/a"})}
+	body, err := jsonutil.BuildJSON(out)
+	require.NoError(t, err)
+	assert.Equal(t, string(body), string(addSDKGapFields(out, body)))
+}
+
+// The re-encode must not rewrite the epoch-second times jsonutil emitted: a
+// float64 round trip renders them in exponent form, which the SDK clients then
+// read back as a different instant.
+func TestAddSDKGapFields_PreservesTimeEncoding(t *testing.T) {
+	out := &ecs.DescribeServicesOutput{Services: []*ecs.Service{
+		{ServiceName: aws.String("a"), CreatedAt: aws.Time(time.Unix(1757980800, 0).UTC())},
+	}}
+	body, err := jsonutil.BuildJSON(out)
+	require.NoError(t, err)
+
+	var before struct {
+		Services []map[string]json.RawMessage `json:"services"`
+	}
+	require.NoError(t, json.Unmarshal(body, &before))
+	var after struct {
+		Services []map[string]json.RawMessage `json:"services"`
+	}
+	require.NoError(t, json.Unmarshal(addSDKGapFields(out, body), &after))
+
+	assert.Equal(t, string(before.Services[0]["createdAt"]), string(after.Services[0]["createdAt"]))
+}
+
+// A value the daemon already supplied wins, so this never overwrites a real
+// answer once the field has a source.
+func TestAddSDKGapFields_KeepsAnExistingValue(t *testing.T) {
+	out := &ecs.DescribeServicesOutput{}
+	body := []byte(`{"services":[{"serviceName":"a","availabilityZoneRebalancing":"ENABLED"}]}`)
+
+	var doc struct {
+		Services []map[string]any `json:"services"`
+	}
+	require.NoError(t, json.Unmarshal(addSDKGapFields(out, body), &doc))
+	assert.Equal(t, "ENABLED", doc.Services[0][availabilityZoneRebalancingField])
+}

spinifex/gateway/ecs/handler.go

@@ -141,7 +141,7 @@ func WriteJSONResponse(w http.ResponseWriter, obj any) {
 		http.Error(w, awserrors.ErrorInternalError, http.StatusInternalServerError)
 		return
 	}
-	writeJSONBody(w, body)
+	writeJSONBody(w, addSDKGapFields(obj, body))
 }
 
 // WriteRawJSONResponse serialises obj with encoding/json (RFC3339 times) for the