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) >
