deis / workflow-cli

The CLI for Deis Workflow
http://deis.com
MIT License
31 stars 43 forks source link

Publisher fails to parse HEALTHCHECK_INITIAL_DELAY if the value contains `\r` #292

Open deis-admin opened 7 years ago

deis-admin commented 7 years ago

From @NickAb on September 3, 2016 2:9

When using

deis config:push -p staging.env -a my-app

if config file being pushed contains Windows style line endings (\r\n), then \r will be treated as part of the value, which will result in error when trying to parse HEALTHCHECK_INITIAL_DELAY from uploaded config:

2016/09/03 01:47:43 strconv.ParseInt: parsing "10\r": invalid syntax

which results in config push failing with

Creating config... Error:
503 SERVICE UNAVAILABLE
detail: aborting, app containers failed to respond to health check

Which does not give any idea what was actual problem (bad delay value).

Changing file line endings to UNIX style fixes the problem, but shouldn't all line endings be ignored? This will require developers using Windows to careful work with configs, and in case of the error it is hard to diagnose. The \r problem is not obvious as when config is pushed the operation will fail saying that containers failed health-check without any details of actual problem (that Publisher was not able to parse value).

Copied from original issue: deis/deis#5091

deis-admin commented 7 years ago

From @bacongobbler on September 3, 2016 15:40

@NickAb since we want to preserve whitespace when someone uses config:set, I think the right way to tackle this would be in the CLI.

If you're up for it, one way we could handle this is by converting all /r/n and /r control characters into /n.

What version are you running?

Cryptophobia commented 6 years ago

This issue was moved to teamhephy/workflow-cli#10