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

Update schema diff to match schema format for customer set values #74

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

pultormi
Copy link
Contributor

@pultormi pultormi commented Oct 25, 2024

Issue #, if available:

SGR evaluates backward compatibility on readOnlyProperties - only new properties can be added to readOnly
SGR incorrectly fails on new properties being added to readOnly.

Example failure:

#Old schema
{
    "definitions": {
        "Workgroup": {
            "additionalProperties": false,
            "properties": {},
            "type": "object"
        }
    },
    "properties": {
        "Workgroup": {
            "$ref": "#/definitions/Workgroup"
        }
    },
    "readOnlyProperties": [
        "/properties/Workgroup"
    ]
}

# New schema
{
    "definitions": {
        "Workgroup": {
            "additionalProperties": false,
            "properties": {
                "MaxCapacity": {
                    "type": "integer"
                }
            },
            "type": "object"
        }
    },
    "properties": {
        "Workgroup": {
            "$ref": "#/definitions/Workgroup",
            "description": "Definition for workgroup resource"
        }
    },
    "readOnlyProperties": [
        "/properties/Workgroup",
        "/properties/Workgroup/MaxCapacity"
    ]
}

#Generated diff
{
    'description': {
        'added': ['/properties/Workgroup']
    },
    'properties': {
        'added': ['/properties/Workgroup/properties/MaxCapacity']
    },
    'readOnlyProperties': {
        'added': ['/properties/Workgroup/MaxCapacity']
    }
}

SGR rules are not be able to handle the mapping from /properties/Workgroup/MaxCapacity to /properties/Workgroup/properties/MaxCapacity. This causes schema check failure when changes are allowed.

The above schema will cause this rule to fail:

let newProps = properties.added
rule ensure_old_property_not_removed_from_readonly when readOnlyProperties exists
{
    when properties.added exists {
        readOnlyProperties.added[*] {
            this IN %newProps
            <<
            {
                "result": "NON_COMPLIANT",
                "check_id": "PR004",
                "message": "Only NEWLY ADDED properties can be marked as readOnlyProperties"
            }
            >>
        }
    }

. . . 

Description of changes:

Change is only one line:

value = value.replace("]['properties'][", "][")

This is placed within the _cast_path method which is called on every path found within the DeepDiff to process them from form:
root['properties']['Workgroup']['properties']['MaxCapacity'] -> properties/Workgroup/properties/MaxCapacity

By replacing all occurrences of ]['properties'][ with ][we shrink the input and remove all nested properties within the path returned by the 'DeepDiff' operation.

Unit tests updated to correct expected path output for schema evaluation.

Testing done locally though integration / unit tests.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@spchit spchit merged commit 3b4502d into aws-cloudformation:main Oct 31, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants