From 31a70f48b771f61777305315b1752c38d3342372 Mon Sep 17 00:00:00 2001 From: Freedom9339 Date: Fri, 28 Feb 2025 16:18:22 +0000 Subject: [PATCH] NIFI-3785: Moved move logic to ServiceFacade. Fixed move api cluster replication. Fixed error handling and made error message be a banner instead of a toast. --- .../apache/nifi/web/NiFiServiceFacade.java | 28 +++++++ .../nifi/web/StandardNiFiServiceFacade.java | 84 +++++++++++++++++++ .../web/api/ControllerServiceResource.java | 81 +++--------------- .../controller-services.effects.ts | 28 +++++-- .../controller-services.reducer.ts | 17 ++-- .../move-controller-service.component.html | 13 ++- .../move-controller-service.component.scss | 5 ++ .../move-controller-service.component.ts | 9 +- 8 files changed, 173 insertions(+), 92 deletions(-) diff --git a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java index 77ae876e1731..79c7bec88875 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java +++ b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiServiceFacade.java @@ -18,6 +18,7 @@ import io.prometheus.client.CollectorRegistry; import org.apache.nifi.authorization.AuthorizeAccess; +import org.apache.nifi.authorization.ProcessGroupAuthorizable; import org.apache.nifi.authorization.RequestAction; import org.apache.nifi.authorization.user.NiFiUser; import org.apache.nifi.bundle.BundleCoordinate; @@ -134,6 +135,7 @@ import org.apache.nifi.web.api.entity.PortStatusEntity; import org.apache.nifi.web.api.entity.ProcessGroupEntity; import org.apache.nifi.web.api.entity.ProcessGroupFlowEntity; +import org.apache.nifi.web.api.entity.ProcessGroupOptionEntity; import org.apache.nifi.web.api.entity.ProcessGroupRecursivity; import org.apache.nifi.web.api.entity.ProcessGroupStatusEntity; import org.apache.nifi.web.api.entity.ProcessorDiagnosticsEntity; @@ -2205,6 +2207,32 @@ ControllerServiceReferencingComponentsEntity updateControllerServiceReferencingC */ ControllerServiceEntity moveControllerService(final Revision revision, final ControllerServiceDTO controllerServiceDTO, final String newProcessGroupID); + /** + * Gets all available process group move options + * + * @param controllerServiceId id + * @return A list of all process group move options + */ + List getAllProcessGroupOptions(String controllerServiceId); + + /** + * Generated a process group move option + * + * @param controllerServiceDTO The controller service DTO + * @param processGroup The authorizable process group + * @return A process group move option + */ + ProcessGroupOptionEntity generateProcessGroupOption(ControllerServiceDTO controllerServiceDTO, ProcessGroupAuthorizable processGroup); + + /** + * Gets all conflicting components preventing a move operation + * + * @param controllerServiceDTO The controller service DTO + * @param processGroup The authorizable process group + * @return A list of all conflicting components + */ + List getConflictingComponents(ControllerServiceDTO controllerServiceDTO, ProcessGroupAuthorizable processGroup); + /** * Performs verification of the given Configuration for the Controller Service with the given ID * @param controllerServiceId the id of the controller service diff --git a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java index 78fa4b503754..67a24e9bc317 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java +++ b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java @@ -37,7 +37,9 @@ import org.apache.nifi.authorization.AuthorizationResult.Result; import org.apache.nifi.authorization.AuthorizeAccess; import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.ComponentAuthorizable; import org.apache.nifi.authorization.Group; +import org.apache.nifi.authorization.ProcessGroupAuthorizable; import org.apache.nifi.authorization.RequestAction; import org.apache.nifi.authorization.Resource; import org.apache.nifi.authorization.User; @@ -360,6 +362,7 @@ import org.apache.nifi.web.api.entity.PortStatusEntity; import org.apache.nifi.web.api.entity.ProcessGroupEntity; import org.apache.nifi.web.api.entity.ProcessGroupFlowEntity; +import org.apache.nifi.web.api.entity.ProcessGroupOptionEntity; import org.apache.nifi.web.api.entity.ProcessGroupRecursivity; import org.apache.nifi.web.api.entity.ProcessGroupStatusEntity; import org.apache.nifi.web.api.entity.ProcessGroupStatusSnapshotEntity; @@ -3002,6 +3005,87 @@ public ControllerServiceEntity moveControllerService(final Revision revision, fi return entityFactory.createControllerServiceEntity(snapshot.getComponent(), dtoFactory.createRevisionDTO(snapshot.getLastModification()), permissions, operatePermissions, bulletinEntities); } + @Override + public List getAllProcessGroupOptions(String controllerServiceId) { + final NiFiUser user = NiFiUserUtils.getNiFiUser(); + List options = new ArrayList<>(); + final ControllerServiceDTO controllerServiceDTO = getControllerService(controllerServiceId, true).getComponent(); + ProcessGroupEntity currentProcessGroup = getProcessGroup(controllerServiceDTO.getParentGroupId()); + + if (currentProcessGroup.getComponent().getParentGroupId() != null) { + final ProcessGroupAuthorizable authorizableProcessGroupParent = authorizableLookup.getProcessGroup(currentProcessGroup.getComponent().getParentGroupId()); + if (authorizableProcessGroupParent.getAuthorizable().isAuthorized(authorizer, RequestAction.READ, user) + && authorizableProcessGroupParent.getAuthorizable().isAuthorized(authorizer, RequestAction.WRITE, user)) { + ProcessGroupOptionEntity option = generateProcessGroupOption(controllerServiceDTO, authorizableProcessGroupParent); + option.setText(option.getText() + " (Parent)"); + options.add(option); + } + } + + currentProcessGroup.getComponent().getContents().getProcessGroups().forEach(processGroup -> { + final ProcessGroupAuthorizable authorizableProcessGroup = authorizableLookup.getProcessGroup(processGroup.getId()); + if (authorizableProcessGroup.getAuthorizable().isAuthorized(authorizer, RequestAction.READ, user) + && authorizableProcessGroup.getAuthorizable().isAuthorized(authorizer, RequestAction.WRITE, user)) { + options.add(generateProcessGroupOption(controllerServiceDTO, authorizableProcessGroup)); + } + }); + + return options; + } + + @Override + public ProcessGroupOptionEntity generateProcessGroupOption(ControllerServiceDTO controllerServiceDTO, ProcessGroupAuthorizable processGroup) { + List conflictingComponents = getConflictingComponents(controllerServiceDTO, processGroup); + + ProcessGroupOptionEntity option = new ProcessGroupOptionEntity(); + option.setText(processGroup.getProcessGroup().getName()); + option.setValue(processGroup.getProcessGroup().getIdentifier()); + option.setDisabled(false); + + if (!conflictingComponents.isEmpty()) { + String errorMessage = "Cannot move to this process group because the following components would be out of scope: "; + errorMessage += String.join(" ", conflictingComponents); + option.setDescription(errorMessage); + option.setDisabled(true); + } + + return option; + } + + @Override + public List getConflictingComponents(ControllerServiceDTO controllerServiceDTO, ProcessGroupAuthorizable processGroup) { + final NiFiUser user = NiFiUserUtils.getNiFiUser(); + List conflictingComponents = new ArrayList<>(); + controllerServiceDTO.getReferencingComponents().forEach(referencingComponent -> { + if (processGroup.getProcessGroup().findProcessor(referencingComponent.getId()) == null + && processGroup.getProcessGroup().findControllerService(referencingComponent.getId(), true, false) == null) { + final Authorizable componentAuthorizable = authorizableLookup.getControllerServiceReferencingComponent(controllerServiceDTO.getId(), referencingComponent.getId()); + if (componentAuthorizable.isAuthorized(authorizer, RequestAction.READ, user)) { + conflictingComponents.add("[" + referencingComponent.getComponent().getName() + "]"); + } else { + conflictingComponents.add("[" + referencingComponent.getId() + "]"); + } + } + }); + + controllerServiceDTO.getProperties().forEach((key, value) -> { + try { + ControllerServiceEntity refControllerService = this.getControllerService(value, false); + if (refControllerService != null) { + if (processGroup.getProcessGroup().findControllerService(value, false, true) == null) { + ComponentAuthorizable componentAuthorizable = authorizableLookup.getControllerService(value); + if (componentAuthorizable.getAuthorizable().isAuthorized(authorizer, RequestAction.READ, user)) { + conflictingComponents.add("[" + refControllerService.getComponent().getName() + "]"); + } else { + conflictingComponents.add("[" + refControllerService.getId() + "]"); + } + } + } + } catch (Exception ignored) { } + }); + return conflictingComponents; + } + @Override public List performControllerServiceConfigVerification(final String controllerServiceId, final Map properties, final Map variables) { return controllerServiceDAO.verifyConfiguration(controllerServiceId, properties, variables); diff --git a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerServiceResource.java b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerServiceResource.java index c6ae6a094719..c0b2f968f7f5 100644 --- a/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerServiceResource.java +++ b/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerServiceResource.java @@ -39,7 +39,6 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import org.apache.commons.lang3.StringUtils; -import org.apache.nifi.authorization.AuthorizableLookup; import org.apache.nifi.authorization.AuthorizeControllerServiceReference; import org.apache.nifi.authorization.AuthorizeParameterReference; import org.apache.nifi.authorization.Authorizer; @@ -88,13 +87,13 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Controller; -import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -694,7 +693,7 @@ public Response getProcessGroupOptions( throw new IllegalArgumentException("Controller service id must be specified."); } - List options = new ArrayList<>(); + AtomicReference> options = new AtomicReference<>(); serviceFacade.authorizeAccess(lookup -> { final NiFiUser user = NiFiUserUtils.getNiFiUser(); @@ -707,75 +706,11 @@ public Response getProcessGroupOptions( authorized = authorized && authorizableProcessGroupCurrent.getAuthorizable().isAuthorized(authorizer, RequestAction.WRITE, user); if (authorized) { - if (authorizableProcessGroupCurrent.getProcessGroup().getParent() != null) { - final ProcessGroupAuthorizable authorizableProcessGroupParent = lookup.getProcessGroup(authorizableProcessGroupCurrent.getProcessGroup().getParent().getIdentifier()); - if (authorizableProcessGroupParent.getAuthorizable().isAuthorized(authorizer, RequestAction.READ, user) - && authorizableProcessGroupParent.getAuthorizable().isAuthorized(authorizer, RequestAction.WRITE, user)) { - options.add(generateProcessGroupOption(controllerServiceDTO, authorizableProcessGroupParent, lookup, user)); - } - } - - authorizableProcessGroupCurrent.getProcessGroup().getProcessGroups().forEach(processGroup -> { - final ProcessGroupAuthorizable authorizableProcessGroup = lookup.getProcessGroup(processGroup.getIdentifier()); - if (authorizableProcessGroup.getAuthorizable().isAuthorized(authorizer, RequestAction.READ, user) - && authorizableProcessGroup.getAuthorizable().isAuthorized(authorizer, RequestAction.WRITE, user)) { - options.add(generateProcessGroupOption(controllerServiceDTO, authorizableProcessGroup, lookup, user)); - } - }); - } - }); - - return generateOkResponse(options).build(); - } - - private ProcessGroupOptionEntity generateProcessGroupOption(ControllerServiceDTO controllerServiceDTO, ProcessGroupAuthorizable processGroup, AuthorizableLookup lookup, NiFiUser user) { - List conflictingComponents = getConflictingComponents(controllerServiceDTO, processGroup, lookup, user); - - ProcessGroupOptionEntity option = new ProcessGroupOptionEntity(); - option.setText(processGroup.getProcessGroup().getName()); - option.setValue(processGroup.getProcessGroup().getIdentifier()); - option.setDisabled(false); - - if (!conflictingComponents.isEmpty()) { - String errorMessage = "Cannot move to this process group because the following components would be out of scope: "; - errorMessage += String.join(" ", conflictingComponents); - option.setDescription(errorMessage); - option.setDisabled(true); - } - - return option; - } - - private List getConflictingComponents(ControllerServiceDTO controllerServiceDTO, ProcessGroupAuthorizable processGroup, AuthorizableLookup lookup, NiFiUser user) { - List conflictingComponents = new ArrayList<>(); - controllerServiceDTO.getReferencingComponents().forEach(referencingComponent -> { - if (processGroup.getProcessGroup().findProcessor(referencingComponent.getId()) == null - && processGroup.getProcessGroup().findControllerService(referencingComponent.getId(), true, false) == null) { - final Authorizable componentAuthorizable = lookup.getControllerServiceReferencingComponent(controllerServiceDTO.getId(), referencingComponent.getId()); - if (componentAuthorizable.isAuthorized(authorizer, RequestAction.READ, user)) { - conflictingComponents.add("[" + referencingComponent.getComponent().getName() + "]"); - } else { - conflictingComponents.add("[Unauthorized Component]"); - } + options.set(serviceFacade.getAllProcessGroupOptions(controllerServiceId)); } }); - controllerServiceDTO.getProperties().forEach((key, value) -> { - try { - ControllerServiceEntity refControllerService = serviceFacade.getControllerService(value, false); - if (refControllerService != null) { - if (processGroup.getProcessGroup().findControllerService(value, false, true) == null) { - ComponentAuthorizable componentAuthorizable = lookup.getControllerService(value); - if (componentAuthorizable.getAuthorizable().isAuthorized(authorizer, RequestAction.READ, user)) { - conflictingComponents.add("[" + refControllerService.getComponent().getName() + "]"); - } else { - conflictingComponents.add("[Unauthorized Component]"); - } - } - } - } catch (Exception ignored) { } - }); - return conflictingComponents; + return generateOkResponse(options.get()).build(); } /** @@ -829,6 +764,12 @@ public Response moveControllerServices( throw new IllegalArgumentException("ParentGroupId must be specified."); } + if (isReplicateRequest()) { + return replicate(HttpMethod.PUT, requestControllerServiceEntity); + } else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestControllerServiceEntity.isDisconnectedNodeAcknowledged()); + } + final Revision requestRevision = getRevision(requestControllerServiceEntity, id); return withWriteLock( serviceFacade, @@ -850,7 +791,7 @@ public Response moveControllerServices( authorizableProcessGroupOld.getAuthorizable().authorize(authorizer, RequestAction.WRITE, user); // Verify all referencing and referenced components are still within scope - List conflictingComponents = getConflictingComponents(requestControllerServiceDTO, authorizableProcessGroupNew, lookup, user); + List conflictingComponents = serviceFacade.getConflictingComponents(requestControllerServiceDTO, authorizableProcessGroupNew); if (!conflictingComponents.isEmpty()) { String errorMessage = "Could not move controller service because the following components would be out of scope: "; diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.effects.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.effects.ts index 334c15fe3fa5..97c39bbfaf2f 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.effects.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.effects.ts @@ -694,11 +694,15 @@ export class ControllerServicesEffects { } }); }), - catchError((errorResponse: HttpErrorResponse) => { - this.dialog.closeAll(); - return of( - ErrorActions.snackBarError({ error: this.errorHelper.getErrorString(errorResponse) }) - ); + tap({ + error: (errorResponse: HttpErrorResponse) => { + this.dialog.closeAll(); + this.store.dispatch( + ErrorActions.snackBarError({ + error: this.errorHelper.getErrorString(errorResponse) + }) + ); + } }) ) ) @@ -719,9 +723,17 @@ export class ControllerServicesEffects { } }) ), - catchError((errorResponse: HttpErrorResponse) => - of(ErrorActions.snackBarError({ error: this.errorHelper.getErrorString(errorResponse) })) - ) + catchError((errorResponse: HttpErrorResponse) => { + if (this.errorHelper.showErrorInContext(errorResponse.status)) { + return of( + ControllerServicesActions.controllerServicesBannerApiError({ + error: this.errorHelper.getErrorString(errorResponse) + }) + ); + } else { + return of(this.errorHelper.fullScreenError(errorResponse)); + } + }) ) ) ) diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.reducer.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.reducer.ts index 9fb773126658..0790e195a10e 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.reducer.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/controller-services/controller-services.reducer.ts @@ -28,7 +28,8 @@ import { inlineCreateControllerServiceSuccess, loadControllerServices, loadControllerServicesSuccess, - resetControllerServicesState + resetControllerServicesState, + moveControllerService } from './controller-services.actions'; import { produce } from 'immer'; import { ControllerServicesState } from './index'; @@ -76,10 +77,16 @@ export const controllerServicesReducer = createReducer( ...state, saving: false })), - on(createControllerService, configureControllerService, deleteControllerService, (state) => ({ - ...state, - saving: true - })), + on( + createControllerService, + configureControllerService, + deleteControllerService, + moveControllerService, + (state) => ({ + ...state, + saving: true + }) + ), on(createControllerServiceSuccess, (state, { response }) => { return produce(state, (draftState) => { draftState.controllerServices.push(response.controllerService); diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.html b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.html index 6408802b67eb..cfe01a6348c9 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.html +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.html @@ -17,6 +17,7 @@

Move Controller Service

+
@@ -38,17 +39,13 @@

Move Controller Service

[tooltipInputData]="option.description" [delayClose]="false"> -
- -
- - {{ option.text }}
+ } diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.scss b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.scss index f538913d1df2..df2813b93e5b 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.scss +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.scss @@ -24,3 +24,8 @@ width: 100%; } } + +mat-divider { + margin-top: 8px; + margin-bottom: 8px; +} diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.ts index 8b073157db03..7783ac0fda2f 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/controller-service/move-controller-service/move-controller-service.component.ts @@ -32,11 +32,14 @@ import { TextTip, NifiTooltipDirective, SelectOption } from '@nifi/shared'; import { Store } from '@ngrx/store'; import { CloseOnEscapeDialog } from '@nifi/shared'; import { NiFiState } from 'apps/nifi/src/app/state'; -import { NgIf } from '@angular/common'; import { ControllerServiceEntity, ControllerServiceReferencingComponent } from 'apps/nifi/src/app/state/shared'; import { ControllerServiceReferences } from 'apps/nifi/src/app/ui/common/controller-service/controller-service-references/controller-service-references.component'; import { MoveControllerServiceDialogRequestSuccess } from '../../../state/controller-services'; import { moveControllerService } from '../../../state/controller-services/controller-services.actions'; +import { MatDivider } from '@angular/material/divider'; +import { ContextErrorBanner } from '../../../../../ui/common/context-error-banner/context-error-banner.component'; +import { ErrorContextKey } from '../../../../../state/error'; +import { NgIf } from '@angular/common'; @Component({ selector: 'move-controller-service', @@ -55,6 +58,8 @@ import { moveControllerService } from '../../../state/controller-services/contro MatDialogActions, MatButton, MatDialogClose, + ContextErrorBanner, + MatDivider, NgIf ], styleUrls: ['./move-controller-service.component.scss'] @@ -112,4 +117,6 @@ export class MoveControllerService extends CloseOnEscapeDialog { override isDirty(): boolean { return this.moveControllerServiceForm.dirty; } + + protected readonly ErrorContextKey = ErrorContextKey; }