Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate $ref correctly #23

Open
tobywf opened this issue Jul 9, 2019 · 0 comments
Open

Validate $ref correctly #23

tobywf opened this issue Jul 9, 2019 · 0 comments
Labels
bug Something isn't working help wanted Extra attention is needed schema Related to the provider meta-schema

Comments

@tobywf
Copy link
Contributor

tobywf commented Jul 9, 2019

Issue

See also aws-cloudformation/cloudformation-cli#66

TL;DR: If we validate the schema as data, and our meta schema allows $ref, it doesn't work, as the "data" i.e. the schema is not further processed:

@Test
public void improperValidation() {
    final JSONObject definition = new JSONObject()
            .put(TYPE_NAME_KEY, EXAMPLE_TYPE_NAME)
            .put(DESCRIPTION_KEY, EXAMPLE_DESCRIPTION)
            .put(PROPERTIES_KEY, new JSONObject()
                .put("propertyA", new JSONObject()
                    .put("$ref", "#/garbage")))
            .put(PRIMARY_IDENTIFIER_KEY, Arrays.asList(EXAMPLE_PRIMARY_IDENTIFIER));

    validator.validateResourceDefinition(definition);
}

this passed, but the $ref is obviously garbage. The reason it "passes" is because we allow $refs:

https://github.com/aws-cloudformation/aws-cloudformation-resource-schema/blob/6a1ef45d612d7c90da1d77e71a9427a64cdd5237/src/main/resources/schema/provider.definition.schema.v1.json#L85-L87

however, all JSON schema mandates is:

"$ref": {
    "type": "string",
    "format": "uri-reference"
},

Solutions?

The obvious solution is to resolve all $refs before validation, and then it'd work. In Python, I'd use jsonref, or disallow circular references and simply inline all references, and disallow $ref.

It may be possible to use org.everit.json.schema.loader.SchemaLoader in some capacity. Instead of validating against draft-7 (the usual meta-schema), it may be possible to use our meta-schema instead. Since SchemaLoader handles $refs semantically, that ought to work. (I had planned to do something like this in the RPDK, where the library/dynamic nature makes it fairly straightforward to do.)

If in future we want to restrict $refs to the local or any hosted schemas, putting in our own code could be beneficial here in the long run.

@tobywf tobywf added bug Something isn't working help wanted Extra attention is needed schema Related to the provider meta-schema labels Jul 9, 2019
vladtsir added a commit to vladtsir/aws-cloudformation-resource-schema that referenced this issue Dec 14, 2019
vladtsir added a commit to vladtsir/aws-cloudformation-resource-schema that referenced this issue Dec 15, 2019
@vladtsir vladtsir mentioned this issue Dec 15, 2019
tobywf pushed a commit to vladtsir/aws-cloudformation-resource-schema that referenced this issue Dec 16, 2019
rjlohan pushed a commit that referenced this issue Jan 6, 2020
* Validate $ref correctly (#23)
* Inject $schema value into resource definitions during validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed schema Related to the provider meta-schema
Projects
None yet
Development

No branches or pull requests

1 participant