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

Don't load config from stdin unless asked #2859

Closed
hdgarrood opened this issue Jun 18, 2020 · 4 comments
Closed

Don't load config from stdin unless asked #2859

hdgarrood opened this issue Jun 18, 2020 · 4 comments

Comments

@hdgarrood
Copy link

Which version and edition of Flyway are you using?

6.4.4 Community edition

If this is not the latest version, can you reproduce the issue with the latest one as well?

(Many bugs are fixed in newer releases and upgrading will often resolve the issue)

n/a

Which client are you using? (Command-line, Java API, Maven plugin, Gradle plugin)

Command line

Which database are you using (type & version)?

PostgreSQL v10

Which operating system are you using?

Ubuntu Linux

What did you do?

(Please include the content causing the issue, any relevant configuration settings, the SQL statement that failed (if relevant) and the command you ran.)

Attempting to run flyway migrate in certain environments where System.in.available() returns something greater than 0 causes Flyway to hang. In particular I've observed this with an ssh session using -t -t and a heredoc (see below).

What did you expect to see?

I expected flyway migrate to complete normally, and in particular I did not expect Flyway to attempt to read config from stdin unless it was specifically asked to do so via a command line argument. I also think Flyway should print a prompt if it is waiting for configuration input from stdin, because if you weren't expecting it to try to read from stdin it's quite surprising, and it looks like a bug causing Flyway to hang.

What did you see instead?
$ ssh -t -t ubuntu@[host redacted] << END
set -e
cd /path/to/database/migrations
flyway migrate -X
exit 0
END

Welcome to Ubuntu 18.04.3 LTS (GNU/Linux 4.15.0-96-generic x86_64)

 * Documentation:  https://help.ubuntu.com
 * Management:     https://landscape.canonical.com
 * Support:        https://ubuntu.com/advantage

  System information as of Thu Jun 18 13:08:34 UTC 2020

  System load:  0.07               Processes:           97
  Usage of /:   12.2% of 48.29GB   Users logged in:     0
  Memory usage: 14%                IP address for eth0: 198.199.113.206
  Swap usage:   0%

 * Canonical Livepatch is available for installation.
   - Reduce system reboots and improve kernel security. Activate at:
     https://ubuntu.com/livepatch

70 packages can be updated.
0 updates are security updates.


*** System restart required ***
Last login: Thu Jun 18 13:07:28 2020 from 82.43.170.174
ubuntu@[host redacted]:~$ set -e
ubuntu@[host redacted]:~$ cd /path/to/database/migrations
ubuntu@[host redacted]:/path/to/database/migrations$ flyway migrate -X
DEBUG: Loading config file: /opt/flyway-6.4.4/conf/flyway.conf
DEBUG: Unable to load config file: /home/ubuntu/flyway.conf
DEBUG: Loading config file: /path/to/database/migrations/flyway.conf
DEBUG: Attempting to load configuration from standard input

At this point flyway hangs and produces no more input. From the code it looks like Flyway tries to read from stdin unconditionally:

config.putAll(readConfigFromInputStream(System.in));

I was able to work around this by writing flyway migrate </dev/null.

@nyctef
Copy link

nyctef commented Jun 24, 2020

A common unix convention is to read from stdin when - is provided in place of a filename - maybe the -configFiles option could follow this convention? It potentially makes flyway more flexible since you can then specify the ordering between stdin and other config files

@Lyeeedar Lyeeedar modified the milestones: Flyway 6.5, Flyway 7.x Jun 26, 2020
@Lyeeedar
Copy link
Contributor

As this will require breaking current users of the system, we will be doing this change as part of the breaking changes in v7

@DoodleBobBuffPants
Copy link
Contributor

This feature is now available in the V7 beta which you can read more about here.

@hdgarrood
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants