fix: FQDN detection when using an existing ALB

legoscia commented 9 months ago

Description

In the definition of the local variable atlantis_url, currently attributes of module.alb are referenced within a coalesce function call. coalesce isn't short-circuiting, so even if var.atlantis.fqdn was provided, it won't be used in the case when an existing ALB is used: the reference to module.alb.route53_records["A"] will fail, and the error will be caught by the top-level try, leaving atlantis_url as https://.

In this change, the individual references to module.alb are wrapped in try calls, meaning that var.atlantis.fqdn will be respected if provided.

Motivation and Context

This change fixes this issue: https://github.com/terraform-aws-modules/terraform-aws-atlantis/issues/377 With this change, links in the "Checks" section of a Github pull request point to the correct URL.

Breaking Changes

This change can force replacement of the ECS task definition in the case where an existing ALB was used and var.atlantis.fqdn was specified (but ignored). If this is not desirable, this can be avoided by removing that variable from the configuration. I believe this is unlikely to happen.

How Has This Been Tested?

[X] I have updated at least one of the examples/* to demonstrate and validate my change(s)
[X] I have tested and validated these changes using one or more of the provided examples/* projects

I added the fqdn variable to github-separate, and deployed it without this change. That resulted in ATLANTIS_ATLANTIS_URL being set to https://. Then I ran terraform apply with this change. That resulted in module.atlantis.module.ecs_service.aws_ecs_task_definition.this[0] being recreated and ATLANTIS_ATLANTIS_URL being set to the URL of the load balancer.
[X] I have executed pre-commit run -a on my pull request

antonbabenko commented 9 months ago

This PR is included in version 4.0.6 :tada:

github-actions[bot] commented 8 months ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

terraform-aws-modules / terraform-aws-atlantis