Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fixed tf tests: TestAccRoute53HealthCheck #5241

Merged
merged 2 commits into from Jun 19, 2022
Merged

fixed tf tests: TestAccRoute53HealthCheck #5241

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
366b83b
fixed tf tests: TestAccRoute53HealthCheck
macnev2013 Jun 17, 2022
217d663
minor fix
macnev2013 Jun 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 60 additions & 3 deletions moto/route53/models.py
View file
Open in desktop
Expand Up @@ -64,10 +64,26 @@ def __init__(self, health_check_id, caller_reference, health_check_args):
self.measure_latency = health_check_args.get("measure_latency") or False
self.inverted = health_check_args.get("inverted") or False
self.disabled = health_check_args.get("disabled") or False
self.enable_sni = health_check_args.get("enable_sni") or False
self.children = health_check_args.get("children") or None
self.enable_sni = health_check_args.get("enable_sni") or True
self.caller_reference = caller_reference

def add_children(self, children):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name set_children would be more accurate, as it doesn't add anything to the existing list of children

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Updated

if children and isinstance(children, list):
self.children = children
elif children and isinstance(children, str):
self.children = [children]
else:
self.children = None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this desired behaviour?
When calling update_healthcheck without the children-argument, this removes any existing children. It seems more intuitive to keep the existing children, if the user doesn't explicitly say they want to change/update them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. if calling update_healthcheck without --child-health-checks argument - its not removing any children. Fixed this.

But it will overwrite all children if --child-health-checks argument is present with valid children.

return self.children
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return value is not used anywhere, as far as I can tell, so we can remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Missed it. Thanks for pointing out.


def add_regions(self, regions):
bblommers marked this conversation as resolved.
Show resolved Hide resolved
if regions and isinstance(regions, list):
self.regions = regions
elif regions and isinstance(regions, str):
self.regions = [regions]
else:
self.regions = None
bblommers marked this conversation as resolved.
Show resolved Hide resolved

@property
def physical_resource_id(self):
return self.id
Expand Down Expand Up @@ -135,10 +151,17 @@ def to_xml(self):
{% if health_check.children %}
<ChildHealthChecks>
{% for child in health_check.children %}
<member>{{ child }}</member>
<ChildHealthCheck>{{ child }}</ChildHealthCheck>
{% endfor %}
</ChildHealthChecks>
{% endif %}
{% if health_check.regions %}
<Regions>
{% for region in health_check.regions %}
<Region>{{ region }}</Region>
{% endfor %}
</Regions>
{% endif %}
</HealthCheckConfig>
<HealthCheckVersion>1</HealthCheckVersion>
</HealthCheck>"""
Expand Down Expand Up @@ -617,9 +640,43 @@ def update_hosted_zone_comment(self, id_, comment):
def create_health_check(self, caller_reference, health_check_args):
health_check_id = str(uuid.uuid4())
health_check = HealthCheck(health_check_id, caller_reference, health_check_args)
health_check.add_children(health_check_args.get("children"))
health_check.add_regions(health_check_args.get("regions"))
self.health_checks[health_check_id] = health_check
return health_check

def update_health_check(self, health_check_id, health_check_args):
health_check = self.health_checks.get(health_check_id)
if not health_check:
raise NoSuchHealthCheck()

if health_check_args.get("ip_address"):
health_check.ip_address = health_check_args.get("ip_address")
if health_check_args.get("port"):
health_check.port = health_check_args.get("port")
if health_check_args.get("resource_path"):
health_check.resource_path = health_check_args.get("resource_path")
if health_check_args.get("fqdn"):
health_check.fqdn = health_check_args.get("fqdn")
if health_check_args.get("search_string"):
health_check.search_string = health_check_args.get("search_string")
if health_check_args.get("request_interval"):
health_check.request_interval = health_check_args.get("request_interval")
if health_check_args.get("failure_threshold"):
health_check.failure_threshold = health_check_args.get("failure_threshold")
if health_check_args.get("health_threshold"):
health_check.health_threshold = health_check_args.get("health_threshold")
if health_check_args.get("inverted"):
health_check.inverted = health_check_args.get("inverted")
if health_check_args.get("disabled"):
health_check.disabled = health_check_args.get("disabled")
if health_check_args.get("enable_sni"):
health_check.enable_sni = health_check_args.get("enable_sni")
health_check.add_children(health_check_args.get("children"))
health_check.add_regions(health_check_args.get("regions"))

return health_check

def list_health_checks(self):
return self.health_checks.values()

Expand Down
35 changes: 31 additions & 4 deletions moto/route53/responses.py
View file
Open in desktop
Expand Up @@ -248,7 +248,7 @@ def rrset_response(self, request, full_url, headers):
)
return 200, headers, template

def health_check_response(self, request, full_url, headers):
def health_check_response1(self, request, full_url, headers):
self.setup_class(request, full_url, headers)

parsed_url = urlparse(full_url)
Expand All @@ -273,6 +273,7 @@ def health_check_response(self, request, full_url, headers):
"disabled": config.get("Disabled"),
"enable_sni": config.get("EnableSNI"),
"children": config.get("ChildHealthChecks", {}).get("ChildHealthCheck"),
"regions": config.get("Regions", {}).get("Region"),
}
health_check = route53_backend.create_health_check(
caller_reference, health_check_args
Expand All @@ -293,22 +294,42 @@ def health_check_response(self, request, full_url, headers):
template.render(health_checks=health_checks, xmlns=XMLNS),
)

def get_or_delete_health_check_response(self, request, full_url, headers):
def health_check_response2(self, request, full_url, headers):
self.setup_class(request, full_url, headers)

parsed_url = urlparse(full_url)
method = request.method
health_check_id = parsed_url.path.split("/")[-1]

if method == "GET":
health_check_id = parsed_url.path.split("/")[-1]
health_check = route53_backend.get_health_check(health_check_id)
template = Template(GET_HEALTH_CHECK_RESPONSE)
return 200, headers, template.render(health_check=health_check)
elif method == "DELETE":
health_check_id = parsed_url.path.split("/")[-1]
route53_backend.delete_health_check(health_check_id)
template = Template(DELETE_HEALTH_CHECK_RESPONSE)
return 200, headers, template.render(xmlns=XMLNS)
elif method == "POST":
config = xmltodict.parse(self.body)["UpdateHealthCheckRequest"]
health_check_args = {
"ip_address": config.get("IPAddress"),
"port": config.get("Port"),
"resource_path": config.get("ResourcePath"),
"fqdn": config.get("FullyQualifiedDomainName"),
"search_string": config.get("SearchString"),
"failure_threshold": config.get("FailureThreshold"),
"health_threshold": config.get("HealthThreshold"),
"inverted": config.get("Inverted"),
"disabled": config.get("Disabled"),
"enable_sni": config.get("EnableSNI"),
"children": config.get("ChildHealthChecks", {}).get("ChildHealthCheck"),
"regions": config.get("Regions", {}).get("Region"),
}
health_check = route53_backend.update_health_check(
health_check_id, health_check_args
)
template = Template(UPDATE_HEALTH_CHECK_RESPONSE)
return 200, headers, template.render(health_check=health_check)

def not_implemented_response(self, request, full_url, headers):
self.setup_class(request, full_url, headers)
Expand Down Expand Up @@ -697,6 +718,12 @@ def reusable_delegation_set(self, request, full_url, headers):
{{ health_check.to_xml() }}
</CreateHealthCheckResponse>"""

