I think we should revisit connecting to the distributed system with properties. In addition to what Kirk mentioned below, I think one of the big limitation of this is the inability to change properties on a running cluster. If there is a property that a user needs to change, they will have to bounce the cluster as opposed to performing a rolling restart of the cluster with that property changed. I think the quickest way for us to get there is to make the connection a two step process, connect just to get the properties from the cluster config avoiding any validation, then reconnect using the new set of properties.
On Fri, Jun 9, 2017 at 2:54 PM Kirk Lund <kl...@apache.org> wrote: > The usage of CacheFactory#setSecurityManager (and setPostProcessor) is > working as expected, both before and after my changes! > > The problem is that Cluster Config is requested during Cache initialization > which means that any gemfire.properties stored in Cluster Config will not > be used by DistributedSystem -- only the properties that affect the Cache > (such as cache-xml-file) are used as expected in Cluster Config. This > problem exists with/without my changes but my changes exposed this fact by > moving the use of the security-manager property to DistributedSystem. I'm > working on a short-term fix specific to security-manager. Correcting the > problem for all gemfire.properties will be a long-term fix. > > On Fri, Jun 9, 2017 at 1:48 PM, John Blum <jb...@pivotal.io> wrote: > > > I am not sure I follow everything that is happening here quite yet as I > > have not had time to go over this in more detail and digest it. > > > > But, I certainly hope that the security-manager property in Geode is not > > impacted by this in anyway since *Spring Data Geode* builds on this and, > > well, this > > <https://github.com/apache/geode/blob/develop/geode-core/ > > src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368> > > [1], > > as way to configure Geode (Integrated) Security. > > > > Thanks, > > John > > > > [1] > > https://github.com/apache/geode/blob/develop/geode-core/ > > src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368 > > > > > > On Fri, Jun 9, 2017 at 1:11 PM, Kirk Lund <kl...@apache.org> wrote: > > > > > Yeah I think we need #2 in the long run. Right now nearly all of the > > > gemfire.properties are ignored if you use Cluster Config. The few > > > gemfire.properties that are mutable by RuntimeDistributionConfigImpl > will > > > work when used in Cluster Config -- I believe all other > > gemfire.properties > > > would be ignored until we complete #2. > > > > > > On Fri, Jun 9, 2017 at 1:02 PM, Udo Kohlmeyer <ukohlme...@pivotal.io> > > > wrote: > > > > > > > +1 for #2. It does seem more correct > > > > > > > > > > > > > > > > On 6/9/17 12:45, Kirk Lund wrote: > > > > > > > >> The new changes for SecurityService don't work with Cluster Config. > We > > > >> only > > > >> had one test that would've failed but it was annotated with @Ignore. > > > >> > > > >> The problem is this: > > > >> > > > >> The security-manager is a gemfire property and is now used by the > > > >> InternalDistributedSystem because membership needs SecurityService > to > > > >> handle peer-to-peer authentication. So with my changes, the > > > >> InternalDistributedSystem now creates SecurityService and passes it > > into > > > >> the constructor of GemFireCacheImpl. > > > >> > > > >> Next, GemFireCacheImpl#initialize requests Cluster Config. If this > is > > a > > > >> new > > > >> Server with no gemfire properties, it will have already joined the > > > Cluster > > > >> (before and after my changes). It then gets Cluster Config that has > > > >> gemfire.properties including security-manager. But it's too late, > the > > > >> immutable SecurityService has already been created by > > > >> InternalDistributedSystem. > > > >> > > > >> In theory, Cluster Config should be requested before the creation of > > > >> InternalDistributedSystem so that other gemfire.properties in > Cluster > > > >> Config can be applied (such as member-timeout). In fact most of the > > > >> gemfire.properties control aspects of InternalDistributedSystem. > > > >> Presently, > > > >> all gemfire.properties that would configure > InternalDistributedSystem > > > are > > > >> ignored because Cluster Config is loaded after > > InternalDistributedSystem > > > >> was created and the member has joined the cluster. > > > >> > > > >> We would need to make one of these changes: > > > >> > > > >> 1) GemFireCacheImpl creates its own SecurityService which is > different > > > >> from > > > >> the one used by membership. Locators are the only members that > really > > > need > > > >> membership to have a fully configured SecurityService (and I think > we > > > >> already have a bug in which 2nd Locators need to be manually > > configured > > > >> rather than use Cluster Config). > > > >> > > > >> 2) Move Cluster Config request from GemFireCacheImpl to > > > >> InternalDistributedSystem. This is actually more correct, since > > Cluster > > > >> Config is potentially going to contain a lot of > > > InternalDistributedSystem > > > >> configuration options that are currently ignored (in both Geode 1.x > > and > > > >> current develop). > > > >> > > > >> Unfortunately #2 is a fairly big refactoring change. > > > >> > > > >> We should probably revert my changes from develop and 1.2 until #1 > or > > #2 > > > >> can be completed. Or we have to file a bug that says Cluster Config > > and > > > >> Security don't play nice together until we can complete one of these > > > >> options. I think the latter is probably not acceptable. > > > >> > > > >> > > > > > > > > > > > > > > > -- > > -John > > john.blum10101 (skype) > > >
