Good point, since registry client is an extension point, using the rules on NiFi side allows it to work for any registry client.
On Wed, Jan 17, 2024 at 11:49 AM Pierre Villard <[email protected]> wrote: > > Yeah so the more I think about it the more we should probably couple this > with the new rules engine feature of NiFi 2.0. Users could define their > rules there and then it could just be a property in NiFi saying whether or > not it should prevent someone from committing a new version if one of the > required rules is raising a violation in the process group being committed. > This way this would work with any registry implementation, not just the > NiFi Registry (I'm saying this in case we ever get to a pure Git-based > implementation of the Registry endpoint). > > Le mer. 17 janv. 2024 à 20:33, Bryan Bende <[email protected]> a écrit : > > > Michael, > > > > I think this validation idea came from the absence of a pull request > > review model. Meaning, the rules being discussed here would check for > > the things someone would most likely look for in a review of the flow > > changes to decide if the flow is good. Obviously it wouldn't be able > > to do everything a person would, but it could be better than nothing. > > As far as I know, there isn't any active work around building a review > > model. > > > > -Bryan > > > > On Wed, Jan 17, 2024 at 11:18 AM Michael Hogue > > <[email protected]> wrote: > > > > > > Could this be accomplished through git webhooks once changes are > > committed > > > through Registry and CI pipelines to perform whatever validation you > > wish? > > > > > > The glaring weakness in this approach is that the changes have already > > been > > > committed. This makes me wonder if we can improve the "flow development > > > lifecycle" to enable teams to review flow changes before they're > > > committed, much like a pull request. Is this on the roadmap for Registry? > > > > > > Mike > > > > > > On Wed, Jan 17, 2024 at 4:14 PM Bryan Bende <[email protected]> wrote: > > > > > > > Thanks guys, that makes sense in the context of the review model. > > > > > > > > Obviously some kind of general pre-event hook is the most flexible, > > > > but I would say we should also consider whether we really want to be > > > > calling out to arbitrary scripts during the main API requests, as well > > > > as consider what someone would have to do to implement a scripted > > > > rule. The current events are all metadata related, they don't have the > > > > content of the snapshots, so in this case we'd have to pass the entire > > > > json string of a snapshot as an arg to the script and then the script > > > > has to parse through it looking for fields to validate/check. I wonder > > > > if we should consider defining the types of rules ourselves and > > > > allowing some kind of rules config file to be provided. This way the > > > > rules are loaded during start up and applied in Java code by our > > > > framework code. Downside is it is less flexible in that we probably > > > > won't be able to know every rule someone wants ahead of time. I guess > > > > the other options are to consider whether tagging or nifi side rules > > > > are better suited to solve this problem. > > > > > > > > On Wed, Jan 17, 2024 at 10:48 AM Simon Bence <[email protected] > > > > > > > wrote: > > > > > > > > > > HI, > > > > > > > > > > I think, both of you are correct, my initial example of the possible > > > > usage was not the best. Having a control over committing versions for > > flows > > > > is maybe the most important benefit we can gain. Contrary to naming > > > > conventions this can be a more complex endeavour. So I suggest moving > > on > > > > with this example instead of the original one. > > > > > > > > > > Regards, > > > > > Bence > > > > > > > > > > > On 2024. Jan 17., at 16:02, Pierre Villard < > > > > [email protected]> wrote: > > > > > > > > > > > > I guess the issue relates to some recurrent discussions around the > > > > lack of > > > > > > mechanism for a Pull Request kind of model for "approving" a > > versioned > > > > > > flow. An alternative we've been discussing for a long time is the > > > > ability > > > > > > to set tags to versions so that a versioned flow could be reviewed > > > > (diff > > > > > > with previous version(s)) and if approved a tag would be applied > > and > > > > this > > > > > > tag would be used to potentially trigger an automatic deployment. > > > > > > > > > > > > The proposal is not helping with the "PR model" but would allow > > > > someone to > > > > > > control what is versioned. A user may have the permissions to > > commit a > > > > new > > > > > > version but one may not want to accept a flow where a processor is > > > > > > configured with 50 concurrent tasks (for example). That would get > > > > deployed > > > > > > in prod and potentially impact existing deployed flows. I guess one > > > > could > > > > > > see this approach as a "checkstyle" plugin to allow or not the > > commit > > > > of a > > > > > > new version. > > > > > > > > > > > > Le mer. 17 janv. 2024 à 18:56, Bryan Bende <[email protected]> a > > écrit > > > > : > > > > > > > > > > > >> I think before we embed something into synchronous API requests we > > > > > >> should try to define a few more real uses besides just a bucket > > naming > > > > > >> pattern to see if we really need to add something like this. For > > > > > >> bucket naming pattern, it could just be a simple property in > > > > > >> nifi-registry.properties with a regex to check names against. If > > we > > > > > >> did add some kind of pluggable validator, I don't think these > > > > > >> validators should be making authorization decisions, it should be > > > > > >> strictly for rules about the content being version controlled. If > > we > > > > > >> are worried about a user version controlling something that > > interferes > > > > > >> with a CI/CD pipeline, that seems to imply our current > > authorization > > > > > >> policy model needs improvement, or it is not being used correctly > > > > > >> (i.e. why does a user have write to a bucket they shouldn't be > > writing > > > > > >> to?). > > > > > >> > > > > > >> On Wed, Jan 17, 2024 at 8:53 AM Pierre Villard > > > > > >> <[email protected]> wrote: > > > > > >>> > > > > > >>> I do think this would be a good idea. That would provide users > > of the > > > > > >> NiFi > > > > > >>> Registry a way to implement custom conditions on what can be > > > > versioned or > > > > > >>> not based on their requirements. Right now if one has write > > > > permissions > > > > > >> for > > > > > >>> a bucket/flow, you could commit anything as a new version which > > could > > > > > >>> potentially be an issue when using CI/CD pipelines to automate > > > > > >> deployments. > > > > > >>> > > > > > >>> As a side-note, another improvement could be to prevent a flow > > from > > > > being > > > > > >>> versioned in the registry if a required rule is violated (I'm > > talking > > > > > >> about > > > > > >>> the new feature coming with NiFi 2.0). Just a thought for an > > > > additional > > > > > >>> improvement. > > > > > >>> > > > > > >>> Overall I do think that giving options to users for better > > > > controlling > > > > > >> what > > > > > >>> is being versioned is good. > > > > > >>> > > > > > >>> Le mer. 17 janv. 2024 à 17:13, Simon Bence < > > [email protected]> > > > > a > > > > > >>> écrit : > > > > > >>> > > > > > >>>> Hi, > > > > > >>>> > > > > > >>>> I recently found the need for custom validation to maintain NiFi > > > > > >> Registry > > > > > >>>> content. This includes checks such as enforcing naming > > conventions > > > > when > > > > > >>>> creating a Bucket and similar usage specific cases. While > > exploring > > > > the > > > > > >>>> Registry's codebase, I came across the EventHookProvider, which > > > > aligns > > > > > >> with > > > > > >>>> a similar concept. However, it does not cover the case here due > > to > > > > its > > > > > >>>> asynchronous nature and being a "post-event" activity. > > > > > >>>> > > > > > >>>> Although the EventHookProvider is not suitable for this specific > > > > need, > > > > > >> I > > > > > >>>> find the Event construct and the "whitelist" concept pretty > > > > overlapping > > > > > >>>> with my objectives. Consequently, I propose the addition of a > > new > > > > type > > > > > >> of > > > > > >>>> Provider covering for "pre-event" validation, operating in a > > manner > > > > > >> similar > > > > > >>>> to the EventHookProvider: a call from the request methods to the > > > > set of > > > > > >>>> providers but filtered using a whitelist. Similarly to the > > mentioned > > > > > >>>> provider, I believe an implementation capable of executing > > scripts > > > > > >> (akin to > > > > > >>>> ScriptEventHookProvider) would be a good starting point, to > > cover a > > > > > >> most > > > > > >>>> use cases. > > > > > >>>> > > > > > >>>> I am keen to hear your opinion on this proposal and welcome any > > > > further > > > > > >>>> ideas. Thank you for your consideration! > > > > > >>>> > > > > > >>>> PS.: using the "event" term comes from the already existing > > > > > >>>> EventHookProvider. In practice these are the methods of the > > Registry > > > > > >> web > > > > > >>>> API. > > > > > >>>> > > > > > >>>> Regards, > > > > > >>>> Bence Simon > > > > > >> > > > > > > > > > > >
