Skip to content

Commit e61a854

Browse files
gemammercadogemammercado
committed
return import failure message as object
1 parent 28068f8 commit e61a854

6 files changed

Lines changed: 120 additions & 79 deletions

File tree

‎src/autocomplete/ResourceStateCompletionProvider.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ export class ResourceStateCompletionProvider implements CompletionProvider {
5555
log.info(`Retrieving resource details from AWS account with id: ${identifier} and type: ${resource.Type}`);
5656
let properties: string;
5757
try {
58-
const resourceState = await this.resourceStateManager.getResource(resource.Type, identifier);
59-
if (!resourceState) {
58+
const result = await this.resourceStateManager.getResource(resource.Type, identifier);
59+
if (!result.resource) {
6060
return [];
6161
}
62-
properties = resourceState.properties;
62+
properties = result.resource.properties;
6363
} catch {
6464
log.info(`No resource found for id: ${identifier} and type: ${resource.Type}`);
6565
return [];

‎src/resourceState/ResourceStateImporter.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export class ResourceStateImporter {
157157
parentResourceType?: string,
158158
): Promise<{ fetchedResourceStates: ResourceTemplateFormat[]; importResult: ResourceStateResult }> {
159159
const fetchedResourceStates: ResourceTemplateFormat[] = [];
160+
const failureReasons: Record<string, Record<string, string>> = {};
160161
const importResult: ResourceStateResult = {
161162
completionItem: undefined,
162163
failedImports: {},
@@ -176,9 +177,13 @@ export class ResourceStateImporter {
176177
}
177178
for (const resourceIdentifier of resourceSelection.resourceIdentifiers) {
178179
try {
179-
const resourceState = await this.resourceStateManager.getResource(resourceType, resourceIdentifier);
180-
if (!resourceState) {
180+
const result = await this.resourceStateManager.getResource(resourceType, resourceIdentifier);
181+
if (!result.resource) {
181182
this.getOrCreate(importResult.failedImports, resourceType, []).push(resourceIdentifier);
183+
if (result.error) {
184+
failureReasons[resourceType] ??= {};
185+
failureReasons[resourceType][resourceIdentifier] = result.error;
186+
}
182187
continue;
183188
}
184189
this.getOrCreate(importResult.successfulImports, resourceType, []).push(resourceIdentifier);
@@ -195,20 +200,27 @@ export class ResourceStateImporter {
195200
Type: resourceType,
196201
DeletionPolicy:
197202
purpose === ResourceStatePurpose.IMPORT ? DeletionPolicyOnImport : undefined,
198-
Properties: this.applyTransformations(resourceState.properties, schema, purpose, logicalId),
203+
Properties: this.applyTransformations(
204+
result.resource.properties,
205+
schema,
206+
purpose,
207+
logicalId,
208+
),
199209
Metadata: await this.createMetadata(resourceIdentifier, purpose),
200210
},
201211
});
202212
} catch (error) {
203213
log.error(error, `Error importing resource state for ${resourceType} id: ${resourceIdentifier}`);
204214
this.getOrCreate(importResult.failedImports, resourceType, []).push(resourceIdentifier);
205215
const errorMessage = extractErrorMessage(error);
206-
importResult.failureReasons ??= {};
207-
importResult.failureReasons[resourceType] ??= {};
208-
importResult.failureReasons[resourceType][resourceIdentifier] = errorMessage;
216+
failureReasons[resourceType] ??= {};
217+
failureReasons[resourceType][resourceIdentifier] = errorMessage;
209218
}
210219
}
211220
}
221+
if (Object.keys(failureReasons).length > 0) {
222+
importResult.failureReasons = failureReasons;
223+
}
212224
return { fetchedResourceStates, importResult };
213225
}
214226

‎src/resourceState/ResourceStateManager.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { ScopedTelemetry } from '../telemetry/ScopedTelemetry';
1515
import { Telemetry, Measure, Count } from '../telemetry/TelemetryDecorator';
1616
import { isClientError } from '../utils/AwsErrorMapper';
1717
import { Closeable } from '../utils/Closeable';
18+
import { extractErrorMessage } from '../utils/Errors';
1819
import { NO_LIST_SUPPORT, REQUIRES_RESOURCE_MODEL } from './ListResourcesExclusionTypes';
1920
import { ListResourcesResult, RefreshResourcesResult } from './ResourceStateTypes';
2021

@@ -27,6 +28,8 @@ export type ResourceState = {
2728
createdTimestamp: DateTime;
2829
};
2930

31+
export type GetResourceResult = { resource?: ResourceState; error?: string };
32+
3033
type ResourceList = {
3134
typeName: string;
3235
resourceIdentifiers: string[];
@@ -60,11 +63,11 @@ export class ResourceStateManager implements SettingsConfigurable, Closeable {
6063
}
6164

6265
@Measure({ name: 'getResource', captureErrorType: true })
63-
public async getResource(typeName: ResourceType, identifier: ResourceId): Promise<ResourceState | undefined> {
66+
public async getResource(typeName: ResourceType, identifier: ResourceId): Promise<GetResourceResult> {
6467
const cachedResources = this.getResourceState(typeName, identifier);
6568
if (cachedResources) {
6669
this.telemetry.count('state.hit', 1);
67-
return cachedResources;
70+
return { resource: cachedResources };
6871
}
6972

7073
this.telemetry.count('state.miss', 1);
@@ -76,19 +79,20 @@ export class ResourceStateManager implements SettingsConfigurable, Closeable {
7679
} catch (error) {
7780
if (error instanceof ResourceNotFoundException) {
7881
log.info(`No resource found for type ${typeName} and identifier "${identifier}"`);
82+
return {};
7983
} else if (isClientError(error)) {
8084
log.info(`Client error for type ${typeName} and identifier "${identifier}"`);
81-
} else {
82-
log.error(error, `CCAPI GetResource failed for type ${typeName} and identifier "${identifier}"`);
85+
return { error: extractErrorMessage(error) };
8386
}
87+
log.error(error, `CCAPI GetResource failed for type ${typeName} and identifier "${identifier}"`);
8488
throw error;
8589
}
8690

8791
if (!output?.TypeName || !output?.ResourceDescription?.Identifier || !output?.ResourceDescription?.Properties) {
8892
log.error(
8993
`GetResource output is missing required fields for type ${typeName} with identifier "${identifier}"`,
9094
);
91-
return;
95+
return { error: 'GetResource output is missing required fields' };
9296
}
9397

9498
const value: ResourceState = {
@@ -99,7 +103,7 @@ export class ResourceStateManager implements SettingsConfigurable, Closeable {
99103
};
100104

101105
this.storeResourceState(typeName, identifier, value);
102-
return value;
106+
return { resource: value };
103107
}
104108

105109
@Measure({ name: 'listResources' })
@@ -136,12 +140,17 @@ export class ResourceStateManager implements SettingsConfigurable, Closeable {
136140
typeName: string,
137141
identifier: string,
138142
): Promise<{ found: boolean; resourceList?: ResourceList }> {
143+
let result: GetResourceResult;
139144
try {
140-
await this.getResource(typeName, identifier);
145+
result = await this.getResource(typeName, identifier);
141146
} catch {
142147
return { found: false };
143148
}
144149

150+
if (!result.resource) {
151+
return { found: false };
152+
}
153+
145154
// Add to cache
146155
const cached = this.resourceListMap.get(typeName);
147156
if (cached && !cached.resourceIdentifiers.includes(identifier)) {

‎tst/unit/autocomplete/ResourceStateCompletionProvider.test.ts‎

Lines changed: 50 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ describe('ResourceStateCompletionProvider', () => {
115115
});
116116

117117
mockComponents.schemaRetriever.getDefault.returns(s3Schemas);
118-
mockComponents.resourceStateManager.getResource.rejects(new Error('Resource not found'));
118+
mockComponents.resourceStateManager.getResource.resolves({ error: 'Resource not found' });
119119

120120
const result = await provider.getCompletions(context, mockYamlParams);
121121

@@ -152,14 +152,16 @@ describe('ResourceStateCompletionProvider', () => {
152152
emptySchemas.schemas.set('Custom::Type', customTypeSchema);
153153
mockComponents.schemaRetriever.getDefault.returns(emptySchemas);
154154
mockComponents.resourceStateManager.getResource.resolves({
155-
typeName: 'Custom::Type',
156-
identifier: 'test',
157-
properties: JSON.stringify({
158-
BucketName: 'test',
159-
VersioningConfiguration: { Status: 'Enabled' },
160-
ExistingProp: 'value',
161-
}),
162-
createdTimestamp: new Date() as any,
155+
resource: {
156+
typeName: 'Custom::Type',
157+
identifier: 'test',
158+
properties: JSON.stringify({
159+
BucketName: 'test',
160+
VersioningConfiguration: { Status: 'Enabled' },
161+
ExistingProp: 'value',
162+
}),
163+
createdTimestamp: new Date() as any,
164+
},
163165
});
164166

165167
const result = await provider.getCompletions(context, mockYamlParams);
@@ -187,13 +189,15 @@ describe('ResourceStateCompletionProvider', () => {
187189
emptySchemas.schemas.set('Custom::Type', customTypeSchema);
188190
mockComponents.schemaRetriever.getDefault.returns(emptySchemas);
189191
mockComponents.resourceStateManager.getResource.resolves({
190-
typeName: 'Custom::Type',
191-
identifier: 'test',
192-
properties: JSON.stringify({
193-
BucketName: 'test',
194-
VersioningConfiguration: { Status: 'Enabled' },
195-
}),
196-
createdTimestamp: new Date() as any,
192+
resource: {
193+
typeName: 'Custom::Type',
194+
identifier: 'test',
195+
properties: JSON.stringify({
196+
BucketName: 'test',
197+
VersioningConfiguration: { Status: 'Enabled' },
198+
}),
199+
createdTimestamp: new Date() as any,
200+
},
197201
});
198202

199203
const result = await provider.getCompletions(context, mockYamlParams);
@@ -268,10 +272,12 @@ describe('ResourceStateCompletionProvider', () => {
268272
});
269273

270274
mockComponents.resourceStateManager.getResource.resolves({
271-
typeName: 'AWS::IAM::Role',
272-
identifier: 'Admin',
273-
properties: `{"Path":"/","ManagedPolicyArns":["arn:aws:iam::aws:policy/AdministratorAccess"],"MaxSessionDuration":43200,"RoleName":"Admin","AssumeRolePolicyDocument":{"Version":"2012-10-17","Statement":[{"Condition":{"StringEquals":{"sts:ExternalId":"IsengardExternalIdAKj8duTfSqL6"}},"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"AWS":"arn:aws:iam::727820809195:root"},"Sid":""}]},"Arn":"arn:aws:iam::783764615233:role/Admin","RoleId":"AROA3M7AC6BAWIZG2LLQY"}`,
274-
createdTimestamp: DateTime.now(),
275+
resource: {
276+
typeName: 'AWS::IAM::Role',
277+
identifier: 'Admin',
278+
properties: `{"Path":"/","ManagedPolicyArns":["arn:aws:iam::aws:policy/AdministratorAccess"],"MaxSessionDuration":43200,"RoleName":"Admin","AssumeRolePolicyDocument":{"Version":"2012-10-17","Statement":[{"Condition":{"StringEquals":{"sts:ExternalId":"IsengardExternalIdAKj8duTfSqL6"}},"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"AWS":"arn:aws:iam::727820809195:root"},"Sid":""}]},"Arn":"arn:aws:iam::783764615233:role/Admin","RoleId":"AROA3M7AC6BAWIZG2LLQY"}`,
279+
createdTimestamp: DateTime.now(),
280+
},
275281
});
276282

277283
const result = await provider.getCompletions(context, mockJsonParams);
@@ -307,10 +313,12 @@ describe('ResourceStateCompletionProvider', () => {
307313
});
308314

309315
mockComponents.resourceStateManager.getResource.resolves({
310-
typeName: 'AWS::IAM::Role',
311-
identifier: 'Admin',
312-
properties: `{"Path":"/","ManagedPolicyArns":["arn:aws:iam::aws:policy/AdministratorAccess"],"MaxSessionDuration":43200,"RoleName":"Admin","AssumeRolePolicyDocument":{"Version":"2012-10-17","Statement":[{"Condition":{"StringEquals":{"sts:ExternalId":"IsengardExternalIdAKj8duTfSqL6"}},"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"AWS":"arn:aws:iam::727820809195:root"},"Sid":""}]},"Arn":"arn:aws:iam::783764615233:role/Admin","RoleId":"AROA3M7AC6BAWIZG2LLQY"}`,
313-
createdTimestamp: DateTime.now(),
316+
resource: {
317+
typeName: 'AWS::IAM::Role',
318+
identifier: 'Admin',
319+
properties: `{"Path":"/","ManagedPolicyArns":["arn:aws:iam::aws:policy/AdministratorAccess"],"MaxSessionDuration":43200,"RoleName":"Admin","AssumeRolePolicyDocument":{"Version":"2012-10-17","Statement":[{"Condition":{"StringEquals":{"sts:ExternalId":"IsengardExternalIdAKj8duTfSqL6"}},"Action":"sts:AssumeRole","Effect":"Allow","Principal":{"AWS":"arn:aws:iam::727820809195:root"},"Sid":""}]},"Arn":"arn:aws:iam::783764615233:role/Admin","RoleId":"AROA3M7AC6BAWIZG2LLQY"}`,
320+
createdTimestamp: DateTime.now(),
321+
},
314322
});
315323

316324
const result = await provider.getCompletions(context, mockYamlParams);
@@ -459,13 +467,15 @@ describe('ResourceStateCompletionProvider', () => {
459467
emptySchemas.schemas.set('Custom::Type', customTypeSchema);
460468
mockComponents.schemaRetriever.getDefault.returns(emptySchemas);
461469
mockComponents.resourceStateManager.getResource.resolves({
462-
typeName: 'Custom::Type',
463-
identifier: 'test',
464-
properties: JSON.stringify({
465-
BucketName: 'test',
466-
VersioningConfiguration: { Status: 'Enabled' },
467-
}),
468-
createdTimestamp: new Date() as any,
470+
resource: {
471+
typeName: 'Custom::Type',
472+
identifier: 'test',
473+
properties: JSON.stringify({
474+
BucketName: 'test',
475+
VersioningConfiguration: { Status: 'Enabled' },
476+
}),
477+
createdTimestamp: new Date() as any,
478+
},
469479
});
470480

471481
const result = await provider.getCompletions(context, mockYamlParams);
@@ -490,13 +500,15 @@ describe('ResourceStateCompletionProvider', () => {
490500
mockComponents.schemaRetriever.getDefault.returns(emptySchemas);
491501
mockComponents.documentManager.getLine.returns('"",');
492502
mockComponents.resourceStateManager.getResource.resolves({
493-
typeName: 'Custom::Type',
494-
identifier: 'test',
495-
properties: JSON.stringify({
496-
BucketName: 'test',
497-
VersioningConfiguration: { Status: 'Enabled' },
498-
}),
499-
createdTimestamp: new Date() as any,
503+
resource: {
504+
typeName: 'Custom::Type',
505+
identifier: 'test',
506+
properties: JSON.stringify({
507+
BucketName: 'test',
508+
VersioningConfiguration: { Status: 'Enabled' },
509+
}),
510+
createdTimestamp: new Date() as any,
511+
},
500512
});
501513

502514
const result = await provider.getCompletions(context, mockJsonParams);

‎tst/unit/resourceState/ResourceStateImporter.test.ts‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ describe('ResourceStateImporter', () => {
9292
},
9393
];
9494

95-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
95+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
9696

9797
const params: ResourceStateParams = {
9898
resourceSelections,
@@ -136,7 +136,7 @@ describe('ResourceStateImporter', () => {
136136
},
137137
];
138138

139-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
139+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
140140

141141
const params: ResourceStateParams = {
142142
resourceSelections,
@@ -225,7 +225,7 @@ describe('ResourceStateImporter', () => {
225225

226226
createAndRegisterDocument(uri, scenario.initialContent, scenario.documentType);
227227

228-
mockResourceStateManager.getResource.mockRejectedValue(new Error('Access denied'));
228+
mockResourceStateManager.getResource.mockResolvedValue({ error: 'Access denied' });
229229

230230
const params: ResourceStateParams = {
231231
resourceSelections: [{ resourceType: 'AWS::S3::Bucket', resourceIdentifiers: ['my-bucket'] }],
@@ -246,7 +246,7 @@ describe('ResourceStateImporter', () => {
246246
createAndRegisterDocument(uri, scenario.initialContent, scenario.documentType);
247247

248248
const mockResource = createMockResourceState('AWS::S3::Bucket');
249-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
249+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
250250

251251
const params: ResourceStateParams = {
252252
resourceSelections: [
@@ -269,9 +269,9 @@ describe('ResourceStateImporter', () => {
269269

270270
const mockResource = createMockResourceState('AWS::S3::Bucket');
271271
mockResourceStateManager.getResource
272-
.mockResolvedValueOnce(mockResource)
273-
.mockRejectedValueOnce(new Error('Not found'))
274-
.mockRejectedValueOnce(new Error('Timeout'));
272+
.mockResolvedValueOnce({ resource: mockResource })
273+
.mockResolvedValueOnce({ error: 'Not found' })
274+
.mockResolvedValueOnce({ error: 'Timeout' });
275275

276276
const params: ResourceStateParams = {
277277
resourceSelections: [
@@ -306,7 +306,7 @@ describe('ResourceStateImporter', () => {
306306
});
307307

308308
const mockResource = createMockResourceState('AWS::S3::Bucket');
309-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
309+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
310310

311311
const params: ResourceStateParams = {
312312
resourceSelections: [
@@ -340,7 +340,7 @@ describe('ResourceStateImporter', () => {
340340
});
341341

342342
const mockResource = createMockResourceState('AWS::S3::Bucket');
343-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
343+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
344344

345345
const params: ResourceStateParams = {
346346
resourceSelections: [
@@ -370,7 +370,7 @@ describe('ResourceStateImporter', () => {
370370
});
371371

372372
const mockResource = createMockResourceState('AWS::S3::Bucket');
373-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
373+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
374374

375375
const params: ResourceStateParams = {
376376
resourceSelections: [
@@ -420,7 +420,7 @@ describe('ResourceStateImporter', () => {
420420
},
421421
];
422422

423-
mockResourceStateManager.getResource.mockResolvedValue(mockResource);
423+
mockResourceStateManager.getResource.mockResolvedValue({ resource: mockResource });
424424

425425
const params: ResourceStateParams = {
426426
resourceSelections,
@@ -460,9 +460,9 @@ describe('ResourceStateImporter', () => {
460460
];
461461

462462
mockResourceStateManager.getResource
463-
.mockResolvedValueOnce(mockResource1)
464-
.mockResolvedValueOnce(mockResource2)
465-
.mockResolvedValueOnce(mockResource3);
463+
.mockResolvedValueOnce({ resource: mockResource1 })
464+
.mockResolvedValueOnce({ resource: mockResource2 })
465+
.mockResolvedValueOnce({ resource: mockResource3 });
466466

467467
const params: ResourceStateParams = {
468468
resourceSelections,

0 commit comments

Comments
 (0)