[Hawkular-dev] Should Keycloak integration be optional?

[Thomas Segismont]

Hi Juca,

Le 27/01/2015 15:21, Juraci Paixão Kröhling a écrit :
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 01/27/2015 10:34 AM, Thomas Segismont wrote:
>> Le 26/01/2015 09:59, Thomas Heute a écrit :
>>> In terms of priority, we should focus on Hawkular (not just
>>> metrics)
>>
>> Agreed
>>
>>> with Keycloak support and having it optional is not the
>>> priority.
>>
>> Some more context maybe. I asked the question during the hangout
>> and here's why. The PR as-is makes it impossible: * to start a
>> development container without KC * to run tests/integration tests
>> without KC * to install a metrics server without KC
>
> While I agree that an optional KC would make the above points easier,
> I'm failing to see the concrete advantages that it would bring,

Here is a concrete advantage: not being dependent on KC to run a metrics 
server. More below.

> specially considering that the main code will depend on features that
> KC is today bringing (multi tenancy security, for one).
>

After your demo, my understanding was as follows.

1. In terms of APIs, the main code would depend on JAAS rather than KC 
(/@RolesAllowed/ and inspection of a /Subject/ for custom interceptors).

So the good news is that we don't have to implement a pluggable 
identity/authorization tool, it's already there, it's JAAS. And KC is a 
possible backend.

2. With the exception of the console, the impact of making KC optional 
is limited to:
* making the build process create two artifacts for the rest-servlet 
module, one WAR with KC configured in the web.xml, another WAR with 
"classic" security definitions
* creating a JAAS login module to lookup for tenants, users, roles, in a 
simple properties file (or a couple of them).
* finding a way for running the REST integration tests with and without 
KC enabled

As for the console project, I understood that it is considered a 
facility for users of the sole metrics project, not a base for the 
future Hawkular suite. I'd be fine if, for now, it only supported 
basic/digest auth.

> For instance, to start a development environment, one can run the
> start.sh script that currently takes care of installing Wildfly and
> Cassandra. With the PR, KC is added to this mix.
>
> About the integration tests, I'd argue that the correct way to run the
> integration tests is with the security framework enabled, whatever
> this framework is. The reason being that this framework (plus the
> container itself) would provide tenant information to the target code.
> What we have on the PR is exactly this: the tests know nothing about
> KC, the target code knows nothing about KC, but KC is installed on the
> container and provides user information to the target code (or blocks
> the request, if the wrong credentials are provided). For tests where
> no tenant-specific logic is performed, a common credential can be used
> for all tests, which would make more sense than disabling the security
> framework for those tests.

See my answer above about integration tests impact.

>
> I'm not sure what you mean in your last point, though.

My last point was "The PR makes it impossible to install a metrics 
server without KC". I meant that if one needs to configure and run a KC 
server in order to run a metrics server, then many potential users will 
not even give it a try. Potential users here are admins and 
production-focused developers who are working with combos like 
Grafana/Graphite/collectd

Even if "metrics server alone" on its own is not the team's top 
priority, I think it should remain *a* priority, for the reason I've 
just given.

>
>>  From my perspective, that's important enough to consider making it
>> or optional or holding the merge to let us think more about the
>> problem.
>>
>
> Indeed! I think discussing this at this phase is critical, so that we
> can get into a consensus about the expectations :-)

True, and thanks for having patiently awaited my reply ;)

>
> - - Juca.

Thomas