From e3fe9a634e7b28bfb9ebc55ca2a05b0888d6b8ad Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:12:56 +1000 Subject: [PATCH] fix: return AWS's error messages for placement group, key pair, launch template and address errors --- spinifex/handlers/ec2/eip/service_impl.go | 6 +- .../handlers/ec2/eip/service_impl_test.go | 19 ++++++ spinifex/handlers/ec2/key/service_impl.go | 27 ++++++--- .../handlers/ec2/key/service_impl_test.go | 19 ++++-- .../ec2/launchtemplate/expand_test.go | 13 ++++- .../ec2/launchtemplate/service_impl.go | 52 +++++++++++++---- .../ec2/launchtemplate/service_impl_test.go | 58 +++++++++++++++++-- .../ec2/placementgroup/service_impl.go | 11 ++-- .../ec2/placementgroup/service_impl_test.go | 15 ++++- 9 files changed, 176 insertions(+), 44 deletions(-) diff --git a/spinifex/handlers/ec2/eip/service_impl.go b/spinifex/handlers/ec2/eip/service_impl.go index 663cbafd9..69551d067 100644 --- a/spinifex/handlers/ec2/eip/service_impl.go +++ b/spinifex/handlers/ec2/eip/service_impl.go @@ -596,9 +596,9 @@ func (s *EIPServiceImpl) DescribeAddresses(ctx context.Context, input *ec2.Descr found[*addr.AllocationId] = true } } - for id := range allocIDs { - if !found[id] { - return nil, errors.New(awserrors.ErrorInvalidAllocationIDNotFound) + for _, id := range input.AllocationIds { + if id != nil && !found[*id] { + return nil, awserrors.Errorf(awserrors.ErrorInvalidAllocationIDNotFound, "The allocation ID '%s' does not exist", *id) } } } diff --git a/spinifex/handlers/ec2/eip/service_impl_test.go b/spinifex/handlers/ec2/eip/service_impl_test.go index 9f817ca26..73bbe8b75 100644 --- a/spinifex/handlers/ec2/eip/service_impl_test.go +++ b/spinifex/handlers/ec2/eip/service_impl_test.go @@ -571,6 +571,25 @@ func TestEIP_DescribeAddresses_PublicIpNotFound(t *testing.T) { assert.Equal(t, "Address '"+*released.PublicIp+"' not found.", msg) } +func TestEIP_DescribeAddresses_AllocationIdNotFound(t *testing.T) { + svc, _, _ := setupTestEIP(t) + + kept, err := svc.AllocateAddress(context.Background(), &ec2.AllocateAddressInput{}, testAccountID) + require.NoError(t, err) + released, err := svc.AllocateAddress(context.Background(), &ec2.AllocateAddressInput{}, testAccountID) + require.NoError(t, err) + _, err = svc.ReleaseAddress(context.Background(), &ec2.ReleaseAddressInput{AllocationId: released.AllocationId}, testAccountID) + require.NoError(t, err) + + _, err = svc.DescribeAddresses(context.Background(), &ec2.DescribeAddressesInput{ + AllocationIds: []*string{kept.AllocationId, released.AllocationId}, + }, testAccountID) + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok) + assert.Equal(t, awserrors.ErrorInvalidAllocationIDNotFound, code) + assert.Equal(t, "The allocation ID '"+*released.AllocationId+"' does not exist", msg) +} + func TestEIP_DescribeAddresses_FilterByAllocationId(t *testing.T) { svc, _, _ := setupTestEIP(t) diff --git a/spinifex/handlers/ec2/key/service_impl.go b/spinifex/handlers/ec2/key/service_impl.go index 3e1bef942..5ec897277 100644 --- a/spinifex/handlers/ec2/key/service_impl.go +++ b/spinifex/handlers/ec2/key/service_impl.go @@ -132,7 +132,7 @@ func (s *KeyServiceImpl) CreateKeyPair(ctx context.Context, input *ec2.CreateKey if err == nil { // Object exists - return duplicate error slog.ErrorContext(ctx, "Key pair already exists", "keyName", keyName) - return nil, errors.New(awserrors.ErrorInvalidKeyPairDuplicate) + return nil, errDuplicateKeyPair() } // Determine key type (default: ed25519, optional: rsa). This deviates from @@ -415,6 +415,17 @@ func (s *KeyServiceImpl) storeKeyPairMetadata(ctx context.Context, accountID, ke return nil } +// errDuplicateKeyPair is AWS's error for a name already in use, on create and +// import alike. +func errDuplicateKeyPair() error { + return awserrors.Errorf(awserrors.ErrorInvalidKeyPairDuplicate, "The keypair already exists") +} + +// errInvalidKeyFormat is AWS's error for import material it cannot use. +func errInvalidKeyFormat() error { + return awserrors.Errorf(awserrors.ErrorInvalidKeyFormat, "Key is not in valid OpenSSH public key format") +} + // getKeyNameFromKeyPairId retrieves the key name by directly reading the metadata file for a given keyPairId. func (s *KeyServiceImpl) getKeyNameFromKeyPairId(ctx context.Context, accountID, keyPairID string) (string, error) { metadataPath := fmt.Sprintf("keys/%s/%s.json", accountID, keyPairID) @@ -841,12 +852,12 @@ func (s *KeyServiceImpl) DescribeKeyPairs(ctx context.Context, input *ec2.Descri } for _, name := range input.KeyNames { if name != nil && !foundNames[*name] { - return nil, errors.New(awserrors.ErrorInvalidKeyPairNotFound) + return nil, awserrors.Errorf(awserrors.ErrorInvalidKeyPairNotFound, "The key pair '%s' does not exist", *name) } } for _, id := range input.KeyPairIds { if id != nil && !foundIDs[*id] { - return nil, errors.New(awserrors.ErrorInvalidKeyPairNotFound) + return nil, awserrors.Errorf(awserrors.ErrorInvalidKeyPairNotFound, "The keyPairId '%s' does not exist", *id) } } } @@ -922,7 +933,7 @@ func (s *KeyServiceImpl) ImportKeyPair(ctx context.Context, input *ec2.ImportKey if err == nil { // Object exists - return duplicate error slog.ErrorContext(ctx, "Key pair already exists", "keyName", keyName) - return nil, errors.New(awserrors.ErrorInvalidKeyPairDuplicate) + return nil, errDuplicateKeyPair() } // The material is stored verbatim and later served to instances as their @@ -940,7 +951,7 @@ func (s *KeyServiceImpl) ImportKeyPair(ctx context.Context, input *ec2.ImportKey publicKeyData := bytes.TrimSpace(input.PublicKeyMaterial) if bytes.ContainsAny(publicKeyData, "\r\n") { slog.ErrorContext(ctx, "Public key material is not a single key", "keyName", keyName) - return nil, errors.New(awserrors.ErrorInvalidKeyFormat) + return nil, errInvalidKeyFormat() } // Parse the authorized-key line ("ssh-rsa AAAAB... comment"), which also @@ -948,7 +959,7 @@ func (s *KeyServiceImpl) ImportKeyPair(ctx context.Context, input *ec2.ImportKey publicKey, _, options, _, err := ssh.ParseAuthorizedKey(publicKeyData) if err != nil { slog.ErrorContext(ctx, "Invalid public key format", "keyName", keyName, "err", err) - return nil, errors.New(awserrors.ErrorInvalidKeyFormat) + return nil, errInvalidKeyFormat() } // An option prefix ("command=...", "from=...") is not covered by the @@ -956,13 +967,13 @@ func (s *KeyServiceImpl) ImportKeyPair(ctx context.Context, input *ec2.ImportKey // launched with this key pair. Refuse to import access the API cannot report. if len(options) > 0 { slog.ErrorContext(ctx, "Public key material carries authorized_keys options", "keyName", keyName, "options", options) - return nil, errors.New(awserrors.ErrorInvalidKeyFormat) + return nil, errInvalidKeyFormat() } keyType, err := keyPairType(publicKey) if err != nil { slog.ErrorContext(ctx, "Unsupported key type", "algorithm", publicKey.Type(), "keyName", keyName, "err", err) - return nil, errors.New(awserrors.ErrorInvalidKeyFormat) + return nil, errInvalidKeyFormat() } fingerprint, err := importedKeyFingerprint(publicKey) diff --git a/spinifex/handlers/ec2/key/service_impl_test.go b/spinifex/handlers/ec2/key/service_impl_test.go index 67afa9e46..4ef341b19 100644 --- a/spinifex/handlers/ec2/key/service_impl_test.go +++ b/spinifex/handlers/ec2/key/service_impl_test.go @@ -255,7 +255,7 @@ func TestCreateKeyPair_Duplicate(t *testing.T) { }, testAccountID) require.Error(t, err) assert.Nil(t, out) - assert.Equal(t, awserrors.ErrorInvalidKeyPairDuplicate, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidKeyPairDuplicate, "The keypair already exists") } func TestCreateKeyPair_InvalidKeyType(t *testing.T) { @@ -377,7 +377,7 @@ func TestImportKeyPair_Duplicate(t *testing.T) { }, testAccountID) require.Error(t, err) assert.Nil(t, out) - assert.Equal(t, awserrors.ErrorInvalidKeyPairDuplicate, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidKeyPairDuplicate, "The keypair already exists") } func TestImportKeyPair_InvalidKeyName(t *testing.T) { @@ -466,7 +466,7 @@ func TestImportKeyPairInvalidKeyFormat(t *testing.T) { PublicKeyMaterial: []byte(tt.publicKey), }, testAccountID) require.Error(t, err) - assert.Equal(t, tt.expectedErrMsg, err.Error()) + assertAWSError(t, err, tt.expectedErrMsg, "Key is not in valid OpenSSH public key format") // Rejection must precede the upload, or the guest is served material // the API refused. @@ -795,7 +795,7 @@ func TestDescribeKeyPairs_NotFound_ByKeyName(t *testing.T) { KeyNames: []*string{aws.String("does-not-exist")}, }, testAccountID) require.Error(t, err) - assert.Contains(t, err.Error(), "InvalidKeyPair.NotFound") + assertAWSError(t, err, awserrors.ErrorInvalidKeyPairNotFound, "The key pair 'does-not-exist' does not exist") } // TestDescribeKeyPairs_NotFound_ByKeyPairId is the KeyPairIds counterpart of @@ -809,7 +809,7 @@ func TestDescribeKeyPairs_NotFound_ByKeyPairId(t *testing.T) { KeyPairIds: []*string{aws.String("key-doesnotexist")}, }, testAccountID) require.Error(t, err) - assert.Contains(t, err.Error(), "InvalidKeyPair.NotFound") + assertAWSError(t, err, awserrors.ErrorInvalidKeyPairNotFound, "The keyPairId 'key-doesnotexist' does not exist") } // TestDescribeKeyPairs_NotFound_PartialMatch asserts that naming one existing @@ -1243,3 +1243,12 @@ func TestKeyPair_CentralTagStoreFailureDoesNotFailOperation(t *testing.T) { assert.Equal(t, 2, central.puts) assert.Equal(t, 1, central.deletes) } + +// assertAWSError checks the code a client receives and the message with it. +func assertAWSError(t *testing.T, err error, wantCode, wantMessage string) { + t.Helper() + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "unresolvable error: %v", err) + assert.Equal(t, wantCode, code) + assert.Equal(t, wantMessage, msg) +} diff --git a/spinifex/handlers/ec2/launchtemplate/expand_test.go b/spinifex/handlers/ec2/launchtemplate/expand_test.go index d33f49f9f..2e4a093ea 100644 --- a/spinifex/handlers/ec2/launchtemplate/expand_test.go +++ b/spinifex/handlers/ec2/launchtemplate/expand_test.go @@ -84,7 +84,7 @@ func TestExpandRunInstances_MissingVersion(t *testing.T) { } err := ExpandRunInstances(context.Background(), svc, input, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, err.Error()) + assertCode(t, err, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound) } func TestExpandRunInstances_IdNameConflict(t *testing.T) { @@ -115,5 +115,14 @@ func TestExpandRunInstances_UnknownTemplate(t *testing.T) { } err := ExpandRunInstances(context.Background(), svc, input, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdNotFound, err.Error()) + assertCode(t, err, awserrors.ErrorInvalidLaunchTemplateIdNotFound) +} + +// assertCode checks the code a client receives; the message is the +// DescribeLaunchTemplateVersions one the expansion passes through. +func assertCode(t *testing.T, err error, want string) { + t.Helper() + code, ok := awserrors.ResolveErrorCode(err) + require.True(t, ok, "unresolvable error: %v", err) + assert.Equal(t, want, code) } diff --git a/spinifex/handlers/ec2/launchtemplate/service_impl.go b/spinifex/handlers/ec2/launchtemplate/service_impl.go index 8dab854a6..83d96ea97 100644 --- a/spinifex/handlers/ec2/launchtemplate/service_impl.go +++ b/spinifex/handlers/ec2/launchtemplate/service_impl.go @@ -143,9 +143,19 @@ func (s *LaunchTemplateServiceImpl) resolveHeader(ctx context.Context, accountID if err := validateTemplateID(id); err != nil { return nil, nil, err } - return s.getHeaderByID(ctx, accountID, id) + h, entry, err := s.getHeaderByID(ctx, accountID, id) + if err != nil && err.Error() == awserrors.ErrorInvalidLaunchTemplateIdNotFound { + return nil, nil, awserrors.Errorf(awserrors.ErrorInvalidLaunchTemplateIdNotFound, + "The specified launch template, with template ID %s, does not exist.", id) + } + return h, entry, err case nm != "": - return s.getHeaderByName(ctx, accountID, nm) + h, entry, err := s.getHeaderByName(ctx, accountID, nm) + if err != nil && err.Error() == awserrors.ErrorInvalidLaunchTemplateNameNotFoundException { + return nil, nil, awserrors.Errorf(awserrors.ErrorInvalidLaunchTemplateNameNotFoundException, + "The specified launch template, with template name %s, does not exist.", nm) + } + return h, entry, err default: return nil, nil, errors.New(awserrors.ErrorMissingParameter) } @@ -308,6 +318,11 @@ func (s *LaunchTemplateServiceImpl) CreateLaunchTemplate(ctx context.Context, in }, nil } +// errNameInUse is AWS's error for a launch template name already taken. +func errNameInUse() error { + return awserrors.Errorf(awserrors.ErrorInvalidLaunchTemplateNameAlreadyExistsException, "Launch template name already in use.") +} + // claimName atomically reserves a template name via kv.Create, with // repair-on-write: if the name already exists but its header is gone (a crash // orphan), reclaim it via a revision-guarded CAS update. Only a name whose header @@ -328,12 +343,12 @@ func (s *LaunchTemplateServiceImpl) claimName(ctx context.Context, accountID, na _, herr := s.kv.Get(ctx, headerKey(accountID, string(entry.Value()))) switch { case herr == nil: - return errors.New(awserrors.ErrorInvalidLaunchTemplateNameAlreadyExistsException) + return errNameInUse() case !errors.Is(herr, jetstream.ErrKeyNotFound): return errors.New(awserrors.ErrorServerInternal) } if _, err := s.kv.Update(ctx, key, []byte(ltID), entry.Revision()); err != nil { - return errors.New(awserrors.ErrorInvalidLaunchTemplateNameAlreadyExistsException) + return errNameInUse() } return nil } @@ -381,11 +396,11 @@ func (s *LaunchTemplateServiceImpl) CreateLaunchTemplateVersion(ctx context.Cont if src := aws.StringValue(input.SourceVersion); src != "" { n, err := s.resolveVersionNumber(ctx, accountID, header, src) if err != nil { - return nil, err + return nil, withVersionNotFoundMessage(err, "Launch template version does not exist.") } base, err := s.getVersion(ctx, accountID, header.LaunchTemplateId, n) if err != nil { - return nil, err + return nil, withVersionNotFoundMessage(err, "Launch template version does not exist.") } data = mergeResponseData(base.Data, override) } @@ -440,14 +455,15 @@ func (s *LaunchTemplateServiceImpl) ModifyLaunchTemplate(ctx context.Context, in } if sel := aws.StringValue(input.DefaultVersion); sel != "" { + notFound := fmt.Sprintf("The launch template version %s is not found for the specified launch template.", sel) n, err := s.resolveVersionNumber(ctx, accountID, header, sel) if err != nil { - return nil, err + return nil, withVersionNotFoundMessage(err, notFound) } // Verify the target body exists immediately before the header CAS so a // concurrent delete degrades to VersionNotFound, never a dangling default. if _, err := s.getVersion(ctx, accountID, header.LaunchTemplateId, n); err != nil { - return nil, err + return nil, withVersionNotFoundMessage(err, notFound) } header.DefaultVersionNumber = n data, err := json.Marshal(header) @@ -635,14 +651,15 @@ func (s *LaunchTemplateServiceImpl) DescribeLaunchTemplates(ctx context.Context, templates = append(templates, headerToEC2(&h, latestByID[h.LaunchTemplateId])) } + const missing = "At least one of the launch templates specified in the request does not exist." for name := range nameSet { if !foundNames[name] { - return nil, errors.New(awserrors.ErrorInvalidLaunchTemplateNameNotFoundException) + return nil, awserrors.Errorf(awserrors.ErrorInvalidLaunchTemplateNameNotFoundException, missing) } } for id := range idSet { if !foundIDs[id] { - return nil, errors.New(awserrors.ErrorInvalidLaunchTemplateIdNotFound) + return nil, awserrors.Errorf(awserrors.ErrorInvalidLaunchTemplateIdNotFound, missing) } } @@ -738,12 +755,14 @@ func (s *LaunchTemplateServiceImpl) selectVersionNumbers(ctx context.Context, ac seen := make(map[int64]bool) var out []int64 for _, v := range input.Versions { + notFound := fmt.Sprintf("Could not find the specified version %s for the launch template with ID %s.", + aws.StringValue(v), header.LaunchTemplateId) n, err := s.resolveVersionNumber(ctx, accountID, header, aws.StringValue(v)) if err != nil { - return nil, err + return nil, withVersionNotFoundMessage(err, notFound) } if _, err := s.getVersion(ctx, accountID, header.LaunchTemplateId, n); err != nil { - return nil, err + return nil, withVersionNotFoundMessage(err, notFound) } if !seen[n] { seen[n] = true @@ -864,6 +883,15 @@ func versionRecToEC2(h *LaunchTemplateHeader, rec *LaunchTemplateVersionRec, isD // --- misc helpers --- +// withVersionNotFoundMessage attaches AWS's message, which differs by action, +// to a VersionNotFound error and passes any other error through. +func withVersionNotFoundMessage(err error, message string) error { + if err.Error() != awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound { + return err + } + return awserrors.Errorf(awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, "%s", message) +} + // isHeaderKey reports whether k is a template header key (account.lt-) as // opposed to a name index, version body (account.lt-.v), or _version key. func isHeaderKey(k, prefix string) bool { diff --git a/spinifex/handlers/ec2/launchtemplate/service_impl_test.go b/spinifex/handlers/ec2/launchtemplate/service_impl_test.go index 1c518a941..d44690ff7 100644 --- a/spinifex/handlers/ec2/launchtemplate/service_impl_test.go +++ b/spinifex/handlers/ec2/launchtemplate/service_impl_test.go @@ -84,7 +84,7 @@ func TestCreateLaunchTemplate_DuplicateName(t *testing.T) { LaunchTemplateData: &ec2.RequestLaunchTemplateData{ImageId: aws.String("ami-1")}, }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateNameAlreadyExistsException, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateNameAlreadyExistsException, "Launch template name already in use.") } func TestLaunchTemplateNameWithKVUnsafeCharacters(t *testing.T) { @@ -158,7 +158,8 @@ func TestDescribeLaunchTemplates_UnknownName(t *testing.T) { LaunchTemplateNames: []*string{aws.String("missing")}, }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateNameNotFoundException, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateNameNotFoundException, + "At least one of the launch templates specified in the request does not exist.") } func TestDescribeLaunchTemplates_TagFilter(t *testing.T) { @@ -331,7 +332,39 @@ func TestDescribeLaunchTemplateVersions_MissingVersion(t *testing.T) { Versions: []*string{aws.String("99")}, }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, + "Could not find the specified version 99 for the launch template with ID "+aws.StringValue(lt.LaunchTemplateId)+".") +} + +func TestLaunchTemplate_NotFoundMessagesNameTheTemplate(t *testing.T) { + svc := setupTestService(t) + lt := createTemplate(t, svc, "web", "t3.micro") + + _, err := svc.DeleteLaunchTemplate(context.Background(), &ec2.DeleteLaunchTemplateInput{ + LaunchTemplateName: aws.String("missing"), + }, testAccountID) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateNameNotFoundException, + "The specified launch template, with template name missing, does not exist.") + + _, err = svc.DescribeLaunchTemplateVersions(context.Background(), &ec2.DescribeLaunchTemplateVersionsInput{ + LaunchTemplateId: aws.String("lt-0123456789abcdef0"), + }, testAccountID) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateIdNotFound, + "The specified launch template, with template ID lt-0123456789abcdef0, does not exist.") + + _, err = svc.ModifyLaunchTemplate(context.Background(), &ec2.ModifyLaunchTemplateInput{ + LaunchTemplateId: lt.LaunchTemplateId, + DefaultVersion: aws.String("42"), + }, testAccountID) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, + "The launch template version 42 is not found for the specified launch template.") + + _, err = svc.CreateLaunchTemplateVersion(context.Background(), &ec2.CreateLaunchTemplateVersionInput{ + LaunchTemplateId: lt.LaunchTemplateId, + SourceVersion: aws.String("42"), + LaunchTemplateData: &ec2.RequestLaunchTemplateData{InstanceType: aws.String("t3.small")}, + }, testAccountID) + assertAWSError(t, err, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, "Launch template version does not exist.") } func TestDescribeLaunchTemplateVersions_LatestAfterTailDelete(t *testing.T) { @@ -388,7 +421,8 @@ func TestModifyLaunchTemplate_DefaultToMissingVersion(t *testing.T) { DefaultVersion: aws.String("42"), }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, err.Error()) + code, _ := awserrors.ResolveErrorCode(err) + assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdVersionNotFound, code) } // --- DeleteLaunchTemplateVersions --- @@ -453,7 +487,8 @@ func TestDeleteLaunchTemplate_RemovesEverything(t *testing.T) { LaunchTemplateIds: []*string{aws.String(id)}, }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdNotFound, err.Error()) + code, _ := awserrors.ResolveErrorCode(err) + assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdNotFound, code) // Version bodies and name index gone. nums, err := svc.listVersionNumbers(t.Context(), testAccountID, id) @@ -472,7 +507,8 @@ func TestDeleteLaunchTemplate_Unknown(t *testing.T) { LaunchTemplateId: aws.String("lt-doesnotexist000"), }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdNotFound, err.Error()) + code, _ := awserrors.ResolveErrorCode(err) + assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdNotFound, code) } func TestResolveHeader_IdNameConflict(t *testing.T) { @@ -503,3 +539,13 @@ func TestDescribeLaunchTemplates_MalformedId(t *testing.T) { require.Error(t, err) assert.Equal(t, awserrors.ErrorInvalidLaunchTemplateIdMalformed, err.Error()) } + +// assertAWSError checks the code a client receives and the message with it. +func assertAWSError(t *testing.T, err error, wantCode, wantMessage string) { + t.Helper() + require.Error(t, err) + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "unresolvable error: %v", err) + assert.Equal(t, wantCode, code) + assert.Equal(t, wantMessage, msg) +} diff --git a/spinifex/handlers/ec2/placementgroup/service_impl.go b/spinifex/handlers/ec2/placementgroup/service_impl.go index 80286cba1..aeeba5955 100644 --- a/spinifex/handlers/ec2/placementgroup/service_impl.go +++ b/spinifex/handlers/ec2/placementgroup/service_impl.go @@ -131,7 +131,7 @@ func (s *PlacementGroupServiceImpl) CreatePlacementGroup(ctx context.Context, in // Atomic create-if-not-exists to prevent TOCTOU race on duplicate names if _, err := s.kv.Create(ctx, key, data); err != nil { // Create fails if key already exists - return nil, errors.New(awserrors.ErrorInvalidPlacementGroupDuplicate) + return nil, awserrors.Errorf(awserrors.ErrorInvalidPlacementGroupDuplicate, "The placement group '%s' already exists.", groupName) } slog.InfoContext(ctx, "CreatePlacementGroup completed", "groupId", groupID, "groupName", groupName, "strategy", strategy, "accountID", accountID) @@ -152,7 +152,8 @@ func (s *PlacementGroupServiceImpl) DeletePlacementGroup(ctx context.Context, in entry, err := s.kv.Get(ctx, key) if err != nil { - return nil, errors.New(awserrors.ErrorInvalidPlacementGroupUnknown) + // Lower case, unlike DescribePlacementGroups' message for the same code. + return nil, awserrors.Errorf(awserrors.ErrorInvalidPlacementGroupUnknown, "The placement group '%s' is unknown.", groupName) } var record PlacementGroupRecord @@ -260,9 +261,9 @@ func (s *PlacementGroupServiceImpl) DescribePlacementGroups(ctx context.Context, found[*g.GroupName] = true } } - for name := range nameSet { - if !found[name] { - return nil, errors.New(awserrors.ErrorInvalidPlacementGroupUnknown) + for _, name := range input.GroupNames { + if name != nil && !found[*name] { + return nil, awserrors.Errorf(awserrors.ErrorInvalidPlacementGroupUnknown, "The Placement Group '%s' is unknown.", *name) } } } diff --git a/spinifex/handlers/ec2/placementgroup/service_impl_test.go b/spinifex/handlers/ec2/placementgroup/service_impl_test.go index b8a7549e3..48ed1e51c 100644 --- a/spinifex/handlers/ec2/placementgroup/service_impl_test.go +++ b/spinifex/handlers/ec2/placementgroup/service_impl_test.go @@ -121,7 +121,7 @@ func TestCreatePlacementGroup_DuplicateName(t *testing.T) { Strategy: aws.String("spread"), }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidPlacementGroupDuplicate, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidPlacementGroupDuplicate, "The placement group 'dup-group' already exists.") } func TestCreatePlacementGroup_PartitionRejected(t *testing.T) { @@ -243,7 +243,7 @@ func TestDeletePlacementGroup_NotFound(t *testing.T) { GroupName: aws.String("nonexistent"), }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidPlacementGroupUnknown, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidPlacementGroupUnknown, "The placement group 'nonexistent' is unknown.") } func TestDeletePlacementGroup_InUse(t *testing.T) { @@ -380,7 +380,7 @@ func TestDescribePlacementGroups_NameNotFound(t *testing.T) { GroupNames: []*string{aws.String("ghost")}, }, testAccountID) require.Error(t, err) - assert.Equal(t, awserrors.ErrorInvalidPlacementGroupUnknown, err.Error()) + assertAWSError(t, err, awserrors.ErrorInvalidPlacementGroupUnknown, "The Placement Group 'ghost' is unknown.") } func TestDescribePlacementGroups_AccountScoped(t *testing.T) { @@ -906,3 +906,12 @@ func TestRemoveInstance_ReadFailureIsNotSuccess(t *testing.T) { require.Error(t, err) assert.True(t, awserrors.IsErrorCode(err, awserrors.ErrorServerInternal), "got %v", err) } + +// assertAWSError checks the code a client receives and the message with it. +func assertAWSError(t *testing.T, err error, wantCode, wantMessage string) { + t.Helper() + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "unresolvable error: %v", err) + assert.Equal(t, wantCode, code) + assert.Equal(t, wantMessage, msg) +}