fix: strengthen types, simplify logic #154
Conversation
* enable & use optional attrs * strengthen types for log_configuration, repository_credentials, system_controls, container_definition * rm redundant lookups * simplify secret & environment var sorting
| })) | ||
| description = "A list of VolumesFrom maps which contain \"sourceContainer\" (name of the container that has the volumes to mount) and \"readOnly\" (whether the container can write to the volume)" | ||
| default = [] | ||
| default = null |
There was a problem hiding this comment.
the field is not required so there's no reason to specify [] rather than null
https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html#ECS-Type-ContainerDefinition-volumesFrom
| description = "Container mount points. This is a list of maps, where each map should contain `containerPath`, `sourceVolume` and `readOnly`" | ||
| default = [] | ||
| default = null |
There was a problem hiding this comment.
| description = "Log configuration options to send to a custom log driver for the container. For more details, see https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_LogConfiguration.html" | ||
| default = null | ||
| } | ||
|
|
||
| # https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_FirelensConfiguration.html | ||
| variable "firelens_configuration" { | ||
| type = object({ | ||
| options = optional(map(string)) |
There was a problem hiding this comment.
explicitly optional per AWS docs
https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_FirelensConfiguration.html#ECS-Type-FirelensConfiguration-options
| drop = list(string) | ||
| }) | ||
| devices = list(object({ | ||
| capabilities = optional(object({ |
There was a problem hiding this comment.
There was a problem hiding this comment.
Please see discussion in cloudposse/terraform-aws-ecs-web-app#189
|
@obataku can you get back to Nitro's requested changes and deal with the branch conflicts that you have? And if no longer relevant, feel free to close out. Thanks! |
|
/terratest |
|
@obataku I merged your PR into the |
* fix: strengthen types, simplify logic (#154) * fix: strengthen types, simplify logic * enable & use optional attrs * strengthen types for log_configuration, repository_credentials, system_controls, container_definition * rm redundant lookups * simplify secret & environment var sorting * Auto Format * fix: address missing optional; update examples/complete * Auto Format --------- Co-authored-by: cloudpossebot <11232728+cloudpossebot@users.noreply.github.com> Co-authored-by: Igor Rodionov <goruha@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com> Co-authored-by: obataku <19821199+obataku@users.noreply.github.com> Co-authored-by: cloudpossebot <11232728+cloudpossebot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
what
log_configuration,repository_credentials,system_controls,container_definitionjsonencode/jsondecodecyclewhy
optionalobviateslookup(..., null)calls for objectslookup(o, k)without adefaultis deprecated in favor ofo[k]or direct attr access (o.k)log_configuration,repository_credentials, andsystem_controlshad needlessly opaqueanytypes which hinder DX and make subtle bugs more likely (e.g. in the types oflog_configuration.optionsvalues)container_definitionare now typed there's no reason not to type it as welllog_configurationappropriately obviates explicittostring&nullhandling; resolves Log Configuration Options should be options #151for-expressions iterate over maps & objects by key in lexicographic orderjsonencode-ing the final container definition only tojsondecodeforjson_map_objectis redundant