[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/17125 )
Change subject: IMPALA-10550: Add External Frontend service port ...................................................................... Patch Set 5: > (2 comments) > > Are there any existing hs2 methods that it might make sense to > block for the "external frontend" server? eg. we might want to > return an error for ExecuteStatement() from it if the external > frontend will never need to call it. > > That would give us a little more safety in case people accidentally > expose this port to the outside world (of course, it would still be > possible for bad actors to use the ExecutePlannedStatement > interface, but its probably a lot harder to put together a valid > TExecRequest to use it than it is to put together a SQL string like > ExecuteStatement takes) > > Also out of curiosity - what's the long run testing plan here? Are > we going to have an actual external FE running in the minicluster > that can exercise this stuff? So the current implementation of external frontend does utilize the ExecuteStatement functionality (for things like COMPUTE STATS). I do agree with your assessment that it would be nice to reduce the surface area in the future. The long term plan would also likely include enabling similar protections that intra-impalad communication use between nodes (that prevent people connecting easily to the backend port and pretending to be a coordinator). One option I considered based on your comment was to add a 2nd flag that would be named something like: external_fe_allow_unsafe which defaulted to false and disallowed ExecuteStatement via the external_fe_port. So a user would have to enable external_fe_port AND set external_fe_allow_unsafe to true to be able to call ExecuteStatement. But if someone is enabling the external_fe_port - it is somewhat assumed they know what they are doing so I'm not 100% convinced this approach is worth it. I am open to suggestions (or if you like the idea of the 2nd flag). As for testing - I do believe once the various external FE commits land we should focus on: 1) auditing and shoring up what we can build unit tests around 2) And, yes, it is my understanding that we will eventually be including an external frontend in the minicluster for more end-to-end testing. Otherwise, we will need to mock up some sort of "send pre-made exec request" and "check response" test framework but I suspect that might be not fun to implement cleanly. -- To view, visit http://gerrit.cloudera.org:8080/17125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I991b5b05e12e37d8739e18ed1086bbb0228acc40 Gerrit-Change-Number: 17125 Gerrit-PatchSet: 5 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Comment-Date: Sat, 27 Feb 2021 01:00:08 +0000 Gerrit-HasComments: No
