From f1f194b16a69d058438b2d976b791a055598cd06 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Thu, 8 Oct 2026 14:43:25 +1000 Subject: [PATCH] fix: return InvalidParameterValue for a missing RDS master password ValidateMasterUserPassword built the empty-password error with a bare errors.New carrying the code only in its text, which awserrors does not resolve, so CreateDBInstance without MasterUserPassword returned HTTP 500 ServerInternal. Build it with awserrors.Errorf like the other branches so the gateway returns 400 InvalidParameterValue, and cover the full gateway to RDS service path with a regression test. --- spinifex/gateway/rds_test.go | 31 +++++++++++++++++++ spinifex/handlers/rds/engine.go | 2 +- .../engine_catalogue_characterization_test.go | 14 +++------ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/spinifex/gateway/rds_test.go b/spinifex/gateway/rds_test.go index b9f16f074..a205f4435 100644 --- a/spinifex/gateway/rds_test.go +++ b/spinifex/gateway/rds_test.go @@ -9,10 +9,13 @@ import ( "strings" "testing" + "github.com/aws/aws-sdk-go/service/rds" "github.com/mulgadc/spinifex/internal/testkit" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + natsmsg "github.com/mulgadc/spinifex/spinifex/foundation/messaging/nats" handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" + "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -361,3 +364,31 @@ func TestRDSErrorHandler_UsesRDSUnsupportedActionWording(t *testing.T) { } var errNotImplementedForTest = errors.New(awserrors.ErrorNotImplemented) + +// TestRDSRequest_CreateDBInstance_EmptyMasterUserPasswordReturns400 runs the real public path: +// a CreateDBInstance query request through RDS_Request, over NATS, into the production +// handlers_rds.Service, and the resulting error through ErrorHandler's HTTP/XML rendering. AWS +// returns 400 InvalidParameterValue for a create with no MasterUserPassword. +func TestRDSRequest_CreateDBInstance_EmptyMasterUserPasswordReturns400(t *testing.T) { + _, nc, _ := testutil.StartTestJetStream(t) + svc := handlers_rds.NewService(nc, "ap-southeast-2") + sub, err := nc.Subscribe(handlers_rds.SubjectCreateDBInstance, func(msg *nats.Msg) { + natsmsg.ServeNATSRequestCtx(msg, func(ctx context.Context, in *rds.CreateDBInstanceInput) (*rds.CreateDBInstanceOutput, error) { + return svc.CreateDBInstance(ctx, in, natsmsg.AccountIDFromMsg(msg)) + }) + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + gw := &GatewayConfig{DisableLogging: true, NATSConn: nc, IAMService: allowAllIAMService()} + req := setupRDSRequest("Action=CreateDBInstance&DBInstanceIdentifier=orders-db&Engine=postgres&" + + "DBInstanceClass=db.t3.medium&AllocatedStorage=20&MasterUsername=appuser&DBName=orders") + + reqErr := gw.RDS_Request(httptest.NewRecorder(), req) + require.Error(t, reqErr) + + w := httptest.NewRecorder() + gw.ErrorHandler(w, req, reqErr) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), ""+awserrors.ErrorInvalidParameterValue+"") +} diff --git a/spinifex/handlers/rds/engine.go b/spinifex/handlers/rds/engine.go index fceb26496..41fa214f6 100644 --- a/spinifex/handlers/rds/engine.go +++ b/spinifex/handlers/rds/engine.go @@ -255,7 +255,7 @@ func validateIdentifier(field, value string, maxLen int, allowEmpty bool) error func ValidateMasterUserPassword(password string) error { switch { case password == "": - return errors.New(awserrors.ErrorInvalidParameterValue + ": MasterUserPassword is required") + return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "MasterUserPassword is required") case len(password) < minMasterPasswordLen || len(password) > maxMasterPasswordLen: return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "MasterUserPassword must be between %d and %d characters", minMasterPasswordLen, maxMasterPasswordLen) diff --git a/spinifex/handlers/rds/engine_catalogue_characterization_test.go b/spinifex/handlers/rds/engine_catalogue_characterization_test.go index 66929f622..0721f9fbb 100644 --- a/spinifex/handlers/rds/engine_catalogue_characterization_test.go +++ b/spinifex/handlers/rds/engine_catalogue_characterization_test.go @@ -56,21 +56,15 @@ func TestEngineValidateVersion_UnsupportedVersionCarriesExactCode(t *testing.T) assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) } -// ValidateMasterUserPassword's three rejections are built two different ways -// today: the length and character checks go through awserrors.Errorf, whose -// wrapped code resolves through ValidErrorCodeFromError; the empty-password -// check is a bare errors.New with the code only prefixed into the message -// text, which ValidErrorCodeFromError's exact-match lookup does not resolve. -// This pins that divergence rather than papering over it. +// ValidateMasterUserPassword's three rejections all go through awserrors.Errorf, +// so each one resolves through ValidErrorCodeFromError to the same code. func TestValidateMasterUserPassword_ExactCodesAndMessages(t *testing.T) { t.Parallel() empty := ValidateMasterUserPassword("") require.Error(t, empty) - assert.Equal(t, "InvalidParameterValue: MasterUserPassword is required", empty.Error()) - assert.Equal(t, awserrors.ErrorServerInternal, awserrors.ValidErrorCodeFromError(empty), - "today this does NOT resolve to InvalidParameterValue: the code is only prefixed into the "+ - "message text, not carried as a wrapped codedError, so the helper's exact-match lookup misses it") + assert.Equal(t, "MasterUserPassword is required: InvalidParameterValue", empty.Error()) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(empty)) tooShort := ValidateMasterUserPassword("short1") require.Error(t, tooShort)