diff --git a/spinifex/gateway/ecs/actions.go b/spinifex/gateway/ecs/actions.go index 81442096a..e46582a31 100644 --- a/spinifex/gateway/ecs/actions.go +++ b/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) } diff --git a/spinifex/gateway/ecs/azrebalancing.go b/spinifex/gateway/ecs/azrebalancing.go new file mode 100644 index 000000000..8da9a103a --- /dev/null +++ b/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 +} diff --git a/spinifex/gateway/ecs/azrebalancing_test.go b/spinifex/gateway/ecs/azrebalancing_test.go new file mode 100644 index 000000000..432f63da2 --- /dev/null +++ b/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]) +} diff --git a/spinifex/gateway/ecs/handler.go b/spinifex/gateway/ecs/handler.go index b0f6668c4..e57036e96 100644 --- a/spinifex/gateway/ecs/handler.go +++ b/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