cli: prevent empty environment variable keys #3112
Conversation
slurms
force-pushed
the
validate_env_vars_before_release
branch
from
8a4a8ba
to
2b06c63
Compare
| @@ -63,6 +63,15 @@ func (r *ReleaseRepo) Add(data interface{}) error { | |||
| release.ArtifactIDs = []string{release.LegacyArtifactID} | |||
| } | |||
|
|
|||
| for key := range release.Env { | |||
| if key == "" { | |||
There was a problem hiding this comment.
Since release.Env is a map, you can query for the key directly:
if _, ok := release.Env[""]; ok {
// empty key exists
return ...
}There was a problem hiding this comment.
Nice, I'll do that instead.
slurms
force-pushed
the
validate_env_vars_before_release
branch
from
2b06c63
to
c2e75b6
Compare
|
Not sure what those test failures are caused by :/ |
|
@slurms those failures are unrelated, your changes aren't to blame. |
| if _, ok := release.Env[""]; ok { | ||
| return ct.ValidationError{ | ||
| Field: "env", | ||
| Message: "you can't create an env var with an empty key", |
There was a problem hiding this comment.
Given this is usually done by mistake, it would be useful to include the value in the response, for example:
$ flynn env set =BAR
validation_error: env you can't create an env var with an empty key (tried to set ""="BAR")
There was a problem hiding this comment.
Ah, yep -- seems like a more helpful message 👍
slurms
force-pushed
the
validate_env_vars_before_release
branch
from
c2e75b6
to
507d883
Compare
| if _, ok := release.Env[""]; ok { | ||
| return ct.ValidationError{ | ||
| Field: "env", | ||
| Message: fmt.Sprintf("you can't create an env var with an empty key (tried to set \"\"=\"%s\")", release.env[""]), |
There was a problem hiding this comment.
Update the if statement so that it assigns the value to a variable and then use it here (rather than using release.Env[""] again):
if value, ok := release.Env[""]; ok {
// ... can reference `value` here
}You can also use %q rather than \"%s\" to double-quote the variable in the output string.
There was a problem hiding this comment.
Haha, sorry about the small issues (and thanks for reviewing)
Signed-off-by: Nick Sandford <nick@sandford.id.au>
slurms
force-pushed
the
validate_env_vars_before_release
branch
from
507d883
to
d3c60a4
Compare
|
LGTM |
|
Thanks! |
Fixes #3029. Not sure if this is at all correct... first attempt at go :).