Add iac.validated_files schema validation, tests, and documentation #72
Conversation
WalkthroughA new "iac" block with a "validated_files" array property was added to the YAML schema and documented in the README. The README now instructs users to add new MCP server-generated Terraform files to this list. Unit tests were introduced to validate the structure and error handling for the "iac" block in the YAML configuration. Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
ftf_cli/schema.py (1)
97-106: Schema structure looks good, but consider adding title and description fields.The
iacproperty is well-defined as an object with avalidated_filesarray of strings. TheadditionalProperties: Trueallows for future extensibility, which is good design.However, for consistency with other top-level properties and better documentation, consider adding
titleanddescriptionfields:"iac": { "type": "object", + "title": "Infrastructure as Code Configuration", + "description": "Configuration for IaC-related settings and file validation", "properties": { "validated_files": { "type": "array", + "title": "Validated Files", + "description": "List of Terraform files that have been validated by the MCP server", "items": {"type": "string"} } }, "additionalProperties": True },README.md (1)
413-425: Good documentation, but specify the language for the fenced code block.The documentation clearly explains when and how to use the
iac.validated_filesconfiguration. The example is helpful and shows the expected structure well.Apply this fix to specify the language for the fenced code block:
Any new tf file created by the mcp server needs to be added to `facets.yaml` in this block: -``` +```yaml iac: validated_files: - variables.tf - tekton.tf # ...add any new files here</blockquote></details> <details> <summary>tests/test_utils_validation.py (1)</summary><blockquote> `4-7`: **Remove unused imports to clean up the code.** The imports `os`, `tempfile`, and `yaml` are not used in the test code and should be removed for cleaner code. ```diff import pytest import click from ftf_cli.utils import check_no_array_or_invalid_pattern_in_spec, check_conflicting_ui_properties -import os -import tempfile -import yaml from ftf_cli.utils import validate_yaml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)ftf_cli/schema.py(1 hunks)tests/test_utils_validation.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
ftf_cli/schema.py (1)
Learnt from: vishnukv-facets
PR: Facets-cloud/module-development-cli#0
File: :0-0
Timestamp: 2025-05-26T13:13:07.039Z
Learning: All user-facing properties in spec must have both 'title' and 'description' fields. Properties with 'x-ui-override-disable': true are considered internal and skip this validation, while properties with 'x-ui-overrides-only': true are still user-facing and require validation. This is enforced by the check_properties_have_required_fields() function in ftf_cli/utils.py.
tests/test_utils_validation.py (1)
Learnt from: vishnukv-facets
PR: Facets-cloud/module-development-cli#0
File: :0-0
Timestamp: 2025-05-26T13:13:07.039Z
Learning: All user-facing properties in spec must have both 'title' and 'description' fields. Properties with 'x-ui-override-disable': true are considered internal and skip this validation, while properties with 'x-ui-overrides-only': true are still user-facing and require validation. This is enforced by the check_properties_have_required_fields() function in ftf_cli/utils.py.
🧬 Code Graph Analysis (1)
tests/test_utils_validation.py (1)
ftf_cli/utils.py (1)
validate_yaml(421-449)
🪛 markdownlint-cli2 (0.17.2)
README.md
417-417: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.11.9)
tests/test_utils_validation.py
4-4: os imported but unused
Remove unused import: os
(F401)
5-5: tempfile imported but unused
Remove unused import: tempfile
(F401)
6-6: yaml imported but unused
Remove unused import: yaml
(F401)
🔇 Additional comments (1)
tests/test_utils_validation.py (1)
253-316: Excellent test coverage for the new iac block validation.The test suite comprehensively covers all validation scenarios for the
iacblock:
- ✅ Valid
iacblock with proper structure- ✅ Invalid
iacfield (not an object)- ✅ Invalid
validated_files(not an array)- ✅ Invalid
validated_fileselements (non-strings)The tests use consistent data structures, proper error assertions, and follow good testing practices. The test names are descriptive and the error message assertions ensure proper validation behavior.
README.md
Outdated
| - aws_security_group.sg (with for_each) | ||
| ``` | ||
|
|
||
| ## Note on MCP Server Generated Files |
There was a problem hiding this comment.
We don't need this in the README, you can remove this
This PR introduces schema validation for the iac block in facets.yaml, adds corresponding tests, and updates the documentation with usage instructions.
Summary by CodeRabbit