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