Github user markap14 commented on the issue:
https://github.com/apache/nifi/pull/2335
@ijokarumawak thanks for all of the work that you've put into this - it is
very much a non-trivial effort! For the most part, the code looks good. I
flagged a couple of minor things in the code, 1 or 2 thread-safety issues that
should be easy to address.
The only 'more significant' concern that I have is the use of the
dummied-up NiFiUser. As-is, this is an anonymous user and in a secured
environment will not retrieve the event details that are necessary. It also
means that we would be validating events against a user who doesn't even exist.
I think there are 2 ways to approach this: first, as I noted inline, we
could have a property to define which user the queries should run on behalf of.
So the user could add a "NiFi Atlas" user and use that. However, that's also a
bit concerning because it means that whoever has access to edit the reporting
task can run provenance queries on behalf of another user.
By far, my preference is to actually just update the ProvenanceRepository
implementations (There are 4 now, I think) so that if a null User is passed in,
we don't check permissions. This would mean that you can pass in null from
Reporting Task. We could also then update the interface to have an overloaded
method that does not require that a user be given.
Once that is addressed, I think it is a +1 from me from a code review
perspective.
Thanks
-Mark
---
