geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rick McGuire <rick...@gmail.com>
Subject Re: Pre-RTC look at the openejb/geronimo yoko support and request for input [long].
Date Mon, 18 Sep 2006 09:05:14 GMT
David Jencks wrote:
> Now I've reviewed this a bit more and have some minor comments:
>
> 1. I think you should include the yoko jars only in j2ee-corba (or 
> openejb) and client-corba configurations, not also j2ee-server and 
> client configurations.  We might need to split more of openejb into a 
> corba support module so j2ee-corba can contain all of it and then we 
> can deploy openejb w/o pulling in any corba stuff.
That's originally what I tried, but that failed because of class loading 
issues.  The  Yoko jars needed to be loaded by the same class loader  
that was  loading the openejb-core code otherwise the ORB adapter code 
(which was in the openejb-core jar) was unable to resolve the yoko 
classes. 

>
> 2. Comment in TSSBean is not quite accurate.  A TSSBean can be used 
> with any number of ejbs.  It just supplies the additional security 
> info beyond the ssl config that the server will require for accessing 
> the ejbs that reference it.  BTW dblevins is rather unhappy with the 
> reference from ejb container to tssbean on the grounds that transport 
> info should not be associated with the ejb container itself.  One way 
> to support this would be to have an additional "link" gbean between 
> the tssbean and the ejb container.  Another way would be to give the 
> TSSBean a collection of ejbs it services: this was the original 
> implementation but it was impossible to configure for the tck interop 
> tests.  One of these approaches might be required to separate the 
> corba stuff out from "core" openejb.
Ok, I'll correct the comments.  Every time I think I've really got a 
handle on what this is doing, I discover something new like this :-)

>
> 3. This code from SocketFactory just doesn't look right: (around line 
> 298 or the patch)
>
> +        // now set our listener creation flags based on the supports 
> and requires values from the
> +        // TSS config.
> +        if ((supports & EstablishTrustInClient.value) != 0) {
> +            clientAuthSupported = true;
> +
> +            if ((requires & EstablishTrustInClient.value) != 0) {
> +                clientAuthRequired = true;
> +            }
> +        }
> +
> +        if ((supports & EstablishTrustInTarget.value) != 0) {
> +            clientAuthSupported = true;
> +
> +            if ((requires & EstablishTrustInTarget.value) != 0) {
> +                clientAuthSupported = true;
> +            }
> +        }
> At least the final clientAuthSupported = true looks redundant to me.
> there's similar code around line 565 of the patch:
Good catch....this needs to be "clientAuthRequired = true;".  Same 
problem in the Sun code.


> +        // figure out the supports and requires information from the 
> flag values
> +        boolean authSupported = false;
> +        boolean authRequired = false;
> +
> +        if ((supports & EstablishTrustInClient.value) != 0) {
> +            authSupported = true;
> +
> +            if ((requires & EstablishTrustInClient.value) != 0) {
> +                authRequired = true;
> +            }
> +        }
> +
> +        if ((supports & EstablishTrustInTarget.value) != 0) {
> +            authSupported = true;
> +
> +            if ((requires & EstablishTrustInTarget.value) != 0) {
> +                authSupported = true;
> +            }
> +        }
>
> 3. Your comment on CSSBean suffers like the one for TSSBean.  A 
> CSSBean just holds the security config info, but you can use one 
> CSSBean to access many remote ejbs.
Yep, I'll correct.

