Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: strengthen types, simplify logic (#154) #164

Merged
merged 28 commits into from
Jun 9, 2023
Merged

fix: strengthen types, simplify logic (#154) #164

merged 28 commits into from
Jun 9, 2023

Conversation

goruha
Copy link
Member

@goruha goruha commented Jun 7, 2023

what

  • enable & use optional attributes where AWS API marks fields optional
  • remove redundant lookups
  • strengthen types for log_configuration, repository_credentials, system_controls, container_definition
  • reorder object var attributes to match AWS docs
  • simplify log configuration sanitization/munging
  • simplify secret & environment var sorting
  • expose unencoded container definition output directly without requiring pointless jsonencode/jsondecode cycle

why

  • optional obviates lookup(..., null) calls for objects
  • lookup(o, k) without a default is deprecated in favor of o[k] or direct attr access (o.k)
  • log_configuration, repository_credentials, and system_controls had needlessly opaque any types which hinder DX and make subtle bugs more likely (e.g. in the types of log_configuration.options values)
  • since all attributes of container_definition are now typed there's no reason not to type it as well
  • following the AWS documentation's field order makes types easier to verify for completeness & correctness
  • typing log_configuration appropriately obviates explicit tostring & null handling; resolves Log Configuration Options should be options #151
  • env/secret map sorting can be dramatically simplified since for-expressions iterate over maps & objects by key in lexicographic order
  • jsonencode-ing the final container definition only to jsondecode for json_map_object is redundant

Refs

* 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>
@goruha goruha requested review from a team as code owners June 7, 2023 11:49
main.tf Outdated Show resolved Hide resolved
goruha and others added 4 commits June 7, 2023 15:04
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
examples/complete/variables.tf Outdated Show resolved Hide resolved
@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

examples/complete/main.tf Outdated Show resolved Hide resolved
@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
@goruha
Copy link
Member Author

goruha commented Jun 7, 2023

/terratest

@goruha
Copy link
Member Author

goruha commented Jun 8, 2023

/terratest

Benbentwo
Benbentwo previously approved these changes Jun 8, 2023
versions.tf Outdated Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comments

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
@goruha
Copy link
Member Author

goruha commented Jun 9, 2023

/terratest

@goruha goruha merged commit 9e0307e into main Jun 9, 2023
@goruha goruha deleted the strict-types branch June 9, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log Configuration Options should be options
4 participants