mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-20 11:21:49 +00:00
fix: carry a handler's refusal message to the client without leaking wrapper context (#1036)
* fix: carry a handler's own refusal message to the client without leaking wrapper context A daemon that explains a refusal had that explanation discarded one step short of the client, so every NATS-backed error surfaced as the generic ErrorLookup blurb for its code. The message crossed NATS intact; resolveErrorDetail then dropped it because decodedNATSError was not the shape it recognises. Rebuild the decoded response through awserrors.Errorf so the walk finds the message, and send only ResolveErrorDetail's curated message from the two encode sites instead of err.Error(). The latter rendered every %w layer, including context a call site added purely for its own logs, so restoring the message without also fixing the encode side would have turned a fidelity bug into an information disclosure. Two existing tests pinned that wrapper context onto the wire, directly contradicting the gateway's own no-leak assertion; both are rewritten to pin the sanitized contract instead. Also name the rejected filter in the twenty-four ParseFilters sites that logged the detail and returned a bare code, and give the ACM force-renew refusals a registered code so their reason still reaches the caller. * fix: stop the filter rejection repeating its own error code to the client ParseFilters spelled the code into its message text, so once the code began travelling separately the client saw it twice: "InvalidParameterValue: InvalidParameterValue: The filter 'x' is invalid". Give ParseFilters the code directly via awserrors.Errorf and have its twenty-five callers return that error unwrapped rather than re-wrapping it. All of them already mapped it to InvalidParameterValue, so nothing changes but the duplication. * fix: surface why a security group rule was rejected ipPermissionsToSGRules already names the reason, "IPv6 rules are not supported" or "IpPermission must specify at least one IpRange or UserIdGroupPair", and both Authorize call sites logged it and returned a bare InvalidParameterValue, so the caller saw only the canned blurb. This is the case mulga-e9qqx names specifically. It does not make IPv6 rules work; it makes the refusal legible.
This commit is contained in:
@@ -115,6 +115,26 @@ func TestResolveErrorDetail_ErrorfMessageWins(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveErrorDetail_ErrorfMessageSurvivesOuterWrapper is the core
|
||||
// anti-leak assertion: a call site's own %w wrapper, added purely for its
|
||||
// logs, must not replace or blend with the curated message Errorf recorded.
|
||||
func TestResolveErrorDetail_ErrorfMessageSurvivesOuterWrapper(t *testing.T) {
|
||||
inner := Errorf(ErrorInsufficientAddressCapacity, "no capacity for m5.large on any eligible node")
|
||||
outer := fmt.Errorf("dial predastore 10.0.1.5:8443: %w", inner)
|
||||
|
||||
code, message, ok := ResolveErrorDetail(outer)
|
||||
if !ok || code != ErrorInsufficientAddressCapacity {
|
||||
t.Fatalf("ResolveErrorDetail() code = (%q, %v), want (%q, true)", code, ok, ErrorInsufficientAddressCapacity)
|
||||
}
|
||||
want := "no capacity for m5.large on any eligible node"
|
||||
if message != want {
|
||||
t.Errorf("ResolveErrorDetail() message = %q, want %q", message, want)
|
||||
}
|
||||
if strings.Contains(message, "dial predastore") {
|
||||
t.Errorf("ResolveErrorDetail() message %q leaked the outer wrapper's context", message)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveErrorDetail_PlainWrapCarriesNoMessage is the compatibility case:
|
||||
// a generic %w wrapper not produced by Errorf must not be mistaken for a
|
||||
// client-facing message, even though it sits right next to the bare code.
|
||||
|
||||
@@ -73,7 +73,8 @@ func respondWithError(nodeID string, msg *nats.Msg, errCode string) {
|
||||
// an opaque ServerInternal, leaving the reason visible only in the daemon log.
|
||||
// Mirrors utils.ServeNATSRequestCtx, which has always preserved the message.
|
||||
func respondWithServiceError(nodeID string, msg *nats.Msg, err error) {
|
||||
payload := utils.GenerateErrorPayloadWithMessage(awserrors.ValidErrorCodeFromError(err), err.Error())
|
||||
_, message, _ := awserrors.ResolveErrorDetail(err)
|
||||
payload := utils.GenerateErrorPayloadWithMessage(awserrors.ValidErrorCodeFromError(err), message)
|
||||
respondNATSMsg(nodeID, msg, payload)
|
||||
}
|
||||
|
||||
|
||||
@@ -44,31 +44,61 @@ func requestErrorEnvelope(t *testing.T, subject string, fn func(context.Context,
|
||||
return errResp
|
||||
}
|
||||
|
||||
// An unregistered error sanitizes to ServerInternal, but its reason must still
|
||||
// reach the caller. This is the ACM force-renew refusal: without the message the
|
||||
// CLI prints a bare "ServerInternal" and the operator has to read the daemon log
|
||||
// to learn that only PRIVATE_CA certificates can be force-renewed.
|
||||
func TestHandleNATSRequest_UncodedErrorCarriesMessage(t *testing.T) {
|
||||
// A plain errors.New carries no client-facing message: only awserrors.Errorf
|
||||
// records one. Falling back to err.Error() here would put whatever context a
|
||||
// caller wrapped in for its own logs onto the wire, which is the disclosure
|
||||
// risk this contract exists to prevent.
|
||||
func TestHandleNATSRequest_UncodedErrorOmitsMessage(t *testing.T) {
|
||||
const reason = "acm renewal: arn:aws:acm:ap-southeast-2:000000000001:certificate/abc is a AMAZON_ISSUED certificate; only PRIVATE_CA certificates can be force-renewed"
|
||||
|
||||
errResp := requestErrorEnvelope(t, "test.err.msg.uncoded", func(context.Context, *testInput, string) (*testOutput, error) {
|
||||
return nil, errors.New(reason)
|
||||
})
|
||||
|
||||
assert.Equal(t, awserrors.ErrorServerInternal, errResp["Code"])
|
||||
assert.Nil(t, errResp["Message"])
|
||||
}
|
||||
|
||||
// The ACM force-renew refusal this test mirrors now builds its error with
|
||||
// awserrors.Errorf specifically so the operator sees the actionable reason
|
||||
// instead of a bare "ServerInternal", without relying on the rejected
|
||||
// err.Error() fallback.
|
||||
func TestHandleNATSRequest_ErrorfMessageOnUnregisteredCodeCarriesMessage(t *testing.T) {
|
||||
const reason = "acm renewal: arn:aws:acm:ap-southeast-2:000000000001:certificate/abc is a AMAZON_ISSUED certificate; only PRIVATE_CA certificates can be force-renewed"
|
||||
|
||||
errResp := requestErrorEnvelope(t, "test.err.msg.uncoded.errorf", func(context.Context, *testInput, string) (*testOutput, error) {
|
||||
return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "%s", reason)
|
||||
})
|
||||
|
||||
assert.Equal(t, awserrors.ErrorServerInternal, errResp["Code"])
|
||||
assert.Equal(t, reason, errResp["Message"],
|
||||
"the actionable reason must survive the transport, not only the daemon log")
|
||||
}
|
||||
|
||||
// A wrapped coded error keeps the real code (so the gateway maps the right HTTP
|
||||
// status) and carries the wrapping context alongside it.
|
||||
func TestHandleNATSRequest_WrappedCodedErrorCarriesMessage(t *testing.T) {
|
||||
// A bare coded error wrapped only for the handler's own log line keeps the
|
||||
// real code (so the gateway maps the right HTTP status) but must not leak the
|
||||
// wrapping context onto the wire — that context is internal detail, not a
|
||||
// message the producing call site attached via awserrors.Errorf.
|
||||
func TestHandleNATSRequest_WrappedCodedErrorOmitsWrapperContext(t *testing.T) {
|
||||
errResp := requestErrorEnvelope(t, "test.err.msg.wrapped", func(context.Context, *testInput, string) (*testOutput, error) {
|
||||
return nil, fmt.Errorf("vol-123 is attached to i-456: %w", errors.New(awserrors.ErrorVolumeInUse))
|
||||
})
|
||||
|
||||
assert.Equal(t, awserrors.ErrorVolumeInUse, errResp["Code"])
|
||||
assert.Contains(t, errResp["Message"], "vol-123 is attached to i-456")
|
||||
assert.Nil(t, errResp["Message"])
|
||||
}
|
||||
|
||||
// A wrapped Errorf-built error keeps its curated message across the wire, but
|
||||
// drops an outer wrapper added purely for the handler's own log line.
|
||||
func TestHandleNATSRequest_WrappedErrorfCarriesMessageNotWrapperContext(t *testing.T) {
|
||||
const curated = "vol-123 is attached to i-456"
|
||||
errResp := requestErrorEnvelope(t, "test.err.msg.wrapped.errorf", func(context.Context, *testInput, string) (*testOutput, error) {
|
||||
cause := awserrors.Errorf(awserrors.ErrorVolumeInUse, "%s", curated)
|
||||
return nil, fmt.Errorf("detach: %w", cause)
|
||||
})
|
||||
|
||||
assert.Equal(t, awserrors.ErrorVolumeInUse, errResp["Code"])
|
||||
assert.Equal(t, curated, errResp["Message"])
|
||||
}
|
||||
|
||||
// A bare code carries no message. Handlers returning errors.New(awserrors.X) are
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
package filterutil
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strings"
|
||||
|
||||
"github.com/aws/aws-sdk-go/service/ec2"
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
)
|
||||
|
||||
// ParseFilters converts AWS SDK filter types to map[string][]string.
|
||||
@@ -23,8 +23,10 @@ func ParseFilters(filters []*ec2.Filter, validNames map[string]bool) (map[string
|
||||
}
|
||||
name := *f.Name
|
||||
|
||||
// Carries the code itself so callers return it unwrapped; wrapping it
|
||||
// again would reach the client stuttered, code repeated in the message.
|
||||
if !strings.HasPrefix(name, "tag:") && !validNames[name] {
|
||||
return nil, fmt.Errorf("InvalidParameterValue: The filter '%s' is invalid", name)
|
||||
return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "The filter '%s' is invalid", name)
|
||||
}
|
||||
|
||||
for _, v := range f.Values {
|
||||
|
||||
@@ -106,8 +106,8 @@ func TestLlamaInvokeAdapter_InvokeModel_UnresolvedEndpointReturnsModelNotReady(t
|
||||
}
|
||||
|
||||
// TestLlamaInvokeAdapter_InvokeModel_EndpointErrorPropagatesCode proves a
|
||||
// resolver err carrying an admission-refusal code (wrapped the way
|
||||
// decodedNATSError's Unwrap tree does) surfaces faithfully rather than being
|
||||
// resolver err carrying an admission-refusal code (wrapped the way a decoded
|
||||
// NATS error's Unwrap tree does) surfaces faithfully rather than being
|
||||
// flattened to ServiceUnavailableException.
|
||||
func TestLlamaInvokeAdapter_InvokeModel_EndpointErrorPropagatesCode(t *testing.T) {
|
||||
wrapped := fmt.Errorf("endpoint resolve: %w", errors.New(awserrors.ErrorModelNotReadyException))
|
||||
|
||||
@@ -79,7 +79,7 @@ func TestVLLMProvider_Converse_UnresolvedEndpointReturnsModelNotReady(t *testing
|
||||
}
|
||||
|
||||
// TestVLLMProvider_Converse_EndpointErrorPropagatesCode proves a resolver err
|
||||
// carrying an admission-refusal code (wrapped the way decodedNATSError's
|
||||
// carrying an admission-refusal code (wrapped the way a decoded NATS error's
|
||||
// Unwrap tree does) surfaces faithfully rather than being flattened to
|
||||
// ServiceUnavailableException.
|
||||
func TestVLLMProvider_Converse_EndpointErrorPropagatesCode(t *testing.T) {
|
||||
|
||||
@@ -376,7 +376,7 @@ func (w *Worker) ForceRenewCertificate(ctx context.Context, input *ForceRenewCer
|
||||
return nil, err
|
||||
}
|
||||
if rec.Type != acm.CertificateTypePrivate {
|
||||
return nil, fmt.Errorf("acm renewal: %s is a %s certificate; only PRIVATE_CA certificates can be force-renewed", input.CertificateArn, rec.Type)
|
||||
return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "acm renewal: %s is a %s certificate; only PRIVATE_CA certificates can be force-renewed", input.CertificateArn, rec.Type)
|
||||
}
|
||||
|
||||
acquired, err := w.renewWithLease(ctx, input.CertificateArn)
|
||||
@@ -384,7 +384,7 @@ func (w *Worker) ForceRenewCertificate(ctx context.Context, input *ForceRenewCer
|
||||
return nil, err
|
||||
}
|
||||
if !acquired {
|
||||
return nil, fmt.Errorf("acm renewal: %s is currently leased by another node; try again shortly", input.CertificateArn)
|
||||
return nil, awserrors.Errorf(awserrors.ErrorServerInternal, "acm renewal: %s is currently leased by another node; try again shortly", input.CertificateArn)
|
||||
}
|
||||
|
||||
// Re-read for the post-renewal outcome only: serial, validity and renewal
|
||||
|
||||
@@ -163,7 +163,7 @@ func (s *EgressOnlyIGWServiceImpl) DescribeEgressOnlyInternetGateways(ctx contex
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeEIGWValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeEgressOnlyInternetGateways: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
|
||||
@@ -449,7 +449,7 @@ func (s *EIPServiceImpl) DescribeAddresses(ctx context.Context, input *ec2.Descr
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeAddressesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeAddresses: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
|
||||
@@ -203,7 +203,7 @@ func (s *IGWServiceImpl) DescribeInternetGateways(ctx context.Context, input *ec
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeIGWValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeInternetGateways: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
|
||||
@@ -132,7 +132,7 @@ func (s *ImageServiceImpl) DescribeImages(ctx context.Context, input *ec2.Descri
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeImagesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeImages: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return s.describeImages(ctx, input, accountID, parsedFilters)
|
||||
|
||||
@@ -1181,7 +1181,10 @@ func TestDescribeImages_FilterUnknownName_Error(t *testing.T) {
|
||||
},
|
||||
}, testAccountID)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
assert.Contains(t, message, "bogus-filter")
|
||||
}
|
||||
|
||||
func TestDescribeImages_FilterWildcard(t *testing.T) {
|
||||
|
||||
@@ -1778,7 +1778,7 @@ func (s *InstanceServiceImpl) DescribeInstances(ctx context.Context, input *ec2.
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, DescribeInstancesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeInstances: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
reservationMap := make(map[string]*ec2.Reservation)
|
||||
@@ -1906,7 +1906,7 @@ func (s *InstanceServiceImpl) describeInstancesFromKV(ctx context.Context, input
|
||||
parsedFilters, filterErr := filterutil.ParseFilters(input.Filters, DescribeInstancesValidFilters)
|
||||
if filterErr != nil {
|
||||
slog.WarnContext(ctx, opName+": invalid filter", "err", filterErr)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, filterErr
|
||||
}
|
||||
|
||||
instances, err := listFn()
|
||||
|
||||
@@ -4199,7 +4199,10 @@ func TestDescribeInstanceStatus_UnknownFilter(t *testing.T) {
|
||||
}},
|
||||
}, owner)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
assert.Contains(t, message, "event.code")
|
||||
}
|
||||
|
||||
func TestDescribeInstanceStatus_StateNameFilter(t *testing.T) {
|
||||
|
||||
@@ -1,12 +1,10 @@
|
||||
package handlers_ec2_instance
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"log/slog"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/ec2"
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
"github.com/mulgadc/spinifex/spinifex/filterutil"
|
||||
"github.com/mulgadc/spinifex/spinifex/vm"
|
||||
)
|
||||
@@ -41,7 +39,7 @@ func ParseStatusSelection(input *ec2.DescribeInstanceStatusInput, accountID stri
|
||||
filters, err := filterutil.ParseFilters(input.Filters, DescribeInstanceStatusValidFilters)
|
||||
if err != nil {
|
||||
slog.Warn("DescribeInstanceStatus: invalid filter", "err", err)
|
||||
return StatusSelection{}, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return StatusSelection{}, err
|
||||
}
|
||||
|
||||
includedStates := describeInstanceStatusRunningOnly
|
||||
|
||||
@@ -605,7 +605,7 @@ func (s *KeyServiceImpl) DescribeKeyPairs(ctx context.Context, input *ec2.Descri
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeKeyPairsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeKeyPairs: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := fmt.Sprintf("keys/%s/", accountID)
|
||||
|
||||
@@ -599,7 +599,7 @@ func (s *LaunchTemplateServiceImpl) DescribeLaunchTemplates(ctx context.Context,
|
||||
filters, err := filterutil.ParseFilters(input.Filters, describeLaunchTemplatesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeLaunchTemplates: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
for _, id := range input.LaunchTemplateIds {
|
||||
@@ -718,7 +718,7 @@ func (s *LaunchTemplateServiceImpl) DescribeLaunchTemplateVersions(ctx context.C
|
||||
filters, err := filterutil.ParseFilters(input.Filters, describeLaunchTemplateVersionsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeLaunchTemplateVersions: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
header, _, err := s.resolveHeader(ctx, accountID, input.LaunchTemplateId, input.LaunchTemplateName)
|
||||
|
||||
@@ -376,7 +376,7 @@ func (s *NatGatewayServiceImpl) DescribeNatGateways(ctx context.Context, input *
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filter, describeNatGatewaysValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeNatGateways: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
natgwIDs := make(map[string]bool)
|
||||
|
||||
@@ -179,7 +179,7 @@ func (s *PlacementGroupServiceImpl) DescribePlacementGroups(ctx context.Context,
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describePlacementGroupsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribePlacementGroups: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Build filter maps for GroupNames/GroupIds parameters
|
||||
|
||||
@@ -613,7 +613,7 @@ func (s *RouteTableServiceImpl) DescribeRouteTables(ctx context.Context, input *
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeRouteTablesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeRouteTables: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
|
||||
@@ -442,7 +442,7 @@ func (s *SnapshotServiceImpl) describeSnapshots(ctx context.Context, input *ec2.
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeSnapshotsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeSnapshots: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// The caller's prefix is the isolation and the whole answer: no other
|
||||
|
||||
@@ -130,7 +130,7 @@ func (s *SpotInstanceServiceImpl) DescribeSpotInstanceRequests(ctx context.Conte
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeSpotRequestsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeSpotInstanceRequests: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
idSet := make(map[string]bool)
|
||||
|
||||
@@ -223,7 +223,10 @@ func TestDescribe_InvalidFilter(t *testing.T) {
|
||||
Filters: []*ec2.Filter{{Name: aws.String("bogus-filter"), Values: []*string{aws.String("x")}}},
|
||||
}, testAccountID)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
assert.Contains(t, message, "bogus-filter")
|
||||
}
|
||||
|
||||
// --- Cancel ---
|
||||
|
||||
@@ -218,7 +218,7 @@ func (s *TagsServiceImpl) DescribeTags(ctx context.Context, input *ec2.DescribeT
|
||||
filters, err = filterutil.ParseFilters(input.Filters, describeTagsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeTags: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -297,7 +297,7 @@ func (s *VolumeServiceImpl) DescribeVolumes(ctx context.Context, input *ec2.Desc
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeVolumesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeVolumes: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Fast path: specific volume IDs requested. Fetch each document
|
||||
@@ -441,7 +441,7 @@ func (s *VolumeServiceImpl) DescribeVolumeStatus(ctx context.Context, input *ec2
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeVolumeStatusValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeVolumeStatus: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return s.describeVolumeStatus(ctx, input, accountID, parsedFilters)
|
||||
@@ -659,7 +659,7 @@ func (s *VolumeServiceImpl) DescribeVolumesModifications(ctx context.Context, in
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeVolumesModificationsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeVolumesModifications: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var modifications []*ec2.VolumeModification
|
||||
|
||||
@@ -2138,7 +2138,10 @@ func TestDescribeVolumes_FilterUnknownName_Error(t *testing.T) {
|
||||
},
|
||||
}, "000000000001")
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
assert.Contains(t, message, "bogus-filter")
|
||||
}
|
||||
|
||||
func TestDescribeVolumes_FilterNoResults(t *testing.T) {
|
||||
@@ -2670,5 +2673,8 @@ func TestDescribeVolumesModifications_UnknownFilter(t *testing.T) {
|
||||
},
|
||||
}, testVolAccountID)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
assert.Contains(t, message, "not-a-real-filter")
|
||||
}
|
||||
|
||||
@@ -223,7 +223,10 @@ func TestGetSecurityGroupsForVpc_Filters(t *testing.T) {
|
||||
Filters: []*ec2.Filter{{Name: aws.String("vpc-id"), Values: []*string{aws.String(vpcID)}}},
|
||||
}, testAccountID)
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, err.Error())
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
assert.Contains(t, message, "vpc-id")
|
||||
|
||||
out, err = svc.GetSecurityGroupsForVpc(context.Background(), &ec2.GetSecurityGroupsForVpcInput{
|
||||
VpcId: aws.String(vpcID),
|
||||
|
||||
@@ -476,7 +476,7 @@ func (s *VPCServiceImpl) DescribeNetworkInterfaces(ctx context.Context, input *e
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeNetworkInterfacesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeNetworkInterfaces: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
enis := make([]*ec2.NetworkInterface, 0)
|
||||
|
||||
@@ -426,7 +426,7 @@ func (s *VPCServiceImpl) DescribeSecurityGroups(ctx context.Context, input *ec2.
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeSecurityGroupsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeSecurityGroups: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
@@ -570,7 +570,7 @@ func (s *VPCServiceImpl) GetSecurityGroupsForVpc(ctx context.Context, input *ec2
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, getSecurityGroupsForVpcValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "GetSecurityGroupsForVpc: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
@@ -726,7 +726,7 @@ func (s *VPCServiceImpl) DescribeSecurityGroupRules(ctx context.Context, input *
|
||||
parsedFilters, err := filterutil.ParseFilters(filters, describeSecurityGroupRulesValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeSecurityGroupRules: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
@@ -876,7 +876,7 @@ func (s *VPCServiceImpl) AuthorizeSecurityGroupIngress(ctx context.Context, inpu
|
||||
newRules, err := ipPermissionsToSGRules(input.IpPermissions, sgParseAuthorize)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "AuthorizeSecurityGroupIngress: invalid rule", "groupId", groupId, "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "%s", err)
|
||||
}
|
||||
if err := s.validateSGRuleReferences(ctx, accountID, record.VpcId, newRules); err != nil {
|
||||
return nil, err
|
||||
@@ -951,7 +951,7 @@ func (s *VPCServiceImpl) AuthorizeSecurityGroupEgress(ctx context.Context, input
|
||||
newRules, err := ipPermissionsToSGRules(input.IpPermissions, sgParseAuthorize)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "AuthorizeSecurityGroupEgress: invalid rule", "groupId", groupId, "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "%s", err)
|
||||
}
|
||||
if err := s.validateSGRuleReferences(ctx, accountID, record.VpcId, newRules); err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -578,7 +578,7 @@ func (s *VPCServiceImpl) DescribeVpcs(ctx context.Context, input *ec2.DescribeVp
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeVpcsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeVpcs: invalid filter", "err", err)
|
||||
return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "%s", err)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
@@ -947,7 +947,7 @@ func (s *VPCServiceImpl) DescribeSubnets(ctx context.Context, input *ec2.Describ
|
||||
parsedFilters, err := filterutil.ParseFilters(input.Filters, describeSubnetsValidFilters)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "DescribeSubnets: invalid filter", "err", err)
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
prefix := accountID + "."
|
||||
|
||||
+3
-17
@@ -267,21 +267,6 @@ func PrincipalARNFromMsg(msg *nats.Msg) string {
|
||||
return msg.Header.Get(PrincipalARNHeader)
|
||||
}
|
||||
|
||||
// decodedNATSError retains a response's diagnostic message while exposing its
|
||||
// sanitized AWS wire code to unwrap-aware classifiers.
|
||||
type decodedNATSError struct {
|
||||
code error
|
||||
message string
|
||||
}
|
||||
|
||||
func (e *decodedNATSError) Error() string {
|
||||
return e.message
|
||||
}
|
||||
|
||||
func (e *decodedNATSError) Unwrap() error {
|
||||
return e.code
|
||||
}
|
||||
|
||||
// NATSRequest performs a NATS request-response with JSON marshaling. It sends
|
||||
// with X-Account-ID (plus any extra headers), unmarshals the successful response
|
||||
// into Out, and carries ctx's trace context onto the wire: it opens a client span
|
||||
@@ -325,7 +310,7 @@ func NATSRequest[Out any](ctx context.Context, conn *nats.Conn, subject string,
|
||||
responseError, err := ValidateErrorPayload(msg.Data)
|
||||
if err != nil {
|
||||
if responseError.Message != nil && *responseError.Message != "" {
|
||||
return nil, &decodedNATSError{code: errors.New(*responseError.Code), message: *responseError.Message}
|
||||
return nil, awserrors.Errorf(*responseError.Code, "%s", *responseError.Message)
|
||||
}
|
||||
return nil, errors.New(*responseError.Code)
|
||||
}
|
||||
@@ -361,7 +346,8 @@ func ServeNATSRequestCtx[I any, O any](msg *nats.Msg, fn func(context.Context, *
|
||||
out, err := fn(ctx, input)
|
||||
if err != nil {
|
||||
MarkSpanError(span, err)
|
||||
respondNATS(msg, GenerateErrorPayloadWithMessage(awserrors.ValidErrorCodeFromError(err), err.Error()))
|
||||
_, message, _ := awserrors.ResolveErrorDetail(err)
|
||||
respondNATS(msg, GenerateErrorPayloadWithMessage(awserrors.ValidErrorCodeFromError(err), message))
|
||||
return false
|
||||
}
|
||||
data, err := json.Marshal(out)
|
||||
|
||||
@@ -286,6 +286,30 @@ func TestNATSRequest_ErrorResponse(t *testing.T) {
|
||||
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
|
||||
}
|
||||
|
||||
// TestNATSRequest_DecodedErrorContainsCode pins the shape strings.Contains(err.Error(), code)
|
||||
// classifiers depend on (e.g. handlers/iam/provision.go's isAlreadyExists,
|
||||
// gateway/auth.go's IAM-not-found branch): the decoded error's Error() text
|
||||
// must still contain the wire code even when a message rides alongside it.
|
||||
func TestNATSRequest_DecodedErrorContainsCode(t *testing.T) {
|
||||
ns := startTestNATSServer(t)
|
||||
|
||||
nc, err := nats.Connect(ns.ClientURL())
|
||||
require.NoError(t, err)
|
||||
defer nc.Close()
|
||||
|
||||
const code = "EntityAlreadyExists"
|
||||
const reason = "access key AKIA... already exists for user provisioning-bot"
|
||||
_, err = nc.Subscribe("test.fail.classify", func(msg *nats.Msg) {
|
||||
msg.Respond(GenerateErrorPayloadWithMessage(code, reason))
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
type Resp struct{}
|
||||
_, err = NATSRequest[Resp](context.Background(), nc, "test.fail.classify", struct{}{}, 2*time.Second, "")
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), code)
|
||||
}
|
||||
|
||||
func TestNATSRequest_ErrorResponseSurfacesMessage(t *testing.T) {
|
||||
ns := startTestNATSServer(t)
|
||||
|
||||
@@ -304,20 +328,22 @@ func TestNATSRequest_ErrorResponseSurfacesMessage(t *testing.T) {
|
||||
type Resp struct{}
|
||||
_, err = NATSRequest[Resp](context.Background(), nc, "test.fail.msg", struct{}{}, 2*time.Second, "")
|
||||
assert.Error(t, err)
|
||||
assert.Equal(t, reason, err.Error())
|
||||
code, ok := awserrors.ResolveErrorCode(err)
|
||||
assert.Contains(t, err.Error(), reason)
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorServerInternal, code)
|
||||
assert.Equal(t, reason, message)
|
||||
}
|
||||
|
||||
func TestServeNATSRequest_WrappedErrorPreservesCodeAndMessage(t *testing.T) {
|
||||
func TestServeNATSRequest_WrappedErrorPreservesCodeButDropsWrapperContext(t *testing.T) {
|
||||
ns := startTestNATSServer(t)
|
||||
|
||||
nc, err := nats.Connect(ns.ClientURL())
|
||||
require.NoError(t, err)
|
||||
defer nc.Close()
|
||||
|
||||
const message = "launch on node-1: InsufficientAddressCapacity"
|
||||
// A bare errors.New(code) wrapped only for the handler's own logs carries
|
||||
// no client-facing message, so the wrapper text must not reach the wire.
|
||||
_, err = nc.Subscribe("test.serve.error", func(msg *nats.Msg) {
|
||||
ServeNATSRequest(msg, func(_ *struct{}) (*struct{}, error) {
|
||||
cause := errors.New(awserrors.ErrorInsufficientAddressCapacity)
|
||||
@@ -329,12 +355,41 @@ func TestServeNATSRequest_WrappedErrorPreservesCodeAndMessage(t *testing.T) {
|
||||
|
||||
_, err = NATSRequest[struct{}](context.Background(), nc, "test.serve.error", struct{}{}, 2*time.Second, "")
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, message, err.Error())
|
||||
assert.Equal(t, awserrors.ErrorInsufficientAddressCapacity, err.Error())
|
||||
assert.NotContains(t, err.Error(), "launch on node-1")
|
||||
code, ok := awserrors.ResolveErrorCode(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInsufficientAddressCapacity, code)
|
||||
}
|
||||
|
||||
func TestServeNATSRequest_ErrorfMessageSurvivesOuterWrapperContext(t *testing.T) {
|
||||
ns := startTestNATSServer(t)
|
||||
|
||||
nc, err := nats.Connect(ns.ClientURL())
|
||||
require.NoError(t, err)
|
||||
defer nc.Close()
|
||||
|
||||
// The curated message from awserrors.Errorf must round-trip over NATS,
|
||||
// but an outer %w wrapper added only for the handler's own logs must not.
|
||||
const curated = "no capacity for m5.large on any eligible node"
|
||||
_, err = nc.Subscribe("test.serve.error.curated", func(msg *nats.Msg) {
|
||||
ServeNATSRequest(msg, func(_ *struct{}) (*struct{}, error) {
|
||||
cause := awserrors.Errorf(awserrors.ErrorInsufficientAddressCapacity, "%s", curated)
|
||||
return nil, fmt.Errorf("launch on node-1: %w", cause)
|
||||
})
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, nc.Flush())
|
||||
|
||||
_, err = NATSRequest[struct{}](context.Background(), nc, "test.serve.error.curated", struct{}{}, 2*time.Second, "")
|
||||
require.Error(t, err)
|
||||
assert.NotContains(t, err.Error(), "launch on node-1")
|
||||
code, message, ok := awserrors.ResolveErrorDetail(err)
|
||||
assert.True(t, ok)
|
||||
assert.Equal(t, awserrors.ErrorInsufficientAddressCapacity, code)
|
||||
assert.Equal(t, curated, message)
|
||||
}
|
||||
|
||||
func TestNATSRequest_NoResponders(t *testing.T) {
|
||||
ns := startTestNATSServer(t)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user