Prevent stolonctl init with empty file #22
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes what is debatably a bug, where supplying the file flag to `stolonctl init` with a value that points to an empty file results in initialising the cluster with an `initMode` of `new`. This behaviour is dangerous because if the user intended to supply a file that contained a cluster specification with an `initMode` of `existing`, but mistakenly supplied a path to an empty file, then it would result in the keeper data being removed. To resolve this, change the behaviour of the command to always attempt the unmarshalling of the supplied cluster spec JSON, which will result in a fatal error in the case of an empty file. If the user intends to initialise the cluster with a blank (`initMode: new`) cluster specification then they should do so by omitting the file flag and cluster specification argument, as is already possible.
|
@lawrencejones Thoughts on this? Do you think it's upstream-able? Technically it's a change of interface, even if you'd really hope nobody is relying on this behaviour. |
|
Yeah I think this is absolutely upstreamable. It's a usability issue- can you make this PR to upstream instead? |
|
Will do! Edit: See here |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes what is debatably a bug, where supplying the file flag to
stolonctl initwith a value that points to an empty file results ininitialising the cluster with an
initModeofnew.This behaviour is dangerous because if the user intended to supply a
file that contained a cluster specification with an
initModeofexisting, but mistakenly supplied a path to an empty file, then itwould result in the keeper data being removed.
To resolve this, change the behaviour of the command to always attempt
the unmarshalling of the supplied cluster spec JSON, which will result
in a fatal error in the case of an empty file.
If the user intends to initialise the cluster with a blank (
initMode: new)cluster specification then they should do so by omitting the file
flag and cluster specification argument, as is already possible.