Support casing for environmental keys#18
Conversation
davidblum
commented
Jul 20, 2018
davidblum
commented
- Add new option for upper case or lower case on environmental variables.
- Add test for new option.
orfin
left a comment
orfin
left a comment
There was a problem hiding this comment.
Thanks for the PR, especially for introducing test case :-)
It would be great if you could also add to README.md section of running the tests? (maybe some makefile shortcut like $ make test ?)
I'm wondering, why do we need that "case" parameter? Isn't it making the tool more complex? In my original idea, aws-env was exporting ssm parameters just as they are named into ENVs. So if one need uppercase params - he would just name them in that way in SSM:
/prod/DB/USERNAME ==> DB_USERNAME
aws-env.go
Outdated
| convertCase := flag.String("case", "upper", "Converts ENV Key to upper or lower case") | ||
| flag.Parse() | ||
|
|
||
| if *convertCase == "upper" || *convertCase == "lower" { |
There was a problem hiding this comment.
Can we use consts here instead?
aws-env.go
Outdated
| recursivePtr := flag.Bool("recursive", false, "recursively process parameters on path") | ||
| flag.Parse() | ||
| recursivePtr := flag.Bool("recursive", false, "recursively process parameters on path") | ||
| convertCase := flag.String("case", "upper", "Converts ENV Key to upper or lower case") |
There was a problem hiding this comment.
Can we use const here instead of "upper"?
|
|
||
| if *convertCase == "upper" || *convertCase == "lower" { | ||
| } else { | ||
| log.Fatal("Unsupported case option. Must be 'upper' or 'lower'") |
There was a problem hiding this comment.
Can we use consts here instead of hardcoding upper/lower?
|
Hi @orfin ! Thanks for the response, I've made your suggested changes. In regard to the "why": In our environment we had migrated several styles of secrets, passwords, and credentials to SSM. They were of all variety of case (some were all upper, some were lower, and some were even Pascal). To make this simpler we chose to down case all of them before uploading to the SSM. This made them uniform, but broke some of our base scripting that relied on traditional bash ENV variable style. I felt this was a nice simple change the provided flexibility in accessing the keys, and exporting them as the scripting expected. Thanks for writing a helpful tool! |
orfin
left a comment
orfin
left a comment
There was a problem hiding this comment.
Sorry for late reply, I've found one bug (BC-break) in your code. Can we fix it and then I will be able to merge PR as I tested rest of your code and its working great!
| recursivePtr := flag.Bool("recursive", false, "recursively process parameters on path") | ||
| flag.Parse() | ||
| recursivePtr := flag.Bool("recursive", false, "recursively process parameters on path") | ||
| convertCase := flag.String("case", upper, "Converts ENV Key to upper or lower case") |
There was a problem hiding this comment.
"Case" should not be required and by default it should not change string-case as currently it breaks backward compatibility - it's now by default changing letters case to upper.
2 participants
