[wildfly-dev] Preventing duplicate registration of capabilities

[Darran Lofthouse]

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