fix: report RDS client errors accurately (#730)

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

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.

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 {

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{

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(), "<Code>"+awserrors.ErrorInvalidParameterValue+"</Code>")
+		})
+	}
+}
+
 // 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, "<Code>"+awserrors.ErrorNotImplemented+"</Code>")
 }
 
+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)

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

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
 }

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) {