Our first step is probably to deprecate it. We have some gfsh commands that have deprecated info in the help (search for deprecated in CliStrings.java) mostly for options we want to remove.
On Mon, Dec 30, 2019 at 9:37 AM Jinmei Liao <jil...@pivotal.io> wrote: > If we can remove these support, I would like this approach too. Current > implementation has too many inconsistencies. > > On Mon, Dec 30, 2019 at 9:34 AM Kirk Lund <kl...@apache.org> wrote: > > > My vote is for #2 in Jinmei's proposal with the following correction: > > > > After removing --host/--port, you still have two ways to get "status > > locator": > > > > a) Use --dir option: > > > > gfsh>status locator --dir=/locator-dir > > > > b) Use connect command first: > > > > gfsh>connect locator <etc> > > gfsh>status locator --name=locator1 > > > > > > On Mon, Dec 30, 2019 at 9:30 AM Kirk Lund <kl...@apache.org> wrote: > > > > > Long before we open-sourced as Geode, GemFire engineering planned to > > > remove support for: > > > > > > gfsh>status locator --host=fooish --port=10334 > > > > > > The reasoning at the time was (1) this method for getting details about > > > the locator was wide open (security hole), and (2) it's inconsistent > with > > > "status server" which has no such counterpart. > > > > > > I prefer: remove --host and --port from status locator to be more > > > consistent with status server and avoid having to add something that no > > > user has requested (as far as I know) > > > > > > If we need to add SSL support for --host and --port to status locator, > > > then we should include adding --host and --port to status server as > well > > to > > > improve consistency and usability -- I use the term "usability" because > > > users expect "status server --host=xxx --port=xxx" to also work IF we > > > continue to provide "status locator --host=xxx --port=xxx". We need to > > get > > > rid of and avoid these weird quirks and inconsistencies. > > > > > > On Mon, Dec 23, 2019 at 11:24 AM Jinmei Liao <jil...@pivotal.io> > wrote: > > > > > >> So, looks like need to do either one of the following: > > >> 1. add --security-property-file option for the user to specify the ssl > > >> connection properties > > >> 2. remove --host/--port combination and remove the capability to > report > > >> the > > >> status of the cluster configuration, this would only allow "status > > >> locator" > > >> command to report the locally started locator's status > > >> > > >> I would vote for option 1, given that option 2 greatly changes > behavior. > > >> > > >> If no other objections, I will go ahead with option 1. > > >> > > >> Jinmei > > >> > > >> On Fri, Dec 20, 2019 at 4:20 PM Jens Deppe <jde...@pivotal.io> wrote: > > >> > > >> > I think Jinmei is also referring to the situation where you might > > *not* > > >> > already > > >> > be connected but want to execute something like this: > > >> > > > >> > gfsh>status locator --host=fooish --port=10334 > > >> > > > >> > > > >> > In this case it would fail as well. Seems like we'd either need to > > >> remove > > >> > those options so you *always* need to 'connect' if you want to > access > > a > > >> > remote system, or we need to add an *optional* > > >> --security-properties-file > > >> > option that would only be necessary if the shell isn't already > > >> connected. > > >> > > > >> > --Jens > > >> > > > >> > > > >> > > > >> > > > >> > On Fri, Dec 20, 2019 at 3:15 PM Ernest Burghardt < > > eburgha...@pivotal.io > > >> > > > >> > wrote: > > >> > > > >> > > I like the approach Jens is suggesting, seems intuitive to me > > >> > > > > >> > > On Thu, Dec 19, 2019 at 6:31 PM Jens Deppe <jde...@pivotal.io> > > wrote: > > >> > > > > >> > > > So it seems that the situation is something like this where I'm > > >> able to > > >> > > > make the initial connection but then retrieving status fails: > > >> > > > > > >> > > > gfsh>connect --security-properties-file=../security.properties > > >> > > > > > >> > > > gfsh>status locator --name=locator1 > > >> > > > Server version response invalid: This could be the result of > > trying > > >> to > > >> > > > connect a non-SSL-enabled client to an SSL-enabled locator. > > >> > > > > > >> > > > > > >> > > > It would seem very weird if I have to provide additional > > connection > > >> > > params > > >> > > > to the 'status' command if I've already provided them as part of > > the > > >> > > > 'connect'. Could we not stash the connection properties (in the > > Gfsh > > >> > > > instance object) for subsequent usage? > > >> > > > > > >> > > > --Jens > > >> > > > > > >> > > > On Wed, Dec 18, 2019 at 9:04 AM Jinmei Liao <jil...@pivotal.io> > > >> wrote: > > >> > > > > > >> > > > > "status locator" command is broken on ssl enabled locators > ever > > >> since > > >> > > we > > >> > > > > fixed a bug that leaked the connection properties from one tcp > > >> socket > > >> > > > > connection to another. Before that it would just magically > work > > >> if we > > >> > > > have > > >> > > > > previously made a successfully tcp connection to that same > > >> locator, > > >> > now > > >> > > > we > > >> > > > > are either required to find a way to specify the ssl > properties > > >> when > > >> > > > > running the `status locator` command or change what we want to > > >> report > > >> > > > back > > >> > > > > to the user. > > >> > > > > > > >> > > > > Here is what's happening now when we issue the command: > > >> > > > > > > >> > > > > 1. it needs to retrieve two sets of info from locator: > > general > > >> > info > > >> > > > like > > >> > > > > (pid, working dir, status, jvm args etc) and whether > cluster > > >> > > > > configuration > > >> > > > > service is running or not. > > >> > > > > 2. when locator’s ssl is on, the retrieval of the cluster > > >> > > > configuration > > >> > > > > info will always fail since it’s using a tcp connection to > > get > > >> > that > > >> > > > info > > >> > > > > and we currently don’t have the ssl security properties to > > >> > connect. > > >> > > > > 3. when locator’s ssl is on, the retrieval of the general > > info > > >> > will > > >> > > > > mostly succeed except when user is only providing a host > and > > >> port, > > >> > > > > there we > > >> > > > > would also need the ssl security properties in order to > > create > > >> an > > >> > > ssl > > >> > > > > socket. > > >> > > > > > > >> > > > > > > >> > > > > So we wither need to add an option for user to specify a > > >> > > > > `--security-properties-file` option to include all the ssl > > >> connection > > >> > > > > properties or change the behavior not to report cluster config > > >> status > > >> > > or > > >> > > > > not allowing host/port combination. (I am not here long enough > > to > > >> > > > > understand what users would use this command for, so this is > > just > > >> a > > >> > > > random > > >> > > > > suggestions). > > >> > > > > > > >> > > > > Comments/thoughts? > > >> > > > > > > >> > > > > -- > > >> > > > > Cheers > > >> > > > > > > >> > > > > Jinmei > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > > >> -- > > >> Cheers > > >> > > >> Jinmei > > >> > > > > > > > > -- > Cheers > > Jinmei >
