NIFI-3785 Added feature to move a controller service to it's parent o…

[Freedom9339]

…r child process group

Summary

NIFI-3785 - Added an option on the controller services grid that moves a controller service to a specified process group. The controller service can be moved to the parent process group or a child process group. However, it can only be moved one level at a time. If the move is to a child process group, any processor that references the controller service that is outside the scope of the new process group will have the reference removed. Likewise, if the move is to a parent node, any controller service that is referenced by the controller service being moved and that is outside of the new scope will have that referenced removed.

Tracking

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using mvn clean install -P contrib-check
  • JDK 17

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[markobean]

I had trouble building, but once I rebased to main the issues resolved. Please rebase.

I installed NiFi and played with this feature. It worked as expected moving to parent, to a child process group where referencing components exist, to a child process group where referencing components do not exist (and references to the controller service were then removed automatically). All worked great.

The UX is intuitive and provides sufficient warning to the user that referencing components may be adversely affected by the controller service move.

I confirmed the API documentation contains the new move endpoint.

Full disclosure: I tested only using single-user-authorizer, so I have not yet tested possible authorization issues such as attempting to move the controller service to a process group where the user does not have modify permission.

See the couple comments on the code itself, but generally this looks great. Thanks @Freedom9339. This is a great feature which has been a long time coming!

[Freedom9339]

@markap14 Would mind taking a look at this PR? I do see you were the one who created the ticket.

[markap14]

Oh nice, thanks @Freedom9339 ! This is something that I've wanted for a while. I would definitely want to get a reviewer who knows the UI as well such as @mcgilman or @scottyaslan. I should have a chance to review the backend soon, probably some time this week. But it looks like the system tests failed and some of the unit tests failed as well. Unfortunately, it looks like the logs have already aged off. I'll see if I can reproduce the failures locally, but we'll definitely want to ensure that we get those cleaned up

[markap14]

@Freedom9339 you mind rebasing against main and pushing again? That should kick off the tests against the latest & greatest. I misread the github comments initially, thinking you pushed a rebased commit just a couple days ago. That explains why I can't kick off the build again

[Freedom9339]

@markap14 I've rebased against main and pushed. Thank You!

[markap14]

Hey @Freedom9339 thanks for putting this together! Sorry it took so long for me to get back around to it. There are a few minor tweaks requested inline, but I have one big concern, which is that moving the Controller Service should never remove references to the service. Rather, if a reference would be invalid, the Controller Service cannot be moved, and we should report back an error indicating why.

[Freedom9339]

@markap14 Thank you for the feedback. I've addressed all the changes and concerns provided. The process will now fail if there is a referencing component scope conflict and an error prompt will be shown listing the components that have a scope conflict.

[Freedom9339]

@markap14 Good Morning, Have you had a chance to look at the changes?

[exceptionfactory]

Thanks for your work on this improvement @Freedom9339. With recent changes the nifi-web-ui module has been removed in favor of the restructured nifi-web-frontend module. Given that the user interface implementation has changed significantly, I am closing this pull request for now. That is not to say that this feature may not be useful, it just means that it will need to be refactored from the ground up for the new user interface. If you would like to consider that improvement, feel free to open a new PR based on the latest version of the user interface.