Parity/sam baseline refresh - #4592
Merged
kddejong merged 3 commits intoJul 24, 2026
Merged
Conversation
The checked-in SAM-translator Python baseline (python_sam_results.json)
was stale: it did not match the output of the pinned Python cfn-lint
1.53.1 that the ratchet's fixture_refs declare. The parity CI job never
runs Python cfn-lint on the SAM templates -- it compares Rust output
against this frozen file -- so the drift went unnoticed and inflated the
"rust-only" count with findings that are actually correct.
Verified by running cfn-lint 1.53.1 per-template over all 546 active SAM
outputs (494 present; 52 listed templates absent from SAM v1.100.0). The
old baseline diverged from real 1.53.1 on exactly 10 rules:
E1159 +13, E1156 +7, W1030 +4, E3031 +3, W1031 +2, W1054 +2,
E1157 +1, E1041 +1, E3510 +1 (baseline UNDER-counted -- Rust was
already correctly emitting these)
W1020 -2 (baseline OVER-counted -- real 1.53.1
does not emit these; Rust correctly
does not either)
Net: the baseline was missing 32 genuine Python findings and claimed 2
phantom ones. After refreshing, the harness reports:
matched 460 -> 493, rust-only 36 -> 3, python-only 8 -> 7,
parity 98.3% -> 98.6%
The remaining 3 rust-only (E3005 x2, W1028 x1) are genuine divergences;
the 7 python-only (E1019 x3, E3660, E3510, E6101, W1032) are genuine Rust
under-reports. Both are addressed in follow-up commits.
Ratchet tightened to match reality (max_rust_only 36->3, min_matched
460->493, min_expected 468->500) via CFN_LINT_UPDATE_RATCHET=1.
scripts/gen_sam_parity_baseline.py makes the baseline reproducible so
future refreshes are an explicit, reviewable diff.
E3005 (DependsOn conditional availability) tested only whether the *target's* condition could be false, in isolation. That flagged a DependsOn as unsafe even when the owner resource can only exist in scenarios where the target also exists. Python cfn-lint instead asks -- via cfn.is_resource_available -- whether there is a scenario where the owner IS present but the target is NOT, constraining both conditions jointly. The distinction matters when conditions share structure. On SAM's usage-plan output (api_with_usageplans_shared_attributes_two) the owner is gated on `Fn::Or [C1, C2]` and depends on resources gated on `C1`/`C2`, where C1 and C2 are the same `Fn::Equals ["test","test"]` (identical hash => one SAT variable). There `owner-true AND C1-false` is `v AND NOT v` -- unsatisfiable -- so the dependency is always present and Python reports nothing; v2 emitted two false positives. Fix: add Context::are_conditions_satisfiable for multi-condition joint queries and rewrite E3005 to emit only when `owner-available AND target-missing` is satisfiable. The same-condition case (owner and target share one condition name) is kept as a fast path -- a single condition cannot be constrained to both true and false in one query. Verified against SAM parity: rust-only drops 3 -> 1 (the remaining W1028 is a case v2 correctly catches and Python misses). _three, where the owner is unconditional, still correctly emits E3005. Regression tests cover both scenarios plus the missing-dependency case.
The E3005 joint-condition fix removed the last two SAM false positives, so the rust-only ceiling drops from 3 to 1 (via CFN_LINT_UPDATE_RATCHET=1). The one remaining rust-only finding (W1028) is a case v2 catches correctly and Python v1 misses, so it is kept, not suppressed. Also documents the residual post-refresh divergences (1 rust-only, 7 python-only) inline in the SAM parity test: which are v2-better, which are genuine under-reports deferred to focused follow-ups (E1019 OpenAPI-body intrinsics, E6101/E1020 Ref-target existence, E3660 property-less resource, W1032 Fn::Join resolution), and which is a Python internal-crash artifact v2 deliberately does not reproduce (E3510).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.