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