Skip to content

Commit

Permalink
fix(service-spec): skip adding a resource property that could not be …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
moelasmar authored Dec 18, 2024
1 parent 76fa496 commit 17231be
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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}`),
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 17231be

Please sign in to comment.