Proof of concept: more concise/useful comments

[raxod502-plaid]

This pull request makes available a number of internal changes that we have made to Atlantis at my workplace in order to make Atlantis' comments more useful and readable. I cannot guarantee that I will have the bandwidth to maintain the PR (in particular, it kind of tramples on some existing Atlantis features that we don't use at my workplace), but I thought I would make it available for the community to build upon.

add summary at top of comment

Before:

After:

always use <details> tag for output, when available

Avoids this kind of inconsistency which hinders readability:

color-code more aggressively

Before:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
~ update in-place
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # module.redacted.aws_instance.redacted must be replaced
-/+ resource "aws_instance" "redacted" {
      ~ ami                          = "ami-redacted" -> "ami-redacted" # forces replacement
      ~ arn                          = "arn:aws:ec2:us-east-1:redacted:instance/i-redacted" -> (known after apply)
      ~ associate_public_ip_address  = false -> (known after apply)
        availability_zone            = "us-east-1b"
      ~ cpu_core_count               = 4 -> (known after apply)
      ~ cpu_threads_per_core         = 2 -> (known after apply)
      - disable_api_termination      = false -> null
      - ebs_optimized                = false -> null
        get_password_data            = false
      - hibernation                  = false -> null
      + host_id                      = (known after apply)
        iam_instance_profile         = "remote_redacted_profile"
      ~ id                           = "i-redacted" -> (known after apply)
      ~ instance_state               = "running" -> (known after apply)
        instance_type                = "c5.2xlarge"
      ~ ipv6_address_count           = 0 -> (known after apply)
      ~ ipv6_addresses               = [] -> (known after apply)
        key_name                     = "RedactedRedactedRedacted"
      - monitoring                   = false -> null
      + outpost_arn                  = (known after apply)
      + password_data                = (known after apply)
      + placement_group              = (known after apply)
      ~ primary_network_interface_id = "eni-redacted" -> (known after apply)
      ~ private_dns                  = "ip-redacted.ec2.internal" -> (known after apply)
      ~ private_ip                   = "redacted" -> (known after apply)
      + public_dns                   = (known after apply)
      + public_ip                    = (known after apply)
      ~ secondary_private_ips        = [] -> (known after apply)
      ~ security_groups              = [] -> (known after apply)
        source_dest_check            = true
        subnet_id                    = "subnet-redacted"
        tags                         = {
            "Name" = "redacted-redacted"
        }
      ~ tenancy                      = "default" -> (known after apply)
        user_data                    = "redacted"
      ~ volume_tags                  = {} -> (known after apply)
        vpc_security_group_ids       = [
            "sg-0e6338016e75368ee",
        ]

      + ebs_block_device {
          + delete_on_termination = (known after apply)
          + device_name           = (known after apply)
          + encrypted             = (known after apply)
          + iops                  = (known after apply)
          + kms_key_id            = (known after apply)
          + snapshot_id           = (known after apply)
          + volume_id             = (known after apply)
          + volume_size           = (known after apply)
          + volume_type           = (known after apply)
        }

      + ephemeral_block_device {
          + device_name  = (known after apply)
          + no_device    = (known after apply)
          + virtual_name = (known after apply)
        }

      ~ metadata_options {
          ~ http_endpoint               = "enabled" -> (known after apply)
          ~ http_put_response_hop_limit = 1 -> (known after apply)
          ~ http_tokens                 = "optional" -> (known after apply)
        }

      + network_interface {
          + delete_on_termination = (known after apply)
          + device_index          = (known after apply)
          + network_interface_id  = (known after apply)
        }

      ~ root_block_device {
          ~ delete_on_termination = true -> (known after apply)
          ~ device_name           = "/dev/sda1" -> (known after apply)
          ~ encrypted             = false -> (known after apply)
          ~ iops                  = 600 -> (known after apply)
          + kms_key_id            = (known after apply)
          ~ volume_id             = "vol-redacted" -> (known after apply)
          ~ volume_size           = 200 -> (known after apply)
          ~ volume_type           = "gp2" -> (known after apply)
        }
    }

  # module.redacted.aws_route53_record.redacted_record will be updated in-place
~ resource "aws_route53_record" "redacted_record" {
        fqdn    = "redacted.redacted.redacted.io"
        id      = "redacted_redacted.redacted.redacted.io_A"
        name    = "redacted.redacted.redacted.io"
      ~ records = [
          - "redacted",
        ] -> (known after apply)
        ttl     = 300
        type    = "A"
        zone_id = "redacted"
    }

