From 7f349a8e3d6379177452be9ca25eee7c8ed26ecf Mon Sep 17 00:00:00 2001
From: Julian Sommer <115516918+juliansommer@users.noreply.github.com>
Date: Wed, 5 Aug 2026 11:00:15 +1000
Subject: [PATCH] fix: report RDS client errors accurately (#730)
---
spinifex/awserrors/awserrors.go | 3 ++
spinifex/awserrors/awserrors_test.go | 15 ++++++++++
spinifex/gateway/rds/handler.go | 2 +-
spinifex/gateway/rds/handler_test.go | 32 +++++++++++++++++++++
spinifex/gateway/rds_test.go | 35 +++++++++++++++++++++++
spinifex/handlers/rds/parametergroup.go | 6 ++--
spinifex/handlers/rds/subnetgroup.go | 10 ++++---
spinifex/handlers/rds/subnetgroup_test.go | 3 +-
8 files changed, 97 insertions(+), 9 deletions(-)
diff --git a/spinifex/awserrors/awserrors.go b/spinifex/awserrors/awserrors.go
index d2eebfc7b..2ee526fe6 100644
--- a/spinifex/awserrors/awserrors.go
+++ b/spinifex/awserrors/awserrors.go
@@ -644,6 +644,9 @@ var errorLookupByService = map[string]map[string]ErrorMessage{
"bedrock-runtime": {
ErrorResourceNotFoundException: {HTTPCode: 404, Message: bedrockResourceNotFoundMessage},
},
+ "rds": {
+ ErrorOperationNotSupported: {HTTPCode: 400, Message: "The specified RDS action is not supported in the RDS v1 API."},
+ },
}
// bedrockResourceNotFoundMessage overrides the EKS wording ErrorLookup carries
diff --git a/spinifex/awserrors/awserrors_test.go b/spinifex/awserrors/awserrors_test.go
index bae50f4e7..237457f7f 100644
--- a/spinifex/awserrors/awserrors_test.go
+++ b/spinifex/awserrors/awserrors_test.go
@@ -160,6 +160,21 @@ func TestLookupErrorMessage(t *testing.T) {
}
}
+func TestLookupErrorMessage_RDSOperationNotSupported(t *testing.T) {
+ got := LookupErrorMessage("rds", ErrorOperationNotSupported)
+ if got.HTTPCode != 400 {
+ t.Errorf("LookupErrorMessage(rds, OperationNotSupportedException).HTTPCode = %d, want 400", got.HTTPCode)
+ }
+ for _, want := range []string{"RDS", "v1"} {
+ if !strings.Contains(got.Message, want) {
+ t.Errorf("LookupErrorMessage(rds, OperationNotSupportedException) = %q, want %q", got.Message, want)
+ }
+ }
+ if strings.Contains(got.Message, "registry") {
+ t.Errorf("LookupErrorMessage(rds, OperationNotSupportedException) = %q, contains ECR wording", got.Message)
+ }
+}
+
// TestLookupErrorMessage_BedrockResourceNotFound guards the second shared-code
// collision: ResourceNotFoundException is an alias of the EKS code, so both
// Bedrock service keys must override it rather than inherit cluster wording.
diff --git a/spinifex/gateway/rds/handler.go b/spinifex/gateway/rds/handler.go
index 6ac9d0a38..d1fec09da 100644
--- a/spinifex/gateway/rds/handler.go
+++ b/spinifex/gateway/rds/handler.go
@@ -41,7 +41,7 @@ func typed[In any](handler func(context.Context, *In, *nats.Conn, Caller) (any,
if errors.Is(err, awsec2query.ErrSliceTooLarge) {
return nil, errors.New(awserrors.ErrorMalformedQueryString)
}
- return nil, err
+ return nil, errors.New(awserrors.ErrorInvalidParameterValue)
}
output, err := handler(ctx, input, nc, caller)
if err != nil {
diff --git a/spinifex/gateway/rds/handler_test.go b/spinifex/gateway/rds/handler_test.go
index 683c652a9..81de18b25 100644
--- a/spinifex/gateway/rds/handler_test.go
+++ b/spinifex/gateway/rds/handler_test.go
@@ -4,6 +4,7 @@ import (
"bytes"
"encoding/json"
"encoding/xml"
+ "strconv"
"testing"
"github.com/aws/aws-sdk-go/aws"
@@ -103,6 +104,37 @@ func TestDispatch_UnknownAction(t *testing.T) {
assert.Equal(t, awserrors.ErrorInvalidAction, err.Error())
}
+func TestDispatch_MalformedScalarIsInvalidParameterValue(t *testing.T) {
+ cases := []struct {
+ name string
+ action string
+ query map[string]string
+ }{
+ {name: "Integer", action: "CreateDBInstance", query: map[string]string{"AllocatedStorage": "abc"}},
+ {name: "Boolean", action: "CreateDBInstance", query: map[string]string{"MultiAZ": "yes"}},
+ {name: "Timestamp", action: "DescribeEvents", query: map[string]string{"StartTime": "not-a-time"}},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ _, err := Dispatch(t.Context(), tc.action, tc.query, nil, testCaller)
+ require.Error(t, err)
+ assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
+ })
+ }
+}
+
+func TestDispatch_OversizedListIsMalformedQueryString(t *testing.T) {
+ query := make(map[string]string, 1025)
+ for i := 1; i <= 1025; i++ {
+ query["Tags.Tag."+strconv.Itoa(i)+".Key"] = "key"
+ }
+
+ _, err := Dispatch(t.Context(), "CreateDBInstance", query, nil, testCaller)
+ require.Error(t, err)
+ assert.Equal(t, awserrors.ErrorMalformedQueryString, err.Error())
+}
+
// The customer actions this phase implements forward to the daemon, so they are
// dispatched against a stub responder rather than a nil connection.
var liveActions = []string{
diff --git a/spinifex/gateway/rds_test.go b/spinifex/gateway/rds_test.go
index f7259b3c8..bc7ad3383 100644
--- a/spinifex/gateway/rds_test.go
+++ b/spinifex/gateway/rds_test.go
@@ -11,6 +11,7 @@ import (
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds"
+ "github.com/mulgadc/spinifex/spinifex/testutil"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -103,6 +104,27 @@ func TestRDSRequest_MalformedQueryString(t *testing.T) {
assert.Equal(t, awserrors.ErrorMalformedQueryString, err.Error())
}
+func TestRDSRequest_MalformedScalarReturnsHTTP400(t *testing.T) {
+ _, nc, _ := testutil.StartTestJetStream(t)
+ gw := &GatewayConfig{DisableLogging: true, NATSConn: nc}
+ for _, body := range []string{
+ "Action=CreateDBInstance&AllocatedStorage=abc",
+ "Action=CreateDBInstance&MultiAZ=yes",
+ } {
+ t.Run(body, func(t *testing.T) {
+ req := setupRDSRequest(body)
+ err := gw.RDS_Request(httptest.NewRecorder(), req)
+ require.Error(t, err)
+ assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
+
+ w := httptest.NewRecorder()
+ gw.ErrorHandler(w, req, err)
+ assert.Equal(t, http.StatusBadRequest, w.Code)
+ assert.Contains(t, w.Body.String(), ""+awserrors.ErrorInvalidParameterValue+"")
+ })
+ }
+}
+
// A known action still needs a NATS connection to reach the control plane, so
// the disconnected case must fail rather than answer from the gateway alone.
func TestRDSRequest_NoNATSConn(t *testing.T) {
@@ -217,4 +239,17 @@ func TestRDSErrorHandler_UsesIAMEnvelope(t *testing.T) {
assert.Contains(t, body, ""+awserrors.ErrorNotImplemented+"")
}
+func TestRDSErrorHandler_UsesRDSUnsupportedActionWording(t *testing.T) {
+ gw := &GatewayConfig{DisableLogging: true}
+ w := httptest.NewRecorder()
+ gw.ErrorHandler(w, setupRDSRequest("Action=CreateDBCluster"),
+ errors.New(awserrors.ErrorOperationNotSupported))
+
+ body := w.Body.String()
+ assert.Equal(t, http.StatusBadRequest, w.Code)
+ assert.Contains(t, body, "RDS")
+ assert.Contains(t, body, "v1")
+ assert.NotContains(t, body, "registry")
+}
+
var errNotImplementedForTest = errors.New(awserrors.ErrorNotImplemented)
diff --git a/spinifex/handlers/rds/parametergroup.go b/spinifex/handlers/rds/parametergroup.go
index b40509d2a..39890b97a 100644
--- a/spinifex/handlers/rds/parametergroup.go
+++ b/spinifex/handlers/rds/parametergroup.go
@@ -31,15 +31,15 @@ func (s *Service) CreateDBParameterGroup(ctx context.Context, input *rds.CreateD
return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "empty request")
}
name := aws.StringValue(input.DBParameterGroupName)
- if err := validateDBGroupName("DBParameterGroupName", name); err != nil {
- return nil, err
- }
// Rejected rather than accepted-and-shadowed: a customer group under the
// reserved prefix would be indistinguishable from the implicit one.
if isDefaultParameterGroupName(name) {
return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
"DBParameterGroupName may not begin with %q, which the service reserves", defaultParameterGroupPrefix)
}
+ if err := validateDBGroupName("DBParameterGroupName", name); err != nil {
+ return nil, err
+ }
description := aws.StringValue(input.Description)
if err := validateDBGroupDescription("Description", description); err != nil {
return nil, err
diff --git a/spinifex/handlers/rds/subnetgroup.go b/spinifex/handlers/rds/subnetgroup.go
index 010d3dd7b..18728cfbf 100644
--- a/spinifex/handlers/rds/subnetgroup.go
+++ b/spinifex/handlers/rds/subnetgroup.go
@@ -308,10 +308,12 @@ func validateDBGroupName(field, name string) error {
case strings.EqualFold(name, "default"):
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
"%s may not be \"default\", which the service reserves", field)
- // A slash would split the name across the KV key layout's own separator, so
- // one group's parameters would land under another's prefix.
- case strings.ContainsAny(name, "/ \t\n"):
- return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "%s may not contain slashes or whitespace", field)
+ }
+ for _, r := range name {
+ if !isLetter(r) && !isDigit(r) && r != '-' {
+ return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
+ "%s may contain only letters, digits and hyphens", field)
+ }
}
return nil
}
diff --git a/spinifex/handlers/rds/subnetgroup_test.go b/spinifex/handlers/rds/subnetgroup_test.go
index 58bd083ef..1413bbc6f 100644
--- a/spinifex/handlers/rds/subnetgroup_test.go
+++ b/spinifex/handlers/rds/subnetgroup_test.go
@@ -116,8 +116,9 @@ func TestCreateDBSubnetGroup_RejectsMalformedRequests(t *testing.T) {
{"NoName", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupName = nil }, awserrors.ErrorInvalidParameterValue},
{"ReservedName", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupName = aws.String("default") }, awserrors.ErrorInvalidParameterValue},
{"LeadingDigit", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupName = aws.String("1group") }, awserrors.ErrorInvalidParameterValue},
- // A slash would split the name across the KV layout's own separator.
{"NameWithSlash", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupName = aws.String("db/private") }, awserrors.ErrorInvalidParameterValue},
+ {"NameWithPlus", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupName = aws.String("db+private") }, awserrors.ErrorInvalidParameterValue},
+ {"NameWithParentheses", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupName = aws.String("db(1)") }, awserrors.ErrorInvalidParameterValue},
{"NoDescription", func(in *rds.CreateDBSubnetGroupInput) { in.DBSubnetGroupDescription = nil }, awserrors.ErrorInvalidParameterValue},
{"NoSubnets", func(in *rds.CreateDBSubnetGroupInput) { in.SubnetIds = nil }, awserrors.ErrorInvalidParameterValue},
{"DuplicateSubnet", func(in *rds.CreateDBSubnetGroupInput) {