>
> 4. The code I don't understand in (2) appears again around line 3980 
> in sunorb/OpenEJBSocketFactory
>
> This looks great to me!
>
> thanks
> david jencks
>
>
> On Sep 14, 2006, at 2:56 PM, David Jencks wrote:
>
>> Great work!!!
>> On Sep 14, 2006, at 12:16 PM, Rick McGuire wrote:
>>
>>> I've just attached patches for issue 
>>> http://issues.apache.org/jira/browse/GERONIMO-2180, which is to add 
>>> Yoko support to Geronimo.  This is really patches for this issue 
>>> plus 2 other issues that are highly related:
>>>
>>>    http://issues.apache.org/jira/browse/GERONIMO-2002  OPENEBJ CORBA
>>>    SSL should use Keystore GBean
>>>    http://issues.apache.org/jira/browse/GERONIMO-2353  Reduce the
>>>    number of places where CORBA config parameters are specified.
>>>
>>> This should also be the first step toward achieving this goal:
>>>
>>>    http://issues.apache.org/jira/browse/GERONIMO-433  Tolerate 
>>> non-Sun JREs
>>>
>>> And should also be a step toward allowing full support of Java 5.
>>>
>>> This code works as far as being able to start and stop the 
>>> j2ee-corba system module.  Fuller testing is going to require 
>>> getting the MagicGBall app working and then see how this works with 
>>> TCK testing.  There are some issues with doing either of those steps 
>>> at the moment, but I decided this is a good point to show people 
>>> I've done, since it will be easier to ask questions about it.
>>>
>>> Let me give the basics of what I've done, and I have a few areas I'd 
>>> like community input on how I should proceed from here.
>>> The bulk of the changes are really around GERONIMO-2353.  While 
>>> trying to fit the Yoko ORB into this structure, I found a number of 
>>> pain points:
>>>
>>>   1. The org.openejb.corba.SunNameService GBean only supported the Sun
>>>      ORB, and was not generally configurable like CORBABean or CSSBean
>>>      were.
>>>   2. The CORBABean and CSSBean configuration included "args" and
>>>      "props" items which were passed directly through to an ORB.init()
>>>      call.  These attributes were used to configure things like the
>>>      initial listener port, the host server name, and the initial
>>>      NameServer location.  In a few cases, the values set were not
>>>      portable between ORB implementations, which made it more difficult
>>>      to switch between ORBs.
>>>   3. The CSSBean and CORBABean declarations needed to be coded with a
>>>      dependency on SystemProperties.  The SystemProperties object was
>>>      initializing various system properties that were needed by the
>>>      ORB, and also enabled the RMI support.  These properties were
>>>      generally not portable between ORB implementations, since they
>>>      included references to Sun specific classes.
>>>
>>> To clean this up, I reworked the ConfigAdapter interface used in the 
>>> current code base.  This interface now has 3 basic operations 1)  
>>> create a name service, 2)  create a server ORB, and 3) create a 
>>> client ORB.  The existing code is just configured with a 
>>> ConfigAdapter class name and the CORBABean/CSSBean services 
>>> instantiated an instance of the class.  Now the ConfigAdapters are 
>>> GBean instances, and the doStart() methods of these GBeans are 
>>> encapsulate the responsibility for setting the RMI system 
>>> properties.  SunNameService has been replaced by a generic 
>>> NameService GBean, and NameService, CORBABean, and CSSBean all take 
>>> a ConfigAdapter instance in their constructors.  Now, from a plan 
>>> standpoint, it's possible to switch between ORBs by changing a 
>>> single line in the plan.   All of this work is really independent of 
>>> the Yoko-specific changes, but did make it easier to write the Yoko 
>>> code.
>>
>> This sounds great!
>>>
>>> Which brings me to
>>>
>>> ISSUE #1:  I added a NameService argument to the CORBABean 
>>> constructor.  The ConfigAdapter would take this NameService 
>>> instance, and configure the ORB to use the NameService.getURI() 
>>> result for it's initial  NameService reference.  Well, when trying 
>>> to get Geronimo to build, I got a failure on one of the client plans 
>>> because there was a CORBABean coded, but no NameService.  The 
>>> CORBABean had use the now obsolete arguments attribute to configure 
>>> the ORB to use a remote NameService.  I thought on this a little, 
>>> and decided to just add a "local" attribute to the NameService 
>>> GBean.  If local is false, then the bean does not launch a local 
>>> server instance and the getURI() returns the remote location of the 
>>> NameService as specified by the host/port combination.  This worked 
>>> very well, but it somehow feels like a convenience hack to me.  Does 
>>> this sound ok, or should I take some other approach with this?
>>>
>>
>> This seems reasonable to me.  There might be an even better way to 
>> deal with this, but we definitely need to support both a name server 
>> in the same vm (in which case with luck we can communicate with it 
>> in-vm without tcp) or a remote name server.  We were starting a name 
>> server in vm mostly because it's simpler to administer.  
>> Theoretically we could start an app client where all it did was run 
>> the name server :-).
>>
>>> For GERONIMO-2002, I create a new SSLConfig GBean.  This class has a 
>>> reference to a KeystoreManager GBean, plus various attributes that 
>>> are required to generate SSLSocketFactory and SSLServerSocketFactory 
>>> instances for creating the SSL sockets.  The CORBABean and CSSBean 
>>> objects can be configured with an SSLConfig reference, which is then 
>>> used whenever an SSL connection is required.  This is separate from 
>>> the TSSConfig/CSSConfig specifications.  TSSConfig/CSSConfig help 
>>> determine WHEN an SSL connection is required.  The SSLConfig 
>>> determines HOW the connection gets created when it is required.
>>> ISSUE #2:  This works fairly well for the j2ee-corba plan, which 
>>> imports the j2ee-security plan.  The j2ee-security plan defines the 
>>> default KeystoreManager instances, so things get resolved properly.
>>>
>>> On the client side, the client-corba plan does not import 
>>> j2ee-security, so I didn't have a configured KeystoreManager to work 
>>> with.  It did not seem appropriate to import the j2ee-security plan, 
>>> since there were items here that did not apply well to a client 
>>> configuration.  As a shortcut, I just copied the KeystoreManager 
>>> definitions into the client plan, but I'm not sure I'm comfortable 
>>> that this will define/locate the KeystoreManagers properly.  Does 
>>> anybody with more experience with the security support have 
>>> suggestions for how this should be handled?
>>
>> I think you should put the KeystoreManager gbean into the 
>> client-security plan.  You will definitely break things if you try to 
>> start any server configuration such as j2ee-security in the client.
>>
>> This is excellent!
>>
>>>
>>> And, finally, GERONIMO-2180.  This code was rather straightfoward 
>>> once I'd completed the above items.  I just created an 
>>> org.openejb.corba.yoko package, added a ConfigAdapter 
>>> implementation, plus whatever ORB-specific classes were required to 
>>> bridge between the ORB and Geronimo.  Not really a lot of code in 
>>> this package.  BUT....
>>>
>>> ISSUE #3:  In order for the Yoko ORB to function properly, the Yoko 
>>> jar files need to be part of the endorsed.dir configuration or 
>>> included on the bootstrap classpath.  This makes it very difficult 
>>> for the Yoko and the Sun code to coexist in the same build tree.  
>>> The code will compile ok, but unit tests are a problem.  There are a 
>>> couple of tests that caused problems.  The SunNameService class had 
>>> a test which I replicated for the Yoko NameService.  If the build 
>>> was enabled for the Sun ORB, the Yoko test would cause a build 
>>> failure.  If enabled for the Yoko ORB, the Sun test would fail.  
>>> When I made the changes to have a generic NameService GBean, both of 
>>> these tests became obsolete, so they are deleted for now.  Once we 
>>> sort out the coexistance strategy, I'll try introducing new tests.  
>>> There was a similar problem with one of the TSSConfigEditorTest, 
>>> which needed to create an configure a CORBABean instance.
>>>
>>> On the Geronimo side, there are similar problems.  Building any of 
>>> the corba configurations depended upon whether the yoko classes were 
>>> in endorsed.dir.  If there were absent, it was not possible to build 
>>> a yoko-based configuration.  If present, it was not possible to 
>>> build the Sun-based configuration.  There was some suggestion that 
>>> we might need to ship additional full assemblies to accommodate this.
>>
>> I think we should consider setting up a parallel universe of yoko 
>> assemblies with additional configs modules as needed and find out how 
>> much work we have to convince the interop tck tests to pass.  At that 
>> point we will have a better idea what to do next.
>>>
>>> For the openejb2 code tree, I see several possibilities:
>>>
>>> 1)  Leave the Sun ORB code in the tree, make the yoko package a 
>>> separate module that with a dependency on the openejb2 code.  The 
>>> existing build works ok, and the tests can be built for the Sun 
>>> ORB.  The build of the yoko package could then have its own versions 
>>> of the tests, which would work find.
>>> 2)  Replace the Sun ORB code with the yoko code and kick the Sun 
>>> code into a separate module.  Same things apply with the test.
>>> 3)  Place both ORB adapters in outside modules, each with their own 
>>> builds and tests.
>>
>> I vote for (3).  I've wished the corba code was in 2 additional 
>> modules (runtime and builder) for a really long time.
>>>
>>> Possibility 1)  Has one serious disadvantage as it leaves the 
>>> openejb2 code tree coupled to the Sun 1.4.2 JVM.  Either 2 or 3 will 
>>> remove that particular Java 1.4.2 dependency.  Does anybody have and 
>>> strong feelings about this?
>>>
>>> ISSUE #4 is then how do we manage the possibility of both the Sun 
>>> ORB support and the Yoko support?  Will this actually require 
>>> separate assemblies to work, or is there some means to easily allow 
>>> the switching?
>>
>> I suspect we should be able to set it up so that the openejb configs 
>> don't depend on the corba configs/jars so we might be able to put in 
>> the corba support as plugins or have several assemblies.
>>
>> After we get yoko working does anyone think we need to preserve sun 
>> orb support?
>>
>>>
>>> Anyway, a lot of words to digest.  Issues #3 and #4 are the ones 
>>> that are going to cause the most pain to implement, so I'm really 
>>> interested in getting community consensus on how to proceed here.
>>
>> This is exciting!
>>
>> thanks
>> david jencks
>>
>>>
>>> Rick
>>
>
>


Mime
View raw message