[
https://issues.apache.org/jira/browse/METRON-1665?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16589380#comment-16589380
]
ASF GitHub Bot commented on METRON-1665:
----------------------------------------
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1111
I really like this feature and think it's an important improvement. This
PR is a great start and I don't see any major architectural issues. I've added
some comments inline for the really minor issues. I noticed that
application.yml in metron-config-host and metron-alerts-host is different
between the 2 but I can't think of any reason why. I didn't comment directly
but it's probably worth reviewing.
There are some bigger issues I want to call out that are more complex.
I noticed there is a fairly significant gap with respect to groups and
roles. As far as I can tell it's not supported right now which is a regression
from the JDBC solution currently in place. I'm sure this is expected and will
be solved in follow on tasks in this feature branch. I do think we should set
ourselves up to be able to do that in future PRs though.
In this PR, metron-ui-host and metron-rest use 2 different authentication
mechanisms for testing. I feel they should use the same one and we should
eliminate JDBC or in memory authentication all together. To do this we need to
move this testing infrastructure (the EmbeddedLdap class and supporting files,
Spring properties, etc) to metron-security. This will allow us to switch REST
to a common security model during testing, which is essentially what we're
doing with this feature. I've submitted a PR with an example of how we could
do that (switching REST to embedded ldap isn't included yet and should be done
later to avoid conflicts with other PRs). This will allow us to eliminate
redundant properties across modules but I think the biggest benefit is that our
testing will more accurately match a real installation and allow us to catch
bugs sooner.
I think it's fair to say we need some documentation. I would say at a
minimum the metron-rest README should be updated to reflect the new
authentication approach. Eventually it would be nice to have architectural
level documentation because there are several new components/frameworks being
introduced here. I have no problem with that being done in future PRs.
I also noticed there are several TODOs. We should at some point document
these as follow on tasks so we have a clear understanding of what is expected
to work in each PR.
> Move hosting of Alerts and Config UIs from Nodejs to Spring Boot
> ----------------------------------------------------------------
>
> Key: METRON-1665
> URL: https://issues.apache.org/jira/browse/METRON-1665
> Project: Metron
> Issue Type: Sub-task
> Reporter: Simon Elliston Ball
> Assignee: Simon Elliston Ball
> Priority: Major
>
> The current UIs are served up by very lightweight nodejs applications, which
> serve the static bundle files produced by the angular build process, and
> proxies the rest api.
> The proposal is to use a spring boot application, allowing us to harmonise
> the security implementation across the UI static servers and the REST layer,
> and to provide a routing platform for later microservices.
> The UIs currently proxy to the REST API to avoid CORS issues, this will be
> achieved with Zuul.
> Spring Security will also be extended to use a Knox SSO authenticator.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
