Skip to content

Commit

Permalink
Set additionalProperties false for CFN objects
Browse files Browse the repository at this point in the history
This commit adds a CfnMapper that defaults the `additionalProperties`
property to `false` for named entries in a schema's `definitions` that
are of the `object` type. This is recommended behavior by CloudFormation
and, if not present, results in a warning when developing a resource.
  • Loading branch information
kstich committed Apr 8, 2021
1 parent dd04869 commit 744875e
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers;

import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;
import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.CfnMapper;
import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.Context;
import software.amazon.smithy.aws.cloudformation.schema.model.ResourceSchema;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.node.StringNode;

/**
* CloudFormation issues a warning in its build tooling every time it detects
* an "object" type in the converted JSON Schema's definitions that doesn't set
* "additionalProperties" to "false". This mapper sets that to reduce the
* warnings customers receive when developing resources from automatically
* generated resource schemas.
*
* @see <a href="https://github.com/aws-cloudformation/cloudformation-cli/blob/8491ac54d12028a7e00295b39f102e3786e52ccf/src/rpdk/core/data_loaders.py#L286-L295">Warning in CFN CLI</a>
*/
final class AdditionalPropertiesMapper implements CfnMapper {

@Override
public byte getOrder() {
// This is a desired behavior from CFN, so set it near the end
// while still having room for other overrides if necessary.
return 124;
}

@Override
public ObjectNode updateNode(Context context, ResourceSchema resourceSchema, ObjectNode node) {
Optional<ObjectNode> definitionsOptional = node.getObjectMember("definitions");

// This replaces every entry in the "definitions" map and uses a
// LinkedHashMap so that order is maintained, as entries will have
// been sorted before we get to this mapper.
Map<StringNode, Node> updatedNodes = new LinkedHashMap<>();
if (definitionsOptional.isPresent()) {
for (Map.Entry<StringNode, Node> entry : definitionsOptional.get().getMembers().entrySet()) {
ObjectNode valueNode = entry.getValue().expectObjectNode();

if (valueNode.expectStringMember("type").getValue().equals("object")) {
valueNode = valueNode.withMember("additionalProperties", false);
}

updatedNodes.put(entry.getKey(), valueNode);
}
node = node.withMember("definitions", Node.objectNode(updatedNodes));
}
return node;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public final class CoreExtension implements Smithy2CfnExtension {
@Override
public List<CfnMapper> getCfnMappers() {
return ListUtils.of(
new AdditionalPropertiesMapper(),
new DeprecatedMapper(),
new DocumentationMapper(),
new IdentifierMapper(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers;

import static org.junit.jupiter.api.Assertions.assertFalse;

import org.junit.jupiter.api.Test;
import software.amazon.smithy.aws.cloudformation.schema.CfnConfig;
import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.CfnConverter;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.shapes.ShapeId;

public class AdditionalPropertiesMapperTest {
@Test
public void setsAdditionalPropertiesFalse() {
Model model = Model.assembler()
.addImport(JsonAddTest.class.getResource("simple.smithy"))
.discoverModels()
.assemble()
.unwrap();

CfnConfig config = new CfnConfig();
config.setOrganizationName("Smithy");
config.setService(ShapeId.from("smithy.example#TestService"));

ObjectNode resourceNode = CfnConverter.create()
.config(config)
.convertToNodes(model)
.get("Smithy::TestService::FooResource");

assertFalse(resourceNode.expectObjectMember("definitions")
.expectObjectMember("ComplexProperty")
.expectBooleanMember("additionalProperties")
.getValue());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@
"property": {
"type": "string"
}
}
},
"additionalProperties": false
},
"FooMap": {
"type": "object",
"patternProperties": {
".+": {
"type": "string"
}
}
},
"additionalProperties": false
}
},
"properties": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@
".+": {
"type": "string"
}
}
},
"additionalProperties": false
},
"ComplexProperty": {
"type": "object",
"properties": {
"AnotherProperty": {
"type": "string"
}
}
},
"additionalProperties": false
}
},
"properties": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@
"DeadLetterTargetArn": {
"type": "string"
}
}
},
"additionalProperties": false
},
"TagMap": {
"type": "object",
"patternProperties": {
".+": {
"type": "string"
}
}
},
"additionalProperties": false
}
},
"properties": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"Another": {
"type": "string"
}
}
},
"additionalProperties": false
}
},
"properties": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@
"Another": {
"type": "string"
}
}
},
"additionalProperties": false
},
"FooMap": {
"type": "object",
"patternProperties": {
".+": {
"type": "string"
}
}
},
"additionalProperties": false
}
},
"properties": {
Expand Down

0 comments on commit 744875e

Please sign in to comment.