Plan: 1 to add, 1 to change, 1 to destroy.
Releasing state lock. This may take a few moments...

After:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
! update in-place
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # module.redacted.aws_instance.redacted must be replaced
-/+ resource "aws_instance" "redacted" {
!       ami                          = "ami-redacted" -> "ami-redacted" # forces replacement
!       arn                          = "arn:aws:ec2:us-east-1:redacted:instance/i-redacted" -> (known after apply)
!       associate_public_ip_address  = false -> (known after apply)
        availability_zone            = "us-east-1b"
!       cpu_core_count               = 4 -> (known after apply)
!       cpu_threads_per_core         = 2 -> (known after apply)
-       disable_api_termination      = false -> null
-       ebs_optimized                = false -> null
        get_password_data            = false
-       hibernation                  = false -> null
+       host_id                      = (known after apply)
        iam_instance_profile         = "remote_redacted_profile"
!       id                           = "i-redacted" -> (known after apply)
!       instance_state               = "running" -> (known after apply)
        instance_type                = "c5.2xlarge"
!       ipv6_address_count           = 0 -> (known after apply)
!       ipv6_addresses               = [] -> (known after apply)
        key_name                     = "RedactedRedactedRedacted"
-       monitoring                   = false -> null
+       outpost_arn                  = (known after apply)
+       password_data                = (known after apply)
+       placement_group              = (known after apply)
!       primary_network_interface_id = "eni-redacted" -> (known after apply)
!       private_dns                  = "ip-redacted.ec2.internal" -> (known after apply)
!       private_ip                   = "redacted" -> (known after apply)
+       public_dns                   = (known after apply)
+       public_ip                    = (known after apply)
!       secondary_private_ips        = [] -> (known after apply)
!       security_groups              = [] -> (known after apply)
        source_dest_check            = true
        subnet_id                    = "subnet-redacted"
        tags                         = {
            "Name" = "redacted-redacted"
        }
!       tenancy                      = "default" -> (known after apply)
        user_data                    = "redacted"
!       volume_tags                  = {} -> (known after apply)
        vpc_security_group_ids       = [
            "sg-0e6338016e75368ee",
        ]

+       ebs_block_device {
+           delete_on_termination = (known after apply)
+           device_name           = (known after apply)
+           encrypted             = (known after apply)
+           iops                  = (known after apply)
+           kms_key_id            = (known after apply)
+           snapshot_id           = (known after apply)
+           volume_id             = (known after apply)
+           volume_size           = (known after apply)
+           volume_type           = (known after apply)
        }

+       ephemeral_block_device {
+           device_name  = (known after apply)
+           no_device    = (known after apply)
+           virtual_name = (known after apply)
        }

!       metadata_options {
!           http_endpoint               = "enabled" -> (known after apply)
!           http_put_response_hop_limit = 1 -> (known after apply)
!           http_tokens                 = "optional" -> (known after apply)
        }

+       network_interface {
+           delete_on_termination = (known after apply)
+           device_index          = (known after apply)
+           network_interface_id  = (known after apply)
        }

!       root_block_device {
!           delete_on_termination = true -> (known after apply)
!           device_name           = "/dev/sda1" -> (known after apply)
!           encrypted             = false -> (known after apply)
!           iops                  = 600 -> (known after apply)
+           kms_key_id            = (known after apply)
!           volume_id             = "vol-redacted" -> (known after apply)
!           volume_size           = 200 -> (known after apply)
!           volume_type           = "gp2" -> (known after apply)
        }
    }

  # module.redacted.aws_route53_record.redacted_record will be updated in-place
! resource "aws_route53_record" "redacted_record" {
        fqdn    = "redacted.redacted.redacted.io"
        id      = "redacted_redacted.redacted.redacted.io_A"
        name    = "redacted.redacted.redacted.io"
!       records = [
-           "redacted",
        ] -> (known after apply)
        ttl     = 300
        type    = "A"
        zone_id = "redacted"
    }

Plan: 1 to add, 1 to change, 1 to destroy.
Releasing state lock. This may take a few moments...

summarize nested property updates

For example, from the Terraform plan

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
~ update in-place

Terraform will perform the following actions:

  # module.incoming_events_iam.aws_iam_policy.plaid_app_extra_inline[0] will be updated in-place
