I'm coming in late to the game here, but for my mind a feature branch should involve the minimum architectural change to accomplish a given feature. The feature in question is SSO integration. It seems to me that the operative question is can we do the feature without making the OTHER architectural change (e.g. migrating from expressjs to spring boot + zuul). I would argue that if we WANT to do that, then it should be a separate feature branch.
Thus, I leave with a question: is there a way to accomplish this feature without ripping out expressjs? - If so and it is feasible, I would argue that we should decouple this into a separate feature branch. - If so and it is infeasible, I'd like to hear an argument as to the infeasibility and let's decide given that - If it is not possible, then I'd argue that we should keep them coupled and move this through as-is. On a side-note, it feels a bit weird that we're narrowing to a bundled proxy, rather than having that be a pluggable thing. I'm not super knowledgeable in this space, so I apologize in advance if this is naive, but isn't this a pluggable, external component (e.g. nginx)? On Thu, Sep 27, 2018 at 5:05 PM Michael Miklavcic < michael.miklav...@gmail.com> wrote: > I've spent some more time reading through Simon's response and the added > sequence diagram. This is definitely helpful - thank you Simon. > > I need to redact my initial list: > > 1. Node migrated to Spring Boot, expressjs migrated to a > non-JS/non-NodeJs proxying mechanism (ie Zuul in this case) > 2. JDBC removed completely in favor of LDAP > 3. Knox/SSO > > I'm a bit conflicted on the best way to move forward and would like some > thoughts from other community members on this. I think an argument can be > made that 1 and 2 are independent of 3, and should/could really be > independent PR's against master. > > The need for a replacement for expressjs (Zuul in this case) is an artifact > that our request/response cycle for REST calls is a simple matter of > forwarding with some additional headers for authentication. There's a > JSESSIONID managed by the client browser in our current architecture, for > example. You login to the alerts or the management UI which forwards a > request to REST, which looks up credentials in a backend database, and > passes the results back up the chain. All browser requests go directly to > the specific UI you're working with - this is the CORS problem. You can't, > without some effort with headers for adding other domains to the safe list > or disabling the security check for CORS, make remote calls directly to > REST. That's why we proxy. Switching over to Spring Boot leaves a gap with > expressjs having handled the proxying and filtering, since it's only > available to a NodeJs application (it's server-side javascript vs the > client side javascript deployed via our Angular applications). Enter Zuul, > which now effectively handles that. At runtime, Zuul is a part of the > Spring app that serves up our UI's. It handles the requests via filtering, > forwards them to REST, manages the response back to the client. Very > similar to what expressjs was doing, per my current understanding. The > sequence diagrams Simon added are useful, and I think some of what was less > clear was what we currently vs what the new changes are doing to the > architecture. This is no fault of Simon's - there simply wasn't any > architecture diagrams/documents around this before. Here's my impression of > the very very basic current state - someone more familiar with this > architecture please advise if I'm incorrect about anything (probably Ryan). > > https://imgur.com/f8GtSmh > > Zuul would be replacing the bit about expressjs in the diagram, and instead > of node we have spring boot. This covers 1. 2 and 3 are other issues. I'd > like to see similar exposition of those server processes with knox > involved. I imagine in that case we bump up from 3 to 4 server instances > for the additional knox endpoint. > > Mike > > > > > > On Wed, Sep 19, 2018 at 11:28 AM James Sirota <jsir...@apache.org> wrote: > > > Thank you, Simon. The diagrams help a lot > > > > 19.09.2018, 21:27, "Simon Elliston Ball" <si...@simonellistonball.com>: > > > To clarify some of this I've put some documentation into > > > https://github.com/apache/metron/pull/1203 under METRON-1755 ( > > > https://issues.apache.org/jira/browse/METRON-1755). Hopefully the > > diagrams > > > there should make it clearer. > > > > > > Simon > > > > > > On Tue, 18 Sep 2018 at 14:17, Simon Elliston Ball < > > > si...@simonellistonball.com> wrote: > > > > > >> Hi Mike, > > >> > > >> Some good points here which could do with some clarification. I > suspect > > >> the architecture documentation could be clearer and fill in some of > > these > > >> gaps, and I'll have a look at working on that and providing some > > diagrams. > > >> > > >> The short version is that the Zuul proxy gateway has been added to > > replace > > >> the Nodejs express proxy used to gateway the REST api calls in the > > current > > >> hosts. This is done in both cases to avoid CORS restrictions by > > allowing > > >> the same host that serves the UI files to proxy call to the API. > > >> > > >> The choice of Zuul was partly a pragmatic one (it's the one that's > > there > > >> in the box as it were with Spring Boot, which we use for the REST > API, > > via > > >> the Spring Cloud Netflix project which wraps a bunch of related > pieces > > into > > >> Spring). The choice of Spring Boot to host the UIs themselves was > > similarly > > >> for parity with the REST host, to simplify the stack (we remove the > > >> occasionally problematic need to install nodejs on target servers, > > which is > > >> outside of the regular OS and HDP stacks we support). > > >> > > >> Arguably, the Zuul proxy is not necessary if we force everything > > through a > > >> Knox instance, since Knox would provide a single endpoint. We > probably > > >> however don't want to force Knox and SSL, hence using Zuul to keep it > > >> closer to our current architecture. Zuul does some other nice things, > > which > > >> might help us in future, so it's really about laying down some > options > > for > > >> potentially doing micro-services style things at a later date. I'm > not > > >> saying we have to, or even should go that way, it will just make life > > >> easier later if we decide to. It will also help us if we want to add > > HA, > > >> circuit breaking etc to the architecture at a later date. That said, > I > > >> regret that I ever said the word micro-services, since it's caused > > >> confusion. Just think of it as a proxy to deal with the CORS problem. > > >> > > >> Zuul is implemented as a set of filters, but we are not using it for > > its > > >> authentication filtering. We're using it as a proxy. Shiro is an > > >> authentication framework, and could arguably used to provide the > > security > > >> piece, but frankly wrapping shiro as a replacement for Spring > Security > > in a > > >> Spring application seemed like it will make life a lot harder. This > > could > > >> be done, but it's not the native happy path, and would pull in > > additional > > >> dependencies that duplicate functionality that's already embedded in > > Spring > > >> Security. > > >> > > >> The version of Knox used is the default from HDP. The link version > you > > >> mention is a docs link. I'll update it to be the older version, which > > is > > >> the same and we can decide if we want to maintain the freshness of it > > when > > >> we look to upgrade underlying patterns. Either way, the content is > the > > >> same. > > >> > > >> I did consider other hosting mechanisms, including Undertow a > > >> > > >> If you have a different suggestion to using the Spring default ways > of > > >> doing things, or we want to use a framework other than Spring for > this, > > >> then maybe we could change to that, but the route chosen here is > > definitely > > >> the easy path in the context of the decision made to use Spring in > > metron > > >> rest, and if anything opens up our choices while minimising, in fact > > >> reducing, our dependency management overhead. > > >> > > >> I hope that explains some of the thinking behind the choices made, > but > > the > > >> guiding principals I followed were: > > >> * Don't fight the framework if you don't have to > > >> * Reduce the need for additional installation pieces and third party > > repos > > >> * Minimize dependencies we would have to manage > > >> * Avoid excessive change of the architecture, or forcing users to > adopt > > >> Knox if they didn't want the SSL overhead. > > >> > > >> Simon > > >> > > >> On Tue, 18 Sep 2018 at 02:46, Michael Miklavcic < > > >> michael.miklav...@gmail.com> wrote: > > >> > > >>> Thanks for the write-up Ryan, this is a great start. I have some > > further > > >>> questions based on your feedback and in addition to my initial > thread. > > >>> > > >>> Just for clarification, what version of Knox are we using? HDP > 2.6.5, > > >>> which > > >>> is what we currently run full dev against, supports 0.12.0. > > >>> > > >>> > > > https://docs.hortonworks.com/HDPDocuments/HDP2/HDP-2.6.5/bk_release-notes/content/comp_versions.html > > >>> . > > >>> I see references to Knox 1.1.0 (latest) in this committed PR - > > >>> > > >>> > > > https://github.com/apache/metron/pull/1111/files#diff-70b412194819f3cb829566f05d77c1a6R122 > > >>> . > > >>> This is probably just a super small mismatch, and it probably goes > > without > > >>> saying, but I just want to be doubly sure that we're installing the > > >>> default > > >>> via the standard install mechanism as opposed to something separate > > and > > >>> manual. > > >>> > > >>> On the subject of Zuul wrt Nodejs filters. I'd like to hear some > more > > >>> detail on: > > >>> > > >>> 1. Why do we need filtering via Zuul? For instance, is filtering > > >>> routing > > >>> not handled by Knox? From the beginner docs: "The gateway itself > > is a > > >>> layer > > >>> over an embedded Jetty JEE server. At the very highest level the > > >>> gateway > > >>> processes requests by using request URLs to lookup specific JEE > > Servlet > > >>> Filter chain that is used to process the request. The gateway > > framework > > >>> provides extensible mechanisms to assemble chains of custom > filters > > >>> that > > >>> support secured access to services." [1] > > >>> 2. What other library options were considered for this feature > and > > how > > >>> was it chosen over the others? I search on "apache spring web > > filters" > > >>> and > > >>> it's almost all about Shiro - > https://shiro.apache.org/spring.html. > > I > > >>> also see quite a bit about filtering for Spring Boot applications > > along > > >>> with a write-up of how to accomplish the same with Web MVC here - > > >>> > > >>> > > > https://stackoverflow.com/questions/19825946/how-to-add-a-filter-class-in-spring-boot > > >>> . > > >>> The Knox documentation boilerplate examples are also using Shiro. > > >>> "shiro.ini - The configuration file for the Shiro authentication > > >>> provider’s > > >>> filters. This information is derived from the information in the > > >>> provider > > >>> section of the topology file." [1] > > >>> > > >>> My assumption is that there are deliberate decisions in favor of > this > > mix > > >>> of technologies over others, and I think some additional explanation > > will > > >>> make that clear. As it stands per the Knox documentation, it looks > > like > > >>> we're going on a different route from the preferred/recommended > > idioms. > > >>> > > >>> [1] > > >>> > > >>> > > > http://knox.apache.org/books/knox-0-12-0/dev-guide.html#Architecture+Overview > > >>> > > >>> Ryan, I agree about microservices. This should not derail nor be a > > major > > >>> part of discussion around this feature, imho. I think there's quite > a > > bit > > >>> left to discuss on that subject. I want to make sure that we're not > > >>> prematurely favoring architectural choices by pulling in libraries > > that > > >>> are > > >>> potentially opinionated about how to accomplish those goals. If they > > are, > > >>> I > > >>> would expect we are comfortable ripping those libraries out if the > > >>> community favors a different direction. > > >>> > > >>> On the subject of Spring Boot vs Nodejs. I can see some rationale > for > > >>> making things homogenous (though, in a microservices architecture, > if > > we > > >>> go > > >>> that route, that's not strictly necessary), but what is the > > justification > > >>> for Spring Boot over Nodejs? Why would want one over the other? > > >>> > > >>> On Mon, Sep 17, 2018 at 3:38 PM Ryan Merriman <merrim...@gmail.com> > > >>> wrote: > > >>> > > >>> > I have reviewed a couple different PRs so I'll add some context > > where I > > >>> > can. Obviously Simon would be the most qualified to answer but > I'll > > >>> add my > > >>> > thoughts. > > >>> > > > >>> > For question 1, while they may not all be necessary I think it > does > > make > > >>> > sense to include them in this feature branch if our primary goal > is > > >>> > integrating Knox SSO. We could push off removing JDBC > authentication > > >>> for > > >>> > reasons I'll get to in my response to question 2. If we want to do > > one > > >>> at > > >>> > a time (switch to spring boot, add Zuul as a dependency, then add > > Knox > > >>> SSO) > > >>> > then that's ok but I do think there are dependencies and should be > > done > > >>> in > > >>> > order. For example, adding Knox SSO requires some work around > > request > > >>> > filtering. If we were to do this before moving to Spring Boot we > > would > > >>> > need to implement the filters in Nodejs which would be throwaway > > once we > > >>> > get around to migrating away from that. For Zuul, I believe it's > > >>> purpose > > >>> > is to facilitate the filtering (although it does a lot more) so it > > >>> doesn't > > >>> > make sense to add that separate from the Knox SSO work. > > >>> > > > >>> > For question 2, I think you bring up a good point. We probably > don't > > >>> want > > >>> > to just rip our current authentication method out. We might want > to > > >>> > consider deprecating it instead and making Knox SSO and LDAP > > >>> authentication > > >>> > optional. > > >>> > > > >>> > For question 3, this is a bigger shift than just a component > > upgrade. > > >>> It's > > >>> > more like shifting platforms, from Elasticsearch to Solr for > > example. > > >>> Like > > >>> > I alluded to in my response to question 1, I don't think we should > > >>> require > > >>> > throwaway work just because we want to review these parts > > separately. > > >>> > > > >>> > For question 4, I will defer to Simon. I don't believe we > > necessarily > > >>> > require Zuul so I will let him elaborate on why he choose that > > library > > >>> and > > >>> > what the potential impact is of adding it to our project. > > >>> > > > >>> > For question 5 and 6, I will also defer to Simon on this. The > focus > > of > > >>> > this feature as I understand it is a consistent authentication > > mechanism > > >>> > and support for SSO. I will let him lay out his vision for micro > > >>> services. > > >>> > > > >>> > Knox SSO would be a great improvement and is what I think we > should > > >>> focus > > >>> > on in this feature branch. Micro services is something we should > > >>> certainly > > >>> > discuss but it might be a bit of a distraction and I wouldn't want > > to > > >>> hold > > >>> > up the other useful parts. > > >>> > > > >>> > On Fri, Sep 14, 2018 at 8:38 PM Michael Miklavcic < > > >>> > michael.miklav...@gmail.com> wrote: > > >>> > > > >>> > > Hey all, > > >>> > > > > >>> > > I started looking through the Knox SSO feature branch (see here > > >>> > > https://issues.apache.org/jira/browse/METRON-1663). This is > some > > >>> great > > >>> > new > > >>> > > security functionality work and it looks like it will bring some > > >>> > important > > >>> > > new features to the Metron platform. I'm coming at this pretty > > green, > > >>> so > > >>> > I > > >>> > > do have some questions regarding the proposed changes from a > high > > >>> level > > >>> > > architectural perspective. There are a few changes within the > > current > > >>> FB > > >>> > > PR's that I think could use some further explanation. At first > > >>> glance, it > > >>> > > seems we could potentially simplify this branch a great deal and > > get > > >>> it > > >>> > > completed much sooner if we narrowed the focus a bit. But I > could > > >>> > certainly > > >>> > > be wrong here and happy for other opinions. I searched through > the > > >>> > mailing > > >>> > > list history to see if there is any additional background and > the > > main > > >>> > > DISCUSS thread I could find was regarding initially setting up > the > > >>> > feature > > >>> > > branch, which talked about adding Knox and LDAP. > > >>> > > > > >>> > > > > >>> > > > >>> > > > https://lists.apache.org/thread.html/cac2e6314284015b487121e77abf730abbb7ebec4ace014b19093b4c@%3Cdev.metron.apache.org%3E > > >>> > > . > > >>> > > If I've missed any follow-up, please let me know. > > >>> > > > > >>> > > Looking at the broader set of Jiras associated with 1663 and the > > >>> first PR > > >>> > > 1665, it looks like there are 4 main thrusts of this branch > right > > now: > > >>> > > > > >>> > > 1. Knox/SSO > > >>> > > 2. Node migrated to Spring Boot > > >>> > > 3. JDBC removed completely in favor of LDAP > > >>> > > 4. Introduction of Zuul, also microservices? > > >>> > > > > >>> > > I strongly urge for the purpose of reviewing this feature branch > > that > > >>> we > > >>> > > base much of the discussion off of > > >>> > > https://issues.apache.org/jira/browse/METRON-1755, the > > architecture > > >>> > > diagram. Minimally, an explanation of the current architecture > > along > > >>> with > > >>> > > discussion around the additional proposed changes and rationale > > would > > >>> be > > >>> > > useful for evaluation. I don't have a solid enough understanding > > yet > > >>> of > > >>> > the > > >>> > > full scope of changes and how they differ from the existing > > >>> architecture > > >>> > > just from looking at the PR's alone. > > >>> > > > > >>> > > 1. The first question is a general one regarding the necessity > of > > >>> the > > >>> > 3 > > >>> > > additional features alongside Knox - migrating Node to Spring > > Boot, > > >>> > > removing JDBC altogether, adding dependencies on Netflix's Zuul > > >>> > > framework. > > >>> > > Are these necessary for adding Knox/SSO? They seem like > > potentially > > >>> > > separate features, imo. > > >>> > > 2. It looks like LDAP will be a required component for > interacting > > >>> > with > > >>> > > Metron via the UI's. I see this PR > > >>> > > https://github.com/apache/metron/pull/1186 which removes JDBC > > >>> > > authentication. Are we ready to remove it completely or would it > > be > > >>> > > better > > >>> > > to leave it as a minimal installation option? What is the > proposed > > >>> > > migration path for existing users? Do we feel comfortable > > requiring > > >>> > that > > >>> > > all installations, including full dev, install and configure > LDAP? > > >>> For > > >>> > > comparison, in the PCAP feature branch we discussed removing the > > >>> > > existing > > >>> > > PCAP REST application in the initial discussion, got agreement, > > and > > >>> > > later > > >>> > > removed it in the course of working on the feature branch. The > PR > > >>> is > > >>> > > fairly > > >>> > > clear, however I think we're just missing some basic discussion > > >>> around > > >>> > > the > > >>> > > implications, as I've outlined above. Some additional relevant > > >>> > > discussion > > >>> > > occurred on this PR https://github.com/apache/metron/pull/1112 > > >>> which > > >>> > > would be good to summarize for purposes of this overarching > > >>> > architecture > > >>> > > discussion. > > >>> > > 3. Migration from Node to Spring Boot. I believe this is already > > >>> used > > >>> > by > > >>> > > the REST application and if anything brings some cohesion to our > > >>> > server > > >>> > > strategy. Strictly speaking, is there a reason this is required > > for > > >>> > > Knox? > > >>> > > It seems comparable to a component upgrade, such as moving from > ES > > >>> 2.x > > >>> > > to > > >>> > > 5.6.x and upgrading Angular 6. > > >>> > > 4. Introduction of Netflix's Zuul. > > >>> > > https://issues.apache.org/jira/browse/METRON-1665. > > >>> > > - > "The UIs currently proxy to the REST API to avoid CORS > > >>> issues, > > >>> > > this will be achieved with Zuul." > > >>> > > - Can we elaborate more on where or how CORS is a problem with > > >>> our > > >>> > > existing architecture, how Zuul will help solve that, and how it > > >>> > > fits with > > >>> > > Knox? Wouldn't this be handled by Knox? Since Larry McCay > > >>> chimed in > > >>> > > with > > >>> > > interest on the original SSO thread about the FB, I'm hoping he > > >>> is > > >>> > > also > > >>> > > willing to chime in on this as well. > > >>> > > - This looks like it has the potential to be a rather large > > >>> piece > > >>> > of > > >>> > > fundamental infrastructure (as it's also pertinent to > > >>> > microservices) > > >>> > > to > > >>> > > pull into the platform, and I'd like to be sure the community is > > >>> > > aware of > > >>> > > and is OK with the implications. > > >>> > > 5. > "The proposal is to use a spring boot application, allowing > > >>> us to > > >>> > > harmonize the security implementation across the UI static > servers > > >>> and > > >>> > > the > > >>> > > REST layer, and to provide a routing platform for later > > >>> > microservices." > > >>> > > - > > >>> > > https://issues.apache.org/jira/browse/METRON-1665. > > >>> > > - Microservices is a pretty loaded term. I know there had been > > >>> some > > >>> > > discussion a while back during the PCAP feature branch start, > > >>> but I > > >>> > > don't > > >>> > > recall ever reaching a consensus on it. More detail in this > > >>> thread > > >>> > - > > >>> > > > > >>> > > > > >>> > > > >>> > > > https://lists.apache.org/thread.html/1db7c6fa1b0f364f8c03520db9989b4f7a446de82eb4d9786055048c@%3Cdev.metron.apache.org%3E > > >>> > > . > > >>> > > Can we get some clarification on what is meant by microservices > > >>> > > in the case > > >>> > > of this FB and relevant PR's, what that architecture looks like, > > >>> > and > > >>> > > how > > >>> > > it's achieved with the proposed changes in this PR/FB? It seems > > >>> > Zuul > > >>> > > is > > >>> > > also pertinent to this discussion, but there are many ways to > > >>> > > skin this cat > > >>> > > so I don't want to presume - > > >>> > > > > >>> > > > > >>> > > https://blog.heroku.com/using_netflix_zuul_to_proxy_your_microservices > > >>> > > 6. Zuul, Spring Boot, and microservices - Closely related to > > >>> > point 5 > > >>> > > above. It seems that we weren't quite ready for this when it was > > >>> > > brought up > > >>> > > in May, or at the very least we had some concern of what > direction > > >>> to > > >>> > > go. > > >>> > > What is the operational impact, mpack impact, and how we propose > > to > > >>> > > manage > > >>> > > it with Kerberos, etc.? > > >>> > > > > >>> > > > > >>> > > > >>> > > > https://lists.apache.org/thread.html/c19904681e6a6d9ea3131be3d1a65b24447dca31b4aff588b263fd87@%3Cdev.metron.apache.org%3E > > >>> > > > > >>> > > There is a lot to like in this feature branch, imo. Great > feature > > >>> > addition > > >>> > > with Knox and SSO. Introduction of LDAP support for > > authentication for > > >>> > > Metron UI's. Simplification/unification of our server hosting > > >>> > > infrastructure. I'm hoping we can flesh out some of the details > > >>> pointed > > >>> > out > > >>> > > above a bit more and get this feature through. Great work so > far! > > >>> > > > > >>> > > Best, > > >>> > > Mike Miklavcic > > >>> > > > > >>> > > > >> > > >> -- > > >> -- > > >> simon elliston ball > > >> @sireb > > > > > > -- > > > -- > > > simon elliston ball > > > @sireb > > > > ------------------- > > Thank you, > > > > James Sirota > > PMC- Apache Metron > > jsirota AT apache DOT org > > > > >
