Skip to content

Conversation

@ammokhov
Copy link
Contributor

@ammokhov ammokhov commented Oct 30, 2025

Issue #, if available:

Description of changes:

Add Tagging Backward Compatibility Guard Rules

Summary

This PR implements stateful guard rules to prevent breaking changes to tagging metadata in CloudFormation resource provider schemas. The implementation ensures that once a resource is marked as taggable, that capability cannot be removed or degraded in future schema versions.

Changes

Core Implementation

  • Added cfn_object_constructs: New construct type for tracking top-level objects with nested properties (like tagging)
  • Flat diff structure: Implemented flat diff format for tagging properties:
    • removed: ['tagging': {'removed': ['/tagging/taggable', '/tagging/tagProperty']}]
    • changed: ['tagging': {'changed': [{'property': '/tagging/taggable', 'old_value': true, 'new_value': false}]}]

Guard Rules (3 rules)

  • TAG100: ensure_tagging_properties_not_removed - Prevents removal of any tagging properties
  • TAG101: ensure_taggable_not_changed - Prevents changing taggable from true to false
  • TAG102: ensure_taggable_not_changed - Prevents changing tagOnCreate from true to false
  • TAG103: ensure_taggable_not_changed - Prevents changing cloudFormationSystemTags from true to false
  • TAG104: ensure_taggable_not_changed - Prevents changing tagProperty

Testing

  • Updated 22 unit tests in test_stateful.py to validate new diff structure
  • Updated 6 integration tests in test_integ_runner.py with correct expectations
  • All tests passing ✅

Files Modified

  • src/rpdk/guard_rail/core/stateful.py - Added tagging diff generation logic
  • src/rpdk/guard_rail/rule_library/stateful/schema-stateful-cfn-enforced-checks.guard - Added 3 guard rules
  • tests/unit/core/test_stateful.py - Updated test expectations for flat diff structure
  • tests/integ/runner/test_integ_runner.py - Updated integration test expectations

Design Document

Overview

This design implements backward compatibility guard rules for tagging metadata in CloudFormation resource provider schemas. The rules validate changes between schema versions to ensure tagging capabilities are not removed or degraded.

Architecture

Component Location

The guard rules are placed in the existing stateful rule library:

src/rpdk/guard_rail/rule_library/stateful/schema-stateful-cfn-enforced-checks.guard

Integration Points

  1. Rule Discovery: Automatically discovered by prepare_ruleset() when mode is "stateful"
  2. Schema Diff: Operates on schema difference generated by schema_diff() in stateful.py
  3. Execution: Executed through __exec_rules__() closure in runner.py

Data Models

Input Schema Difference Structure

{
  "tagging": {
    "removed": ["/tagging/taggable", "/tagging/tagProperty"],
    "changed": [
      {
        "property": "/tagging/taggable",
        "old_value": true,
        "new_value": false
      }
    ]
  }
}

Output Structure

{
  "result": "NON_COMPLIANT",
  "check_id": "TAG100",
  "message": "tagging properties MUST NOT be removed from schema"
}

Edge Cases (Allowed)

  • Adding tagging to non-taggable resource: ✅ COMPLIANT
  • Changing taggable from false to true: ✅ COMPLIANT
  • Adding new tagging properties: ✅ COMPLIANT

Implementation Tasks

✅ Completed Tasks

  • 1. Investigate and update stateful.py to track tagging metadata changes

    • Analyzed how stateful.py handles top-level schema properties
    • Added cfn_object_constructs for tracking objects with nested properties
    • Implemented flat diff structure for tagging properties
    • Verified nested tagging properties are captured in diffs
  • 2. Add backward compatibility guard rules

    • 2.1 Implement rule to detect tagging section removal (TAG100)
    • 2.2 Implement rule to detect taggable changed to false (TAG101)
    • 2.3 Implement rule to detect tagging properties changed (TAG102)
  • 3. Testing

    • 3.1 Add unit tests for stateful diff generation
    • 3.2 Add integration tests for guard rule execution
    • 3.3 Verify rule integration with guard rail system
  • 4. Cleanup

    • 4.1 Remove temporary test files from root directory
    • 4.2 Remove unused JSON test data files

Test Results

Unit Tests: 22/22 passing ✅
Integration Tests: 6/6 passing ✅


Testing Instructions

To test this PR locally:

# Activate virtual environment
source ../sgr-env/bin/activate

# Run unit tests
python3 -m pytest tests/unit/core/test_stateful.py -v

# Run integration tests
python3 -m pytest tests/integ/runner/test_integ_runner.py::test_exec_compliance_stateful_tagging_backward_compatibility -v

# Test with CLI
guard-rail --schema schema-v1.json --schema schema-v2.json --stateful

Breaking Changes

None. This PR only adds new guard rules and does not modify existing behavior.

Related Issues

Implements backward compatibility checks for tagging metadata as per CloudFormation resource provider requirements.

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

Implement stateful guard rules to prevent breaking changes to tagging metadata in CloudFormation resource schemas.

Changes:
- Add cfn_object_constructs for tracking top-level objects with nested properties
- Implement flat diff structure for tagging properties (removed/changed)
- Add 3 guard rules: TAG100 (properties removed), TAG101 (taggable changed to false), TAG102 (properties changed)
- Update unit and integration tests to validate new diff structure
- Clean up temporary test files and unused JSON test data

All tests passing: 22 unit tests, 6 integration tests
@ammokhov ammokhov self-assigned this Oct 30, 2025
@ammokhov ammokhov added enhancement New feature or request new rule labels Oct 30, 2025

def _is_tagging_property(path_list):
"""This method checks if path is a tagging nested property"""
return len(path_list) == 2 and path_list[0] == "tagging"
Copy link
Contributor

@agarikana agarikana Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we put 2 inside a var like TAGGING_NEST_DEPTH or something and use that var everywhere?


def _get_path(path_list):
"""This method converts array into schema path notation"""
return "/".join([""] + path_list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is [""] required or is it AI generated 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request new rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants