feat: Add rule W4002 for parameters missing from Metadata Interface#4526
Conversation
1092c6b to
0c5ccb4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4526 +/- ##
=======================================
Coverage 94.42% 94.43%
=======================================
Files 430 431 +1
Lines 15099 15123 +24
Branches 2924 2931 +7
=======================================
+ Hits 14257 14281 +24
Misses 461 461
Partials 381 381
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
32e344d to
52b1635
Compare
Implements the inverse of W4001. While W4001 warns when a parameter referenced in Metadata AWS::CloudFormation::Interface does not exist in the Parameters section, W4002 warns when a declared Parameter is not listed in any ParameterGroup, so it would not appear in the CloudFormation Console UI. Closes aws-cloudformation#4115
Adds parametrized cases for malformed Metadata Interface structures: - ParameterGroups with non-dict entries - ParameterGroups where Parameters contains non-string entries Brings patch coverage on InterfaceParameterInGroup.py to 100%.
52b1635 to
420b051
Compare
kddejong
left a comment
kddejong
left a comment
There was a problem hiding this comment.
Thanks for the contribution! A few items to address:
1. False positives when ParameterGroups is absent (src/cfnlint/rules/metadata/InterfaceParameterInGroup.py)
The rule fires when AWS::CloudFormation::Interface exists but has no ParameterGroups key. This will produce false positives for templates that only use ParameterLabels — a valid pattern where the user hasn't opted into grouping. If there are no ParameterGroups defined, CloudFormation shows all parameters in a default list anyway, so there's nothing to warn about.
Suggest early-returning when ParameterGroups is absent or empty:
parameter_groups = instance.get("ParameterGroups", [])
if not parameter_groups:
return2. Error path should include ParameterGroups (src/cfnlint/rules/metadata/InterfaceParameterInGroup.py)
When ParameterGroups is specified but missing parameters, the error path should include ParameterGroups — that's where the user needs to add the missing entries.
3. Test structure (test/unit/rules/metadata/test_interface_parameter_in_group.py)
The project convention for rule unit tests is to assert against full ValidationError objects (errs == expected), not just counts. See the sibling tests (test_interface_configuration.py, test_interface_parameter_exists.py, test_config.py) — they all construct explicit expected error lists with message, rule, path, etc. This catches regressions in error messages, paths, and rule attribution that a length check would miss.
Additionally, please add a test case for when the template has no parameters defined (empty Parameters section).
| return | ||
|
|
||
| grouped: set[str] = set() | ||
| for group in instance.get("ParameterGroups", []): |
There was a problem hiding this comment.
The rule fires when AWS::CloudFormation::Interface exists but has no ParameterGroups key. This will produce false positives for templates that only use ParameterLabels. If there are no ParameterGroups defined, CloudFormation shows all parameters in a default list anyway, so there is nothing to warn about.
Suggest early-returning when ParameterGroups is absent or empty:
parameter_groups = instance.get("ParameterGroups", [])
if not parameter_groups:
return
| f"Parameter {param_name!r} is not listed in any " | ||
| "Metadata AWS::CloudFormation::Interface ParameterGroup", | ||
| rule=self, | ||
| ) |
There was a problem hiding this comment.
When ParameterGroups is specified but missing parameters, the error path should include ParameterGroups — that is where the user needs to add the missing entries.
| ) | ||
| for err in errs: | ||
| assert isinstance(err, ValidationError) | ||
| assert err.rule == rule |
There was a problem hiding this comment.
The project convention for rule unit tests is to assert against full ValidationError objects (errs == expected), not just counts. See the sibling tests (test_interface_configuration.py, test_interface_parameter_exists.py, test_config.py) — they all construct explicit expected error lists with message, rule, path, etc. This catches regressions in error messages, paths, and rule attribution that a length check would miss.
Additionally, please add a test case for when the template has no parameters defined (empty Parameters section).
2 participants
Issue #, if available: Closes #4115
Description of changes:
Adds a new warning rule W4002 that implements the inverse check of W4001.
Metadata.AWS::CloudFormation::Interfacedoes not exist in theParameterssection.Parameteris not listed in anyParameterGroup, so it would not appear in the CloudFormation Console UI.Changes:
src/cfnlint/rules/metadata/InterfaceParameterInGroup.py— new rule (49 lines)test/unit/rules/metadata/test_interface_parameter_in_group.py— 5 parametrized unit tests covering the all-grouped, partial, no-groups, multi-group, and non-dict casestest/fixtures/results/integration/andtest/fixtures/results/quickstart/(pure additions of new W4002 entries; no existing entries modified)test/unit/module/cfn_json/test_cfn_json.py— updated expected failure count forvpc-management.jsonfixture (11 → 27)Verification:
pre-commit runpasses (ruff, isort, bandit, mypy, file hygiene)pytest test/unit/— 2568 passedpytest test/integration/— 27 passedParameterGroupsBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.