~ resource "aws_iam_policy" "redacted" {
        arn    = "arn:aws:iam::redacted:policy/redacted"
        id     = "arn:aws:iam::redacted:policy/redacted"
        name   = "redacted"
        path   = "/"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                  ~ {
                        Action   = "ssm:GetParameters"
                        Effect   = "Allow"
                      ~ Resource = "arn:aws:ssm:us-east-1:redacted:parameter/redacted" -> [
                          + "arn:aws:ssm:us-east-1:redacted:parameter/redacted",
                          + "arn:aws:ssm:us-east-1:redacted:parameter/redacted",
                          + "arn:aws:ssm:us-east-1:redacted:parameter/redacted",
                        ]
                        Sid      = ""
                    },
                ]
                Version   = "2012-10-17"
            }
        )
    }

Plan: 0 to add, 1 to change, 0 to destroy.
Releasing state lock. This may take a few moments...

we would get a summary as such, where the specific properties within the modified objects are enumerated.

Modify 1 (modified properties: add 3)

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
! update in-place

Terraform will perform the following actions:

  # module.incoming_events_iam.aws_iam_policy.plaid_app_extra_inline[0] will be updated in-place
! resource "aws_iam_policy" "redacted" {
        arn    = "arn:aws:iam::redacted:policy/redacted"
        id     = "arn:aws:iam::redacted:policy/redacted"
        name   = "redacted"
        path   = "/"
!       policy = jsonencode(
!           {
!               Statement = [
!                   {
                        Action   = "ssm:GetParameters"
                        Effect   = "Allow"
!                       Resource = "arn:aws:ssm:us-east-1:redacted:parameter/redacted" -> [
+                           "arn:aws:ssm:us-east-1:redacted:parameter/redacted",
+                           "arn:aws:ssm:us-east-1:redacted:parameter/redacted",
+                           "arn:aws:ssm:us-east-1:redacted:parameter/redacted",
                        ]
                        Sid      = ""
                    },
                ]
                Version   = "2012-10-17"
            }
        )
    }

Plan: 0 to add, 1 to change, 0 to destroy.
Releasing state lock. This may take a few moments...

[acastle]

Hello @raxod502-plaid, thank you for the contribution!

I would be interested in getting the community feedback before we proceed with this one as it is a fairly substantial change to the look and feel of the comments from atlantis. A large number of our e2e tests assert against the specific outputs in the current format so we will need to bring them in line with the new format before we can move forward with this PR.

The new format is a big 👍 from me but let's hold off until we can gather some more feedback.

[raxod502-plaid]

Yes, of course. This PR changes things all over the place that would reasonably be controversial and I expected to need a community discussion about the desired output format. And yes, I didn't update any of the tests yet.

[dmattia]

I really like this idea. Every now and then, I end up with a PR that will run a plan on 500ish modules, and scrolling through all of the plans with no changes is less than ideal

[tomasbackman]

I really like the summary and the details parts. The color coding.. not so much but it is acceptable at least.

[tomasbackman]

Feels kind of related: #1267

[raxod502-plaid]

The color coding should probably be optional.

[patrickjahns]

What would be needed to get this into a mergeable state? Would love to see this and happy to help if I somehow can :-)

[kitos9112]

I really like this idea moving forward - it would help us see what's actually changing as part of MR/PR comment(s)

[nishkrishnan]

I'm in favor of the color coding 💯. Others have asked for this as well such as: #1395

W.r.t to the output restructure, i definitely like it better than what's there currently. However, I know we've been talking about making these templates configurable for anyone running Atlantis. Though I'm unsure of when we'll get around to something like that.

Either way in order to get this to a mergeable state we basically just need to fix all the tests and change their fixtures which is quite a bit of work.... if anyone is up for it 😅

[jamengual]

@raxod502-plaid is this relevant still? thanks.

[raxod502-plaid]

Yes, unless #1751 did more than I was aware of.

[jamengual]

can you check @raxod502-plaid ? and if so update the PR description with the differences? ( and code if necessary)

[raxod502-plaid]

Looking at the diff for the linked PR, only color-code more aggressively was implemented, and unless someone else made additional changes, the other features in this PR remain unimplemented.

[jamengual]

ok, good so if you fix the conflicts I will ask for another review.

[raxod502-plaid]

Conflicts fixed.

[jamengual]

@raxod502-plaid could you check the errors?

[raxod502-plaid]

Before I update tests, can somebody confirm that the changes are acceptable? No point in changing tests if changes are requested about the output format, since then the tests will have to be changed again.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.