aws-cloudformation
CLI crashes when "oneOf" is used the in schema
When oneof is used the resource schema, the Cli crashes.
Here is an example of a resource schema that uses oneof:
{
"typeName": "brian::foo::me",
"description": "An example resource schema demonstrating some basic constructs and validation rules.",
"sourceUrl": "https://github.com/aws-cloudformation/aws-cloudformation-rpdk.git",
"definitions": {
"InitechDateFormat": {
"$comment": "Use the `definitions` block to provide shared resource property schemas",
"type": "string",
"format": "date-time"
},
"Memo": {
"type": "object",
"properties": {
"Heading": {
"type": "string"
},
"Body": {
"type": "string"
}
},
"additionalProperties": false
},
"Tag": {
"description": "A key-value pair to associate with a resource.",
"type": "object",
"properties": {
"Key": {
"type": "string",
"description": "The key name of the tag. You can specify a value that is 1 to 128 Unicode characters in length and cannot be prefixed with aws:. You can use any of the following characters: the set of Unicode letters, digits, whitespace, _, ., /, =, +, and -.",
"minLength": 1,
"maxLength": 128
},
"Value": {
"type": "string",
"description": "The value for the tag. You can specify a value that is 0 to 256 Unicode characters in length and cannot be prefixed with aws:. You can use any of the following characters: the set of Unicode letters, digits, whitespace, _, ., /, =, +, and -.",
"minLength": 0,
"maxLength": 256
}
},
"required": [
"Key",
"Value"
],
"additionalProperties": false
}
},
"properties": {
"Cookies": {
"oneOf": [
{
"additionalProperties": false,
"properties": {
"Forward": {
"description": "Specifies which cookies to forward to the origin for this cache behavior.",
"enum": [
"all",
"none"
],
"type": "string"
}
},
"required": [
"Forward"
]
},
{
"additionalProperties": false,
"properties": {
"Forward": {
"description": "Specifies which cookies to forward to the origin for this cache behavior.",
"enum": [
"whitelist"
],
"type": "string"
},
"WhitelistedNames": {
"description": "Required if you specify whitelist for the value of Forward.",
"items": {
"type": "string"
},
"minItems": 1,
"type": "array"
}
},
"required": [
"Forward",
"WhitelistedNames"
]
}
],
"type": "object"
}
},
"additionalProperties": false,
"required": [
"TestCode",
"Title"
],
"readOnlyProperties": [
"/properties/TPSCode"
],
"primaryIdentifier": [
"/properties/TPSCode"
],
"handlers": {
"create": {
"permissions": [
"initech:CreateReport"
]
},
"read": {
"permissions": [
"initech:DescribeReport"
]
},
"update": {
"permissions": [
"initech:UpdateReport"
]
},
"delete": {
"permissions": [
"initech:DeleteReport"
]
},
"list": {
"permissions": [
"initech:ListReports"
]
}
}
}
$ cfn generate
Explicitly specify value for insertionOrder for array: WhitelistedNames
Explicitly specify value for tagging
Resource schema is valid.
=== Unhandled exception ===
Please report this issue to the team.
Issue tracker: github.com/aws-cloudformation/cloudformation-cli/issues
Please include the log file 'rpdk.log'
rpdk.log
[2022-03-04T19:38:08Z] DEBUG - Logging set up successfully
[2022-03-04T19:38:08Z] DEBUG - Running generate: Namespace(version=False, subparser_name='generate', command=<function generate at 0x116803b80>, verbose=0, endpoint_url=None, region=None, target_schemas=[])
[2022-03-04T19:38:08Z] DEBUG - Root directory: /private/tmp/foo
[2022-03-04T19:38:08Z] DEBUG - Loading project file '/private/tmp/foo/.rpdk-config'
[2022-03-04T19:38:08Z] INFO - Validating your resource specification...
[2022-03-04T19:38:08Z] WARNING - Explicitly specify value for insertionOrder for array: WhitelistedNames
[2022-03-04T19:38:08Z] WARNING - Explicitly specify value for tagging
[2022-03-04T19:38:08Z] DEBUG - Rewriting refs in '<BASE>' (file:///private/tmp/foo/brian-foo-me.json)
[2022-03-04T19:38:08Z] WARNING - Resource schema is valid.
[2022-03-04T19:38:08Z] INFO - Validating your resource schema...
[2022-03-04T19:38:08Z] DEBUG - Writing Execution Role CloudFormation template: /private/tmp/foo/resource-role.yaml
[2022-03-04T19:38:08Z] DEBUG - Overwriting '/private/tmp/foo/resource-role.yaml'
[2022-03-04T19:38:08Z] DEBUG - Generate started
[2022-03-04T19:38:08Z] DEBUG - Removing generated sources: /private/tmp/foo/target/generated-sources/rpdk
[2022-03-04T19:38:08Z] DEBUG - Removing generated tests: /private/tmp/foo/target/generated-test-sources/rpdk
[2022-03-04T19:38:08Z] DEBUG - Making generated folder structure: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me
[2022-03-04T19:38:08Z] DEBUG - Making generated tests folder structure: /private/tmp/foo/target/generated-test-sources/rpdk/com/brian/foo/me
[2022-03-04T19:38:08Z] DEBUG - generate_resource started
[2022-03-04T19:38:08Z] DEBUG - Writing handler wrapper: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/HandlerWrapper.java
[2022-03-04T19:38:08Z] DEBUG - Overwriting '/private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/HandlerWrapper.java'
[2022-03-04T19:38:08Z] DEBUG - Writing handler wrapper: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/HandlerWrapperExecutable.java
[2022-03-04T19:38:08Z] DEBUG - Overwriting '/private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/HandlerWrapperExecutable.java'
[2022-03-04T19:38:08Z] DEBUG - Writing base configuration: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/BaseConfiguration.java
[2022-03-04T19:38:08Z] DEBUG - Overwriting '/private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/BaseConfiguration.java'
[2022-03-04T19:38:08Z] DEBUG - Writing base handler: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/BaseHandler.java
[2022-03-04T19:38:08Z] DEBUG - Overwriting '/private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/BaseHandler.java'
[2022-03-04T19:38:08Z] DEBUG - Writing 3 POJOs
[2022-03-04T19:38:08Z] DEBUG - ResourceModel POJO: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/ResourceModel.java
[2022-03-04T19:38:08Z] DEBUG - Overwriting '/private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/ResourceModel.java'
[2022-03-04T19:38:08Z] DEBUG - Cookies POJO: /private/tmp/foo/target/generated-sources/rpdk/com/brian/foo/me/Cookies.java
[2022-03-04T19:38:08Z] DEBUG - Unhandled exception
Traceback (most recent call last):
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/core/cli.py", line 100, in main
args.command(args)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/core/generate.py", line 15, in generate
project.generate(args.endpoint_url, args.region, args.target_schemas)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/core/project.py", line 558, in generate
self._plugin.generate(self)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/java/codegen.py", line 39, in wrapper
result = func(*args, **kwargs)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/java/codegen.py", line 449, in generate
self.generate_resource(src, project)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/java/codegen.py", line 39, in wrapper
result = func(*args, **kwargs)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/java/codegen.py", line 526, in generate_resource
contents = pojo_template.render(
File "/private/tmp/foo/env/lib/python3.9/site-packages/jinja2/environment.py", line 1090, in render
self.environment.handle_exception()
File "/private/tmp/foo/env/lib/python3.9/site-packages/jinja2/environment.py", line 832, in handle_exception
reraise(*rewrite_traceback_stack(source=source))
File "/private/tmp/foo/env/lib/python3.9/site-packages/jinja2/_compat.py", line 28, in reraise
raise value.with_traceback(tb)
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/java/templates/generate/POJO.java", line 22, in top-level template code
private {{ type|translate_type }} {{ name|lowercase_first_letter|safe_reserved }};
File "/private/tmp/foo/env/lib/python3.9/site-packages/rpdk/java/resolver.py", line 21, in translate_type
primitive_format = PRIMITIVE_TYPES[resolved_type.type][
TypeError: unhashable type: 'OrderedSet'
Hi Brian,
Other models solve this by putting the oneOf around required, that does mean that you would not be able to define what is required based on the value of another property. Additionally, I think you have to move the Cookies definition to the definitions section (nested properties are not allowed).
I see some difficulties with allowing oneOf in properties (especially for strongly typed languages). It might be solvable, but I think the generated code would become rather complex (technically it's allowed by the schema though, it would be nice if that changes so cfn validate catches this).
As an example: what code would you expect to be generated if you use oneOf to define a different type for one property in two different cases?
In the meantime, I think you can either only make Forward required, or do something like this, where you deviate more from the underlying API (there might be clearer ways to express the same idea)
{
"typeName": "brian::foo::me",
"description": "An example resource schema demonstrating some basic constructs and validation rules.",
"sourceUrl": "https://github.com/aws-cloudformation/aws-cloudformation-rpdk.git",
"definitions": {
"Cookies": {
"type": "object",
"additionalProperties": false,
"properties": {
"ForwardAll": {
"description": "Set to True to forward all cookies. Defaults to False",
"type": "boolean"
},
"WhitelistedNames": {
"description": "Whitelist of cookies to forward, ForwardAll must not be set to True.",
"items": {
"type": "string"
},
"minItems": 1,
"type": "array"
}
},
"oneOf": [
{
"required": [
"ForwardAll"
]
},
{
"required": [
"WhitelistedNames"
]
}
]
}
},
"properties": {
"Identifier": { "type": "string" },
"Cookies": {
"description": "Cookie configuration.",
"$ref": "#/definitions/Cookies"
}
},
"additionalProperties": false,
"required": [],
"primaryIdentifier": ["/properties/Identifier"],
"readOnlyProperties": ["/properties/Identifier"],
"handlers": {
"create": { "permissions": [] },
"read": { "permissions": [] },
"update": { "permissions": [] },
"delete": { "permissions": [] },
"list": { "permissions": [] }
}
}
Edit: I think in the example, you would end up not having anything that's required, but I'm leaving it like that, because it shows how to do "X or Y" is required
Hey Ben! I'm trying to use oneOf to allow the user to input two different subschemas; both are valid input.
It's called out in this example
I agree that the Cookies definition should be moved to the definitions section as a best practice; however, this is not a requirement for a valid schema, and the CLI should not break. I provided this simple example to show how the CLI fails when the oneOf keyword is used with a subschema.
In the example, when forward is all or none enums, those two states are valid, and WhitelistedNames should not be defined. When the user specifies Forward to be whitelist, WhitelistedNames must be defined.
example:
{
Forward: "all",
}
or
{
Forward: "whitelist",
WhitelistedNames[ "foo", "bar" ]
}
That's super interesting, and I see how that would work from a schema validation perspective.
I don't see yet how that would work for all possible schemas, for model generation (see an example below).
I don't have a solution for this, the best I can come up with is either making generation optional, or having two schema, one for generation and one for validation. Neither sounds very appealing. The former is a lot of work and error prone; for the latter the CLI would have to somehow validate that the validation schema is a strict subset of the generation schema (ie. you can reject more, but not accept more).
An example of how generation goes wrong, even with a valid schema:
This sub-schema
"definitions": {
"Cookies": {
"type": "object",
"additionalProperties": false,
"properties": {
"WhitelistedNames": {
"items": {"type": "string"},
"type": "array"
}
}
}
}
is responsible for this (java) code:
public class Cookies {
@JsonProperty("WhitelistedNames")
private List<String> whitelistedNames;
}
If you want to support extra options on the WhitelistedNames, you could write a schema like this:
"definitions": {
"Cookies": {
"type": "object",
"additionalProperties": false,
"properties": {
"WhitelistedNames": {
"items": {"$ref": "#/definitions/Name"},
"type": "array"
}
}
},
"Name": {
"type": "object",
"additionalProperties": false,
"properties": {
"Name": {"type": "string"},
"CaseSensitive": {"type": "boolean"}
}
}
}
which would give you these two pieces of code:
public class Cookies {
@JsonProperty("WhitelistedNames")
private List<Name> whitelistedNames;
}
public class Name {
@JsonProperty("Name")
private String name;
@JsonProperty("CaseSensitive")
private Boolean caseSensitive;
}
To preserve backward compatibility, you could combine those with oneOf, and the validator should be able to see which case is relevant for any input. However, I can't come up with java code that (for the general case) is better than
public class Cookies {
@JsonProperty("WhitelistedNames")
private Object whitelistedNames;
}
Of course, this is an extreme example, for the code you provided (with the enum selection), you could argue that generating the following snippet is possible (and I agree). Having an extra requirement of "within oneOf properties should have the same type" or "within oneOf all properties should be defined and have the same type" is more flexible than "within oneOf you can't specify properties" (and like you pointed out, the current spec doesn't have any restrictions)
In this case, whitelistedNames might be Null if Forward is all
public class Cookies {
@JsonProperty("WhitelistedNames")
private List<String> whitelistedNames;
@JsonProperty("Forward")
private String Forward;
}
However, this is not necessarily true in all languages. Eg. python dataclasses make a distinction between Nullable and non-nullable fields, that's still not impossible, but it makes for less-useful code
eg. the generated code would have to be:
class Cookies(BaseModel):
Forward: str
WhitelistedNames: Optional[Sequence[str]]
instead of one of the following for the case where Forward is whitelist
class Cookies(BaseModel):
Forward: str
WhitelistedNames: Sequence[str]
In conclusion:
I don't know what the best solution is here, it would be nice if the cli would:
- be more explicit about what is allowed in oneOf for code generation to work (I don't think allowing properties with multiple types is practical, and it might makes sense to require every property to be always specified)
- Would support a way to where you can do enum-based related-properties validation
Sidenote:
I agree that the Cookies definition should be moved to the definitions section as a best practice; however, this is not a requirement for a valid schema.
This is not a requirement for json-schema (and I believe is not enforced in the meta-schema), but https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/resource-type-schema.html#schema-properties-properties does say:
Nested properties are not allowed. Instead, define any nested properties in the definitions element, and use a $ref pointer to reference them in the desired property.