provider block validation using python-hcl2 #71
Conversation
WalkthroughA new validation step was introduced to scan Terraform files for provider blocks, disallowing their presence in modules. Documentation was updated to reflect this policy, a new utility function was added for enforcement, dependencies were updated to include Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Utils
User->>CLI: Run validate-directory command
CLI->>Utils: validate_facets_tf_vars(path)
CLI->>Utils: validate_no_provider_blocks(path)
Utils->>Utils: Scan all *.tf files
Utils->>CLI: Raise error if provider block found
CLI->>CLI: Continue with other validation steps
CLI->>User: Output results
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 (4)
ftf_cli/utils.py (1)
937-957: LGTM! Well-implemented validation function with one minor improvement opportunity.The function correctly implements the provider block validation with proper recursive file discovery, clear error reporting, and appropriate fail-fast behavior for parse errors.
Minor improvement for exception handling:
Consider using proper exception chaining for better error traceability:
except Exception as e: - raise click.UsageError(f"❌ Failed to parse {tf_file}: {e}") + raise click.UsageError(f"❌ Failed to parse {tf_file}: {e}") from etests/test_utils.py (3)
193-194: Remove unused import and improve exception specificity in tests.The
osimport is unused and should be removed. Additionally, the test functions should catch the specificclick.UsageErrorexception thatvalidate_no_provider_blocksraises, rather than genericException.from ftf_cli.utils import validate_no_provider_blocks -import os +import click
552-557: Use specific exception type for more precise testing.The test should catch
click.UsageErrorspecifically since that's whatvalidate_no_provider_blocksraises according to the implementation.def test_validate_no_provider_blocks_detects_provider(tmp_path): tf_file = tmp_path / "main.tf" tf_file.write_text('provider "aws" { region = "us-west-2" }') - with pytest.raises(Exception) as excinfo: + with pytest.raises(click.UsageError) as excinfo: validate_no_provider_blocks(str(tmp_path)) assert "Provider blocks are not allowed" in str(excinfo.value)
567-572: Use specific exception type for parse error testing.Similar to the first test, this should catch
click.UsageErrorspecifically.def test_validate_no_provider_blocks_parse_error(tmp_path): tf_file = tmp_path / "main.tf" tf_file.write_text('this is not valid hcl') - with pytest.raises(Exception) as excinfo: + with pytest.raises(click.UsageError) as excinfo: validate_no_provider_blocks(str(tmp_path)) assert "Failed to parse" in str(excinfo.value)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)TESTING.md(1 hunks)ftf_cli/commands/validate_directory.py(2 hunks)ftf_cli/utils.py(1 hunks)setup.py(2 hunks)tests/test_utils.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
README.md (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.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 (2)
ftf_cli/commands/validate_directory.py (1)
ftf_cli/utils.py (1)
validate_no_provider_blocks(937-957)
tests/test_utils.py (1)
ftf_cli/utils.py (1)
validate_no_provider_blocks(937-957)
🪛 Ruff (0.11.9)
ftf_cli/utils.py
951-951: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
tests/test_utils.py
194-194: os imported but unused
Remove unused import: os
(F401)
🔇 Additional comments (5)
setup.py (1)
29-29: LGTM! Dependency additions are correct.The
python-hcl2dependency is properly added to bothinstall_requiresanddevdependencies, ensuring it's available for the new validation functionality in both production and development environments.Also applies to: 40-40
TESTING.md (1)
97-100: LGTM! Clear documentation of the new validation behavior.The documentation clearly explains the provider block validation policy and references the appropriate test coverage. This will help developers understand the validation requirements and locate relevant tests.
README.md (1)
132-132: LGTM! Documentation accurately reflects the new validation functionality.The updates clearly communicate the provider block validation to users and explain the policy rationale. The documentation is consistent with the implementation and other documentation updates.
Also applies to: 143-143
ftf_cli/commands/validate_directory.py (1)
7-7: LGTM! Clean integration of the new validation step.The import and function call are properly integrated into the existing validation flow. The placement after
validate_facets_tf_varsand before the success message maintains the logical validation sequence.Also applies to: 59-59
tests/test_utils.py (1)
560-564: Test coverage looks good for the success case.This test correctly verifies that Terraform files without provider blocks are allowed to pass validation.
Summary by CodeRabbit
.tffiles and fails with a clear error if provider blocks are detected.facets.yaml.python-hcl2as a required dependency.