UPDATE_HEALTH_CHECK_RESPONSE = """<?xml version="1.0" encoding="UTF-8"?>
<UpdateHealthCheckResponse>
{{ health_check.to_xml() }}
</UpdateHealthCheckResponse>
"""

LIST_HEALTH_CHECKS_RESPONSE = """<?xml version="1.0" encoding="UTF-8"?>
<ListHealthChecksResponse xmlns="{{ xmlns }}">
<HealthChecks>
Expand Down
4 changes: 2 additions & 2 deletions moto/route53/urls.py
View file
Open in desktop
Expand Up @@ -22,8 +22,8 @@ def tag_response2(*args, **kwargs):
r"{0}/(?P<api_version>[\d_-]+)/hostedzonesbyname": Route53().list_hosted_zones_by_name_response,
r"{0}/(?P<api_version>[\d_-]+)/hostedzonesbyvpc": Route53().list_hosted_zones_by_vpc_response,
r"{0}/(?P<api_version>[\d_-]+)/hostedzonecount": Route53().get_hosted_zone_count_response,
r"{0}/(?P<api_version>[\d_-]+)/healthcheck$": Route53().health_check_response,
r"{0}/(?P<api_version>[\d_-]+)/healthcheck/(?P<health_check_id>[^/]+)$": Route53().get_or_delete_health_check_response,
r"{0}/(?P<api_version>[\d_-]+)/healthcheck$": Route53().health_check_response1,
r"{0}/(?P<api_version>[\d_-]+)/healthcheck/(?P<health_check_id>[^/]+)$": Route53().health_check_response2,
r"{0}/(?P<api_version>[\d_-]+)/tags/healthcheck/(?P<zone_id>[^/]+)$": tag_response1,
r"{0}/(?P<api_version>[\d_-]+)/tags/hostedzone/(?P<zone_id>[^/]+)$": tag_response2,
r"{0}/(?P<api_version>[\d_-]+)/trafficpolicyinstances/*": Route53().not_implemented_response,
Expand Down
12 changes: 12 additions & 0 deletions tests/terraformtests/terraform-tests.success.txt
View file
Open in desktop
Expand Up @@ -179,6 +179,18 @@ route53:
- TestAccRoute53ZoneDataSource_name
- TestAccRoute53ZoneDataSource_tags
- TestAccRoute53ZoneDataSource_vpc
- TestAccRoute53HealthCheck_basic
- TestAccRoute53HealthCheck_tags
- TestAccRoute53HealthCheck_withSearchString
- TestAccRoute53HealthCheck_withChildHealthChecks
- TestAccRoute53HealthCheck_withHealthCheckRegions
- TestAccRoute53HealthCheck_ip
- TestAccRoute53HealthCheck_ipv6
- TestAccRoute53HealthCheck_cloudWatchAlarmCheck
- TestAccRoute53HealthCheck_withSNI
- TestAccRoute53HealthCheck_disabled
- TestAccRoute53HealthCheck_withRoutingControlARN
- TestAccRoute53HealthCheck_disappears
s3:
- TestAccS3BucketPolicy
- TestAccS3BucketPublicAccessBlock
Expand Down