diff --git a/.changes/next-release/bugfix-cfn-resource-import-1772138230.json b/.changes/next-release/bugfix-cfn-resource-import-1772138230.json new file mode 100644 index 00000000000..69f8c7df2a8 --- /dev/null +++ b/.changes/next-release/bugfix-cfn-resource-import-1772138230.json @@ -0,0 +1,4 @@ +{ + "type": "bugfix", + "description": "Display AWS error message when importing CloudFormation resource state fails. Currently users only see 'Failed to import * resource(s)'." +} diff --git a/packages/core/src/awsService/cloudformation/resources/resourceRequestTypes.ts b/packages/core/src/awsService/cloudformation/resources/resourceRequestTypes.ts index 9b64782526f..354705eda34 100644 --- a/packages/core/src/awsService/cloudformation/resources/resourceRequestTypes.ts +++ b/packages/core/src/awsService/cloudformation/resources/resourceRequestTypes.ts @@ -68,6 +68,7 @@ export interface ResourceStateResult { completionItem?: CompletionItem successfulImports: Map failedImports: Map + failureReasons?: Record> warning?: string } @@ -95,6 +96,7 @@ export type SearchResourceParams = { export type SearchResourceResult = { found: boolean resource?: ResourceList + error?: string } export const SearchResourceRequest = new RequestType( diff --git a/packages/core/src/awsService/cloudformation/resources/resourcesManager.ts b/packages/core/src/awsService/cloudformation/resources/resourcesManager.ts index bdba551c7cd..19e3b053e84 100644 --- a/packages/core/src/awsService/cloudformation/resources/resourcesManager.ts +++ b/packages/core/src/awsService/cloudformation/resources/resourcesManager.ts @@ -267,7 +267,7 @@ export class ResourcesManager { } await this.applyCompletionSnippet(result) const [successCount, failureCount] = this.getSuccessAndFailureCount(result) - this.renderResultMessage(successCount, failureCount, purpose) + this.renderResultMessage(successCount, failureCount, purpose, result.failureReasons) } ) } catch (error) { @@ -392,22 +392,41 @@ export class ResourcesManager { return this.getResourcesToImportInput(selections) } - private renderResultMessage(successCount: number, failureCount: number, purpose: ResourceStatePurpose) { + private renderResultMessage( + successCount: number, + failureCount: number, + purpose: ResourceStatePurpose, + failureReasons?: Record> + ) { const action = purpose === ResourceStatePurpose.Import ? 'imported' : 'cloned' + const reasonsSuffix = this.formatFailureReasons(failureReasons) if (successCount > 0 && failureCount === 0) { void window.showInformationMessage(`Successfully ${action} ${successCount} resource(s)`) } else if (successCount > 0 && failureCount > 0) { void window.showWarningMessage( - `${action.charAt(0).toUpperCase() + action.slice(1)} ${successCount} resource(s), ${failureCount} failed` + `${action.charAt(0).toUpperCase() + action.slice(1)} ${successCount} resource(s), ${failureCount} failed${reasonsSuffix}` ) } else if (failureCount > 0) { - showErrorMessage(`Failed to ${action.replace('ed', '')} ${failureCount} resource(s)`) + showErrorMessage(`Failed to ${action.replace('ed', '')} ${failureCount} resource(s)${reasonsSuffix}`) } else { void window.showInformationMessage(`No resources were ${action}`) } } + private formatFailureReasons(failureReasons?: Record>): string { + if (!failureReasons) { + return '' + } + const reasons: string[] = [] + for (const [, identifiers] of Object.entries(failureReasons)) { + for (const [id, reason] of Object.entries(identifiers)) { + reasons.push(`[${id}: ${reason}]`) + } + } + return reasons.length > 0 ? `: ${reasons.join(', ')}` : '' + } + private getResourcesArray(): ResourceList[] { return Array.from(this.resources.values()) } diff --git a/packages/core/src/awsService/cloudformation/ui/resourceSelector.ts b/packages/core/src/awsService/cloudformation/ui/resourceSelector.ts index 9444896a489..28eb4442c09 100644 --- a/packages/core/src/awsService/cloudformation/ui/resourceSelector.ts +++ b/packages/core/src/awsService/cloudformation/ui/resourceSelector.ts @@ -143,9 +143,10 @@ export class ResourceSelector { this.refreshCallback?.() if (!result.found) { - void window.showErrorMessage( - `${resourceType} with identifier '${identifier}' was not found. The identifier must match exactly.` - ) + const reason = result.error + ? `${result.error}` + : `${resourceType} with identifier '${identifier}' was not found` + void window.showErrorMessage(reason) return [] } diff --git a/packages/core/src/test/awsService/cloudformation/resources/resourcesManager.test.ts b/packages/core/src/test/awsService/cloudformation/resources/resourcesManager.test.ts index b1ac434c5f0..f9d9e0bc3ab 100644 --- a/packages/core/src/test/awsService/cloudformation/resources/resourcesManager.test.ts +++ b/packages/core/src/test/awsService/cloudformation/resources/resourcesManager.test.ts @@ -7,10 +7,16 @@ import * as assert from 'assert' import * as sinon from 'sinon' import { ResourcesManager } from '../../../../awsService/cloudformation/resources/resourcesManager' import { ResourceSelector } from '../../../../awsService/cloudformation/ui/resourceSelector' -import { ResourceStateResult } from '../../../../awsService/cloudformation/resources/resourceRequestTypes' +import { + ResourceStateResult, + ResourceStatePurpose, +} from '../../../../awsService/cloudformation/resources/resourceRequestTypes' import { Range, SnippetString, TextEditor, window } from 'vscode' import { getLogger } from '../../../../shared/logger' import globals from '../../../../shared/extensionGlobals' +import * as setContextModule from '../../../../shared/vscode/setContext' +import { getTestWindow } from '../../../shared/vscode/window' +import { SeverityLevel } from '../../../shared/vscode/message' describe('ResourcesManager - applyCompletionSnippet', () => { let sandbox: sinon.SinonSandbox @@ -277,3 +283,134 @@ describe('ResourcesManager - removeResourceType', () => { assert.deepStrictEqual(updatedTypes, ['AWS::S3::Bucket', 'AWS::Lambda::Function']) }) }) + +describe('ResourcesManager - formatFailureReasons and renderResultMessage', () => { + let sandbox: sinon.SinonSandbox + let mockClient: any + let mockResourceSelector: ResourceSelector + let resourcesManager: ResourcesManager + + beforeEach(() => { + sandbox = sinon.createSandbox() + mockClient = { sendRequest: sandbox.stub() } + mockResourceSelector = {} as ResourceSelector + resourcesManager = new ResourcesManager(mockClient, mockResourceSelector) + }) + + afterEach(() => { + sandbox.restore() + }) + + it('should return empty string when failureReasons is undefined', () => { + const result = (resourcesManager as any).formatFailureReasons(undefined) + assert.strictEqual(result, '') + }) + + it('should return empty string when failureReasons is empty', () => { + const result = (resourcesManager as any).formatFailureReasons({}) + assert.strictEqual(result, '') + }) + + it('should format single failure reason', () => { + const failureReasons = { + 'AWS::S3::Bucket': { 'my-bucket': 'Resource not found' }, + } + const result = (resourcesManager as any).formatFailureReasons(failureReasons) + assert.strictEqual(result, ': [my-bucket: Resource not found]') + }) + + it('should format multiple failure reasons across resource types', () => { + const failureReasons = { + 'AWS::S3::Bucket': { 'my-bucket': 'Resource not found' }, + 'AWS::Lambda::Function': { 'my-func': 'Access denied' }, + } + const result = (resourcesManager as any).formatFailureReasons(failureReasons) + assert.strictEqual(result, ': [my-bucket: Resource not found], [my-func: Access denied]') + }) + + it('should format multiple identifiers within same resource type', () => { + const failureReasons = { + 'AWS::S3::Bucket': { 'bucket-1': 'Not found', 'bucket-2': 'Permission denied' }, + } + const result = (resourcesManager as any).formatFailureReasons(failureReasons) + assert.strictEqual(result, ': [bucket-1: Not found], [bucket-2: Permission denied]') + }) + + it('should append failure reasons to warning message on partial failure', () => { + const failureReasons = { 'AWS::S3::Bucket': { 'my-bucket': 'Not found' } } + ;(resourcesManager as any).renderResultMessage(1, 1, ResourceStatePurpose.Import, failureReasons) + const message = getTestWindow().getFirstMessage() + assert.ok(message.message.includes('[my-bucket: Not found]')) + message.assertSeverity(SeverityLevel.Warning) + }) + + it('should append failure reasons to error message on full failure', () => { + const failureReasons = { 'AWS::S3::Bucket': { 'my-bucket': 'Access denied' } } + ;(resourcesManager as any).renderResultMessage(0, 1, ResourceStatePurpose.Import, failureReasons) + const message = getTestWindow().getFirstMessage() + assert.ok(message.message.includes('[my-bucket: Access denied]')) + message.assertSeverity(SeverityLevel.Error) + }) + + it('should not append reasons suffix when failureReasons is undefined', () => { + ;(resourcesManager as any).renderResultMessage(0, 1, ResourceStatePurpose.Import, undefined) + const message = getTestWindow().getFirstMessage() + assert.strictEqual(message.message, 'Failed to import 1 resource(s)') + message.assertSeverity(SeverityLevel.Error) + }) + + it('should show success message without reasons suffix', () => { + ;(resourcesManager as any).renderResultMessage(2, 0, ResourceStatePurpose.Import, undefined) + const message = getTestWindow().getFirstMessage() + assert.strictEqual(message.message, 'Successfully imported 2 resource(s)') + message.assertSeverity(SeverityLevel.Information) + }) + + it('should use cloned action with failure reasons', () => { + const failureReasons = { 'AWS::S3::Bucket': { 'my-bucket': 'Access denied' } } + ;(resourcesManager as any).renderResultMessage(0, 1, ResourceStatePurpose.Clone, failureReasons) + const message = getTestWindow().getFirstMessage() + assert.ok(message.message.includes('clone')) + assert.ok(message.message.includes('[my-bucket: Access denied]')) + message.assertSeverity(SeverityLevel.Error) + }) + + it('should show no resources cloned message', () => { + ;(resourcesManager as any).renderResultMessage(0, 0, ResourceStatePurpose.Clone, undefined) + const message = getTestWindow().getFirstMessage() + assert.strictEqual(message.message, 'No resources were cloned') + message.assertSeverity(SeverityLevel.Information) + }) + + describe('importResourceStates end-to-end', () => { + beforeEach(() => { + sandbox.stub(setContextModule, 'setContext').resolves() + sandbox.stub(window, 'activeTextEditor').get(() => ({ + insertSnippet: sandbox.stub().resolves(true), + edit: sandbox.stub().resolves(true), + document: { + uri: { toString: () => 'file:///test.yaml' }, + lineCount: 100, + lineAt: sandbox.stub().returns({ range: { end: { line: 99, character: 0 } } }), + }, + })) + sandbox.stub(getLogger(), 'warn') + sandbox.stub(getLogger(), 'info') + }) + + it('should pass failureReasons through the full import flow', async () => { + mockClient.sendRequest.resolves({ + successfulImports: {}, + failedImports: { 'AWS::S3::Bucket': ['my-bucket'] }, + failureReasons: { 'AWS::S3::Bucket': { 'my-bucket': 'Resource not found in account' } }, + }) + + const resourceNodes = [{ resourceType: 'AWS::S3::Bucket', resourceIdentifier: 'my-bucket' }] as any + await resourcesManager.importResourceStates(resourceNodes) + + const messages = getTestWindow().shownMessages.filter((m) => m.message !== 'Importing Resource State') + assert.ok(messages.length > 0, `No non-progress messages shown`) + assert.ok(messages[0].message.includes('[my-bucket: Resource not found in account]')) + }) + }) +}) diff --git a/packages/core/src/test/awsService/cloudformation/ui/resourceSelector.test.ts b/packages/core/src/test/awsService/cloudformation/ui/resourceSelector.test.ts new file mode 100644 index 00000000000..f762fe5ac9d --- /dev/null +++ b/packages/core/src/test/awsService/cloudformation/ui/resourceSelector.test.ts @@ -0,0 +1,75 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as assert from 'assert' +import * as sinon from 'sinon' +import { ResourceSelector, ResourceOperations } from '../../../../awsService/cloudformation/ui/resourceSelector' +import { getTestWindow } from '../../../shared/vscode/window' +import { SeverityLevel } from '../../../shared/vscode/message' + +describe('ResourceSelector - search error handling', () => { + let sandbox: sinon.SinonSandbox + let mockClient: any + let resourceSelector: ResourceSelector + let mockResourceOperations: ResourceOperations + + beforeEach(() => { + sandbox = sinon.createSandbox() + mockClient = { sendRequest: sandbox.stub() } + resourceSelector = new ResourceSelector(mockClient) + + mockResourceOperations = { + getCached: sandbox.stub().returns({ + typeName: 'AWS::S3::Bucket', + resourceIdentifiers: ['id1'], + nextToken: 'token', + }), + loadMore: sandbox.stub().resolves(), + search: sandbox.stub(), + } + + // Setup UI interactions: select "Search" from quick pick, then enter identifier + getTestWindow().onDidShowQuickPick((picker) => { + const searchItem = picker.items.find((i) => i.label.includes('Search')) + if (searchItem) { + picker.acceptItem(searchItem) + } + }) + getTestWindow().onDidShowInputBox((input) => { + input.acceptValue('my-resource-id') + }) + }) + + afterEach(() => { + sandbox.restore() + }) + + it('should display result.error when search fails with error message', async () => { + ;(mockResourceOperations.search as sinon.SinonStub).resolves({ + found: false, + error: 'Resource does not exist in the account', + }) + + const result = await resourceSelector.selectResources(true, ['AWS::S3::Bucket'], mockResourceOperations) + + const message = getTestWindow().getFirstMessage() + assert.strictEqual(message.message, 'Resource does not exist in the account') + message.assertSeverity(SeverityLevel.Error) + assert.deepStrictEqual(result, []) + }) + + it('should display default not-found message when search fails without error', async () => { + ;(mockResourceOperations.search as sinon.SinonStub).resolves({ + found: false, + }) + + const result = await resourceSelector.selectResources(true, ['AWS::S3::Bucket'], mockResourceOperations) + + const message = getTestWindow().getFirstMessage() + assert.strictEqual(message.message, "AWS::S3::Bucket with identifier 'my-resource-id' was not found") + message.assertSeverity(SeverityLevel.Error) + assert.deepStrictEqual(result, []) + }) +})