Skip to content

Commit 43ccef3

Browse files
johnanand707johnanand707
authored
feat: Add rule W3046 for Route53 AliasTarget HostedZoneId references (#4527)
* feat: Add rule W3046 for Route53 AliasTarget HostedZoneId references Warns on a common copy-paste mistake where the AliasTarget HostedZoneId references the hosted zone the record is being created in (typically a Ref to an AWS::Route53::HostedZone resource) instead of the alias target's canonical hosted zone (typically a GetAtt to a known attribute on a Load Balancer or API Gateway DomainName resource). The rule: - Flags Refs to AWS::Route53::HostedZone resources at AliasTarget.HostedZoneId - Flags Fn::GetAtt to wrong attributes on known hosted-zone-providing resources: AWS::ElasticLoadBalancing::LoadBalancer (expects CanonicalHostedZoneNameID), AWS::ElasticLoadBalancingV2::LoadBalancer (CanonicalHostedZoneID), AWS::ApiGateway::DomainName (DistributionHostedZoneId or RegionalHostedZoneId), and AWS::ApiGatewayV2::DomainName (RegionalHostedZoneId) - Leaves static strings, parameter references, ImportValue, and unrelated resources untouched to avoid false positives Closes #642 * test: Cover defensive returns in W3046 rule Adds parametrized cases for malformed intrinsics that hit the previously uncovered defensive returns: - Ref with a non-string value - Fn::GetAtt with non-string parts - Fn::GetAtt as a single string with no attribute portion Brings patch coverage on RecordSetAliasTargetHostedZoneId.py to 100%.
1 parent cc1c48f commit 43ccef3

2 files changed

Lines changed: 273 additions & 0 deletions

File tree

‎src/cfnlint/rules/resources/route53/RecordSetAliasTargetHostedZoneId.py‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
from __future__ import annotations
7+
8+
from collections import deque
9+
from typing import Any, Iterator
10+
11+
from cfnlint.helpers import ensure_list, is_function
12+
from cfnlint.jsonschema import ValidationError, ValidationResult, Validator
13+
from cfnlint.rules.helpers import get_value_from_path
14+
from cfnlint.rules.jsonschema.CfnLintKeyword import CfnLintKeyword
15+
16+
# Known resource attributes that return a HostedZoneId suitable for use as a
17+
# Route53 AliasTarget.HostedZoneId. When a user references a different
18+
# attribute on one of these resources, or uses Ref on an
19+
# AWS::Route53::HostedZone, the value will not be a valid alias target zone.
20+
_VALID_ALIAS_TARGET_ATTRIBUTES: dict[str, set[str]] = {
21+
"AWS::ElasticLoadBalancing::LoadBalancer": {"CanonicalHostedZoneNameID"},
22+
"AWS::ElasticLoadBalancingV2::LoadBalancer": {"CanonicalHostedZoneID"},
23+
"AWS::ApiGateway::DomainName": {
24+
"DistributionHostedZoneId",
25+
"RegionalHostedZoneId",
26+
},
27+
"AWS::ApiGatewayV2::DomainName": {"RegionalHostedZoneId"},
28+
}
29+
30+
31+
class RecordSetAliasTargetHostedZoneId(CfnLintKeyword):
32+
"""Warn on suspicious AliasTarget.HostedZoneId references"""
33+
34+
id = "W3046"
35+
shortdesc = "Validate Route53 AliasTarget HostedZoneId references"
36+
description = (
37+
"An AliasTarget HostedZoneId should reference the canonical hosted zone "
38+
"of the alias target (for example !GetAtt LoadBalancer.CanonicalHostedZoneID), "
39+
"not the hosted zone the record is being created in"
40+
)
41+
source_url = "https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-route53-aliastarget.html"
42+
tags = ["resources", "route53", "record_set"]
43+
44+
def __init__(self) -> None:
45+
super().__init__(
46+
keywords=[
47+
"Resources/AWS::Route53::RecordSet/Properties",
48+
"Resources/AWS::Route53::RecordSetGroup/Properties/RecordSets/*",
49+
],
50+
)
51+
52+
def _check_value(
53+
self, validator: Validator, value: Any
54+
) -> Iterator[ValidationError]:
55+
fn_k, fn_v = is_function(value)
56+
if fn_k is None:
57+
return
58+
59+
if fn_k == "Ref":
60+
if not isinstance(fn_v, str):
61+
return
62+
resource = validator.context.resources.get(fn_v)
63+
if resource is not None and resource.type == "AWS::Route53::HostedZone":
64+
yield ValidationError(
65+
(
66+
f"{{'Ref': {fn_v!r}}} returns the Id of an "
67+
"AWS::Route53::HostedZone, which is the hosted zone the "
68+
"record is created in, not the alias target's canonical "
69+
"hosted zone. Use !GetAtt on the alias target resource "
70+
"instead (for example "
71+
"!GetAtt LoadBalancer.CanonicalHostedZoneID)"
72+
),
73+
rule=self,
74+
)
75+
return
76+
77+
if fn_k == "Fn::GetAtt":
78+
parts = ensure_list(fn_v)
79+
if len(parts) == 1 and isinstance(parts[0], str):
80+
logical_id, _, attribute = parts[0].partition(".")
81+
elif len(parts) >= 2 and all(isinstance(p, str) for p in parts[:2]):
82+
logical_id, attribute = parts[0], parts[1]
83+
else:
84+
return
85+
86+
if not attribute:
87+
return
88+
89+
resource = validator.context.resources.get(logical_id)
90+
if resource is None:
91+
return
92+
93+
valid_attrs = _VALID_ALIAS_TARGET_ATTRIBUTES.get(resource.type)
94+
if valid_attrs is None:
95+
return
96+
97+
if attribute not in valid_attrs:
98+
expected = ", ".join(sorted(valid_attrs))
99+
yield ValidationError(
100+
(
101+
f"{{'Fn::GetAtt': [{logical_id!r}, {attribute!r}]}} "
102+
f"does not return a HostedZoneId for {resource.type!r}. "
103+
f"Expected one of: {expected}"
104+
),
105+
rule=self,
106+
)
107+
108+
def validate(
109+
self, validator: Validator, _, instance: Any, schema: Any
110+
) -> ValidationResult:
111+
for hosted_zone_id, hosted_zone_id_validator in get_value_from_path(
112+
validator, instance, deque(["AliasTarget", "HostedZoneId"])
113+
):
114+
if hosted_zone_id is None:
115+
continue
116+
for err in self._check_value(hosted_zone_id_validator, hosted_zone_id):
117+
yield err

‎test/unit/rules/resources/route53/test_recordset_alias_target_hosted_zone_id.py‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
"""
2+
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
SPDX-License-Identifier: MIT-0
4+
"""
5+
6+
import pytest
7+
8+
from cfnlint.jsonschema import ValidationError
9+
from cfnlint.rules.resources.route53.RecordSetAliasTargetHostedZoneId import (
10+
RecordSetAliasTargetHostedZoneId,
11+
)
12+
13+
14+
@pytest.fixture(scope="module")
15+
def rule():
16+
return RecordSetAliasTargetHostedZoneId()
17+
18+
19+
@pytest.fixture
20+
def template():
21+
return {
22+
"Resources": {
23+
"MyHostedZone": {"Type": "AWS::Route53::HostedZone"},
24+
"MyClassicElb": {"Type": "AWS::ElasticLoadBalancing::LoadBalancer"},
25+
"MyAlb": {"Type": "AWS::ElasticLoadBalancingV2::LoadBalancer"},
26+
"MyApiDomain": {"Type": "AWS::ApiGateway::DomainName"},
27+
"MyHttpApiDomain": {"Type": "AWS::ApiGatewayV2::DomainName"},
28+
"MyBucket": {"Type": "AWS::S3::Bucket"},
29+
},
30+
}
31+
32+
33+
def _props(hosted_zone_id):
34+
return {
35+
"AliasTarget": {
36+
"DNSName": "example.com",
37+
"HostedZoneId": hosted_zone_id,
38+
},
39+
}
40+
41+
42+
@pytest.mark.parametrize(
43+
"name,instance,expect_error",
44+
[
45+
("Static string is not flagged", _props("Z2FDTNDATAQYW2"), False),
46+
(
47+
"Ref to AWS::Route53::HostedZone is flagged",
48+
_props({"Ref": "MyHostedZone"}),
49+
True,
50+
),
51+
(
52+
"Ref to an unrelated resource is not flagged",
53+
_props({"Ref": "MyBucket"}),
54+
False,
55+
),
56+
(
57+
"Ref to a parameter or unknown name is not flagged",
58+
_props({"Ref": "SomeParameter"}),
59+
False,
60+
),
61+
(
62+
"GetAtt classic ELB CanonicalHostedZoneNameID is not flagged",
63+
_props({"Fn::GetAtt": ["MyClassicElb", "CanonicalHostedZoneNameID"]}),
64+
False,
65+
),
66+
(
67+
"GetAtt classic ELB DNSName is flagged (wrong attribute)",
68+
_props({"Fn::GetAtt": ["MyClassicElb", "DNSName"]}),
69+
True,
70+
),
71+
(
72+
"GetAtt ALB CanonicalHostedZoneID is not flagged",
73+
_props({"Fn::GetAtt": ["MyAlb", "CanonicalHostedZoneID"]}),
74+
False,
75+
),
76+
(
77+
"GetAtt ALB DNSName is flagged (wrong attribute)",
78+
_props({"Fn::GetAtt": ["MyAlb", "DNSName"]}),
79+
True,
80+
),
81+
(
82+
"GetAtt API Gateway DistributionHostedZoneId is not flagged",
83+
_props({"Fn::GetAtt": ["MyApiDomain", "DistributionHostedZoneId"]}),
84+
False,
85+
),
86+
(
87+
"GetAtt API Gateway RegionalHostedZoneId is not flagged",
88+
_props({"Fn::GetAtt": ["MyApiDomain", "RegionalHostedZoneId"]}),
89+
False,
90+
),
91+
(
92+
"GetAtt API Gateway DistributionDomainName is flagged",
93+
_props({"Fn::GetAtt": ["MyApiDomain", "DistributionDomainName"]}),
94+
True,
95+
),
96+
(
97+
"GetAtt HTTP API RegionalHostedZoneId is not flagged",
98+
_props({"Fn::GetAtt": ["MyHttpApiDomain", "RegionalHostedZoneId"]}),
99+
False,
100+
),
101+
(
102+
"GetAtt to an unrelated resource is not flagged",
103+
_props({"Fn::GetAtt": ["MyBucket", "DomainName"]}),
104+
False,
105+
),
106+
(
107+
"GetAtt to an unknown resource is not flagged",
108+
_props({"Fn::GetAtt": ["Missing", "CanonicalHostedZoneID"]}),
109+
False,
110+
),
111+
(
112+
"GetAtt with dotted string form is supported",
113+
_props({"Fn::GetAtt": "MyAlb.CanonicalHostedZoneID"}),
114+
False,
115+
),
116+
(
117+
"GetAtt with dotted string form catches wrong attribute",
118+
_props({"Fn::GetAtt": "MyAlb.DNSName"}),
119+
True,
120+
),
121+
(
122+
"Fn::ImportValue is not flagged",
123+
_props({"Fn::ImportValue": "SomeExport"}),
124+
False,
125+
),
126+
(
127+
"Properties without AliasTarget is not flagged",
128+
{"HostedZoneId": {"Ref": "MyHostedZone"}, "Name": "foo", "Type": "A"},
129+
False,
130+
),
131+
(
132+
"Ref with non-string value is not flagged",
133+
_props({"Ref": ["MyHostedZone"]}),
134+
False,
135+
),
136+
(
137+
"GetAtt with malformed parts is not flagged",
138+
_props({"Fn::GetAtt": [123, 456]}),
139+
False,
140+
),
141+
(
142+
"GetAtt single string without dot is not flagged",
143+
_props({"Fn::GetAtt": "MyAlb"}),
144+
False,
145+
),
146+
],
147+
)
148+
def test_validate(name, instance, expect_error, rule, validator):
149+
errs = list(rule.validate(validator, "", instance, {}))
150+
151+
if expect_error:
152+
assert len(errs) == 1, f"Test {name!r}: expected 1 error, got {errs!r}"
153+
assert isinstance(errs[0], ValidationError)
154+
assert errs[0].rule == rule
155+
else:
156+
assert errs == [], f"Test {name!r}: expected no errors, got {errs!r}"

0 commit comments

Comments
 (0)