Skip to content

Conversation

@albertorm95
Copy link
Contributor

@albertorm95 albertorm95 commented Nov 11, 2025

Description

  • Add missing fqdn attribute for variable atlantis

Motivation and Context

  • fqdn was missing in the atlantis variable so it was not being picked up by the:
    • atlantis_url = "https://${try(coalesce(  try(var.atlantis.fqdn, module.alb.route53_records["A"].fqdn, null),  module.alb.dns_name, ), "")}" 

Breaking Changes

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request
  • I have deploy our current atlantis with this branch version and its working as expected
@bryantbiggs
Copy link
Member

Atlantis does not support running multiple copies at the same time, nor doe EFS allow setting values other than whats now hardcoded for the deployment_*_percent so I don't see us changing these values from whats hardcoded (unless someone can show a very valid reason to do so)

Why do you need exec access to the Atlantis container - this is a really large security concern given how much elevated access Atlantis typically has. Again, I don't see moving off the current value of having it disabled - that just seems like a very poor security decision

@albertorm95
Copy link
Contributor Author

Atlantis does not support running multiple copies at the same time, nor doe EFS allow setting values other than whats now hardcoded for the deployment_*_percent so I don't see us changing these values from whats hardcoded (unless someone can show a very valid reason to do so)

Why do you need exec access to the Atlantis container - this is a really large security concern given how much elevated access Atlantis typically has. Again, I don't see moving off the current value of having it disabled - that just seems like a very poor security decision

ok, I'll sync back with the team, I'll update the PR to just fix the fqdn

@albertorm95 albertorm95 changed the title feat: Add missing service attributes fix: Add missing fqdn attribute from atlantis variable Nov 11, 2025
@bryantbiggs
Copy link
Member

Atlantis does not support running multiple copies at the same time, nor doe EFS allow setting values other than whats now hardcoded for the deployment_*_percent so I don't see us changing these values from whats hardcoded (unless someone can show a very valid reason to do so)
Why do you need exec access to the Atlantis container - this is a really large security concern given how much elevated access Atlantis typically has. Again, I don't see moving off the current value of having it disabled - that just seems like a very poor security decision

ok, I'll sync back with the team, I'll update the PR to just fix the fqdn

Thank you! Its not a hard no but more of a firm no unless theres a really good reason to change this. trying to reduce the number of choices/complexity to something that "just works" out of the box for users and does the "right" things

@bryantbiggs bryantbiggs changed the title fix: Add missing fqdn attribute from atlantis variable fix: Add missing fqdn attribute from the atlantis variable Nov 11, 2025
@bryantbiggs bryantbiggs merged commit 192da24 into terraform-aws-modules:master Nov 11, 2025
12 checks passed
antonbabenko pushed a commit that referenced this pull request Nov 11, 2025
## [5.0.1](v5.0.0...v5.0.1) (2025-11-11) ### Bug Fixes * Add missing `fqdn` attribute from the `atlantis` variable ([#427](#427)) ([192da24](192da24))
@antonbabenko
Copy link
Member

This PR is included in version 5.0.1 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants