12:54 p.m.
It is not a problem for us to go through our subsystem and change a
value across all resources if you need us to if the decision goes that way.
BUT ;-)
We do expect other subsystems to have resources that advertise our
capabilities, 'security' is already one. An external one would be KeyCloak.
If Elytron was fully configured for unique names and KeyCloak allowed
duplicates would this be enough to get a Stage.MODEL error?
On 07/02/17 18:28, Brian Stansberry wrote:
tl;dr;
Currently on a server (let’s ignore an HC as it’s a tangent) we allow the same capability
to be registered by more than one resource. In the large majority but not all cases a user
doing this has made a mistake we should catch. I plan to make whether this is allowed
configurable by the capability author. I *propose* making the default behavior ‘false’
which which is a behavior change and will require some cleanup.
Only user impact is if some capability needs to allow multiple registrations and doesn’t
pick this up and configure it, and that isn’t caught before release.
Long version
In most cases a server configuration should only have a single resource that registers a
given capability. If more than one registers it, which really provides it? More than one
will likely fail due to conflicts over things like MSC service names.
But, the capability registry actually allows more than one registration point (i.e.
resource address) for the same capability.
In the large bulk of cases this doesn’t matter. Only one type of resource provides a
capability, and if the user tries to configure two of that type of resource with the same
name it will fail due to resource duplication (you can’t add socket-binding=http if there
already is a resource at that address.)
Elytron changes all this a bit as that subsystem has different resources with different
types that all provide the same capability, but each with different implementation deals
(*analogous* to an API providing Set but you can configure use of HashSet or LinkedHashSet
or ConcurrentSkipListSet). Here we want to enforce that the user doesn’t configure two
resources that provide the same capability. If they do, right now this will fail in
runtime (due to MSC service name conflicts) but we want to detect the mistake earlier.
OTOH there are some specialized cases where it is valid for two resources to register the
same capability. Clustering has one (see WFLY-8040); there are others on a Host
Controller. Basically the capability implementations know about each other, check for
duplication at runtime and if present deal with it in a correct manner.
I plan to make whether a capability allows multiple registration points to be
configurable. See https://github.com/wildfly/wildfly-core/pull/2154
Question is whether the default behavior should remain ‘true’ (allow multiple
registrations).
Pros:
1) Any existing capabilties that really need multiple registration can remain unchanged.
This is more problematic for other projects based on WildFly Core that use capabilities as
they may not know about this until they switch to core 3.0. But are there such projects?
(I specifically copied a few folks to directly check).
2) Some subsystem unit test setups that don’t need this but inadvertently rely on it
still work. (I’ve already fixed these in WildFly and WildFly Core so this only relates to
other projects based on WildFly Core that use capabilities.)
Cons:
1) The Elytron use case is more standard, IMO, while the use cases like the clustering
one that allow multiple registrations already require special logic to make that work. So
I’d rather isolate any configuration to allow it to work to the cases that already are
taking special steps.
2) The elytron team is really busy and doesn’t need more things to do.
My thought is the Cons outweigh the Pros, so I plan to change the default behavior. But
your thoughts are welcome.