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
