fix: Adds environment to keys to preserve case
#2446
Conversation
Jiafi
requested review from
ansgarm and
Maed223
and removed request for
a team
|
|
Jiafi
changed the title
environment to keys to preserve caseenvironment to keys to preserve case
| @@ -90,7 +90,7 @@ export function keysToSnakeCase(object: any): any { | |||
| } | |||
| const keys = Object.keys(object); | |||
| return keys.reduce((newObject: any, key: string) => { | |||
| if (key === "tags") { | |||
| if (key === "tags" || key === "environment") { | |||
There was a problem hiding this comment.
Arguably it might be nice to continue applying snake casing to environment variables, since they're typically UPPER_SNAKE_CASE. Although I agree case should be preserved.
Could definitely see the argument, though, that environment variables should simply be left literally as specified (like tags).
There was a problem hiding this comment.
Yeah, I would vote for preserve as is as that matches what is expected and there might sometimes be variables that require lowercase, too. One example I can think off, would be TF_VAR_my_variable env vars.
There was a problem hiding this comment.
Thank you so much for taking a stab at fixing this, @Jiafi 👏 Appreciate it!
I'm a bit torn about this, as the current special case for tags is already something that is probably not used anymore (because bindings generated for Terraform providers don't use keysToSnakeCase anymore since #395, and this was probably added originally for the tags attribute in various AWS resources).
Upon researching the use of the keysToSnakeCase method, I noticed that it probably also wrongly converts keys in assumeRoleTags in the S3Backend and possibly other backends that have attributes with the type { [key: string]: string }.
However, I'm overall still in favor of this particular PR because it is simple and we currently already have a special case for tags in here and there's no other environment in e.g. backends that could be affected by this 😄
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Adds
environmentto keys to preserve case. When usinglocal-exec, the environment would get lower cased. This is unintuitive with how environment variables generally work.