Reduce diff spam and unset defaults

[dekimsey]

Currently when the terraform plan is generated, there is a great deal of spurious noise from default values being re-set to null. This change alters how the JSON is being generated such that null entries are dropped entirely from the resultant task specification. It's not perfect, as there are some complex entries that have keys set as a result of the type system (logConfiguration for instance). But it does significantly reduce the noise in the delta.

Additionally, this drops all default values that don't correspond.

Before a plan might look like:

                { 
                  ....
                    portMappings           = []
                  + privileged             = null
                    readonlyRootFilesystem = false
                  + repositoryCredentials  = null
                  + secrets                = null
                    startTimeout           = 120
                    stopTimeout            = 30
                  + systemControls         = null
                  + ulimits                = null
                  + user                   = null
                  ~ volumesFrom            = [] -> null
                  + workingDirectory       = null
                } # forces replacement,

After:

              ~ {
                  - cpu                    = 0 -> null
                  ~ environment            = [
                      ~ {
                            name  = "CT_LOCAL_CONFIG"
                          ~ value = <<~EOT
                                log_level = "debug"

                              - telemetry {
                              -   prometheus {
                              -     reporting_interval = "60s"
                              -     port = 8888
                              -   }
                              - }
                              -
                            EOT
                        },
                    ]
                  ~ logConfiguration       = {
                        logDriver     = "awslogs"
                        options       = {
                            awslogs-group         = "stg"
                            awslogs-region        = "us-east-2"
                            awslogs-stream-prefix = "app"
                        }
                      + secretOptions = null
                    }
                    name                   = "config"
                    portMappings           = []
                    readonlyRootFilesystem = false
                    startTimeout           = 30
                    stopTimeout            = 30
                    user                   = "root"
                    volumesFrom            = []
                } # forces replacement,

Which is significantly reduced change noise. There are a few places it's still happening due to the sub-dictionary keys. I'm not sure if I want to go bother trying to address those.

[jtdoepke]

Couldn't this be done with a for-loop?

container_definition_without_null = {
  for k, v in container_definition :
  k => v
  if v != null
}

[dekimsey]

Maybe, I hadn't considered it. I'll give it a try though. I like the idea.

The sub-maps are still somewhat problematic, but I didn't have any good ideas on how to address those.

[aknysh]

@dekimsey please rebase

[dekimsey]

@jtdoepke Your recommendation is on point, much saner change.

And at least for Fargate tasks, due the key validation we are doing in the module, there's still some diff noise (see below) It's possible when the new validation rules become mainstream, this could be tweaked again to improve output.

I wanted to play with it some more, but I feared it might behave differently on Fargate vs ECS task deltas and I don't have any good tests for that.

So right now, we still see some delta spam:

# Only hostPort is not valid to be set, but validation requires we set one (setting to explict null results in this diff)
                  ~ portMappings           = [
                      ~ {
                            containerPort = 8301
                          ~ hostPort      = 8301 -> null
                            protocol      = "tcp"
                        },
                    ]
# Only initProcessEnabled is valid, but the validation requires all keys.
                  ~ linuxParameters        = {
                      + capabilities       = null
                      + devices            = null
                        initProcessEnabled = true
                      + maxSwap            = null
                      + sharedMemorySize   = null
                      + swappiness         = null
                      + tmpfs              = null
                    }
# defaults when unset appears to be empty lists instead
                  - mountPoints            = [] -> null
                  - environment            = [] -> null

[Gowiem]

/test all

[Gowiem]

/rebuild-readme

[Gowiem]

/test all

[nitrocode]

/test all

[nitrocode]

/rebuild-readme

[nitrocode]

/test all

[nitrocode]

/rebuild-readme

[nitrocode]

/test all

[nitrocode]

lgtm