From 17231be70b3110a7aec0ffca236c79cbd78ad991 Mon Sep 17 00:00:00 2001 From: Mohamed Elasmar <71043312+moelasmar@users.noreply.github.com> Date: Wed, 18 Dec 2024 15:00:55 -0800 Subject: [PATCH] fix(service-spec): skip adding a resource property that could not be resolved (#1508) Fixes: if the CFN schema contains some resource or type that contains a property of type that could not be resolved (like by mistake the type definition does not exist in the schema), now the specs importer failed to continue processing the remaining properties, and types defined in this resource/type. This PR is to only skip the property that has issues, and continue importing everything else. --- .../import-cloudformation-registry.ts | 24 ++++++++------ .../test/cloudformation-registry.test.ts | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/service-spec-importers/src/importers/import-cloudformation-registry.ts b/packages/@aws-cdk/service-spec-importers/src/importers/import-cloudformation-registry.ts index 6829b7cae..17aeb814e 100644 --- a/packages/@aws-cdk/service-spec-importers/src/importers/import-cloudformation-registry.ts +++ b/packages/@aws-cdk/service-spec-importers/src/importers/import-cloudformation-registry.ts @@ -71,16 +71,22 @@ export function importCloudFormationRegistryResource(options: LoadCloudFormation const required = calculateDefinitelyRequired(source); for (const [name, property] of Object.entries(source.properties)) { - let resolvedSchema = resolve(property); - - withResult(schemaTypeToModelType(name, resolvedSchema, fail.in(`property ${name}`)), (type) => { - target.setProperty(name, { - type, - documentation: descriptionOf(resolvedSchema), - required: required.has(name), - defaultValue: describeDefault(resolvedSchema), + try { + let resolvedSchema = resolve(property); + withResult(schemaTypeToModelType(name, resolvedSchema, fail.in(`property ${name}`)), (type) => { + target.setProperty(name, { + type, + documentation: descriptionOf(resolvedSchema), + required: required.has(name), + defaultValue: describeDefault(resolvedSchema), + }); }); - }); + } catch (e) { + report.reportFailure( + 'interpreting', + fail(`Skip generating property ${name} for resource ${resource.typeName} because of ${e}`), + ); + } } } diff --git a/packages/@aws-cdk/service-spec-importers/test/cloudformation-registry.test.ts b/packages/@aws-cdk/service-spec-importers/test/cloudformation-registry.test.ts index 120cf3c01..e5ea34711 100644 --- a/packages/@aws-cdk/service-spec-importers/test/cloudformation-registry.test.ts +++ b/packages/@aws-cdk/service-spec-importers/test/cloudformation-registry.test.ts @@ -84,6 +84,38 @@ test('type definitions in deprecated properties do not fail', () => { // THEN: no failure }); +test('referring to non-exist type do not fail', () => { + importCloudFormationRegistryResource({ + db, + report, + resource: { + typeName: 'AWS::Some::Type', + description: 'resource with property has type non exist', + properties: { + id: { $ref: '#/definitions/Type' }, + prop: { $ref: '#/definitions/NotExistType' }, + prop2: { $ref: '#/definitions/Type' }, + }, + definitions: { + Type: { + type: 'object', + properties: { + id: { + type: 'string', + }, + }, + }, + }, + }, + }); + + // THEN: + const resource = db.lookup('resource', 'cloudFormationType', 'equals', 'AWS::Some::Type').only(); + expect(Object.keys(resource.properties)).toContain('id'); + expect(Object.keys(resource.properties)).toContain('prop2'); + expect(Object.keys(resource.properties)).not.toContain('prop'); +}); + test('empty objects are treated as "any"', () => { importCloudFormationRegistryResource({ db,