[GitHub] nifi pull request #3102: NIFI-5737: Removing need client auth property as cl...
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3102 NIFI-5737: Removing need client auth property as cluster communications no longer support it NIFI-5737: - Removing needClientAuth property since cluster comms now requires two way ssl. Jetty client auth settings are based on configured features. - Removing dead code. - Updating documentation. - Removing references to needClientAuth property in all test resources. - Removing overloaded util method with strict parameter. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-5737 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3102.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3102 commit b10403bc447ef47fc31dc5c3e146b4e8e87257e2 Author: Matt Gilman Date: 2018-10-22T18:58:43Z NIFI-5737: - Removing needClientAuth property since cluster comms now requires two way ssl. Jetty client auth settings are based on configured features. - Removing dead code. - Updating documentation. - Removing references to needClientAuth property in all test resources. - Removing overloaded util method with strict parameter. ---
[GitHub] nifi issue #3076: NIFI-5705: Added Hive 3 attribution to nifi-assembly and t...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3076 Thanks @mattyb149! This has been merged to master. ---
[GitHub] nifi pull request #3087: NIFI-5715: Updating allow values of runStatus for s...
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3087 NIFI-5715: Updating allow values of runStatus for swagger spec NIFI-5715: - Updating the allowable values for the runStatus. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-5715 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3087.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3087 commit 76e41bb75e62fc1efe174e406a2f8691781d36b6 Author: Matt Gilman Date: 2018-10-17T14:18:18Z NIFI-5715: - Updating the allowable values for the runStatus. ---
[GitHub] nifi issue #3083: NIFI-5709, NIFI-5710: Addressed issue that causes NiFi to ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3083 Thanks @markap14! This has been merged to master. ---
[GitHub] nifi issue #3083: NIFI-5709, NIFI-5710: Addressed issue that causes NiFi to ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3083 Will review... ---
[GitHub] nifi pull request #3082: NIFI-375: Ensuring the run status endpoints are pro...
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3082 NIFI-375: Ensuring the run status endpoints are properly merged NIFI-375: - Ensuring the run status endpoints are properly merged. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-375 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3082.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3082 commit 30642c5d73b6078ec8197d9e2ffecb2e72064da0 Author: Matt Gilman Date: 2018-10-16T18:03:26Z NIFI-375: - Ensuring the run status endpoints are properly merged. ---
[GitHub] nifi issue #2977: NIFI-5562 - Upgraded guava versions from v18.0 to v26.0-jr...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2977 Happy to help review this PR once it's ready. ---
[GitHub] nifi issue #3069: NIFI-4806: Bumping to tika 1.19.1
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3069 Thanks @joewitt ---
[GitHub] nifi issue #3067: NIFI-5665 - Setting zookeeper's io.netty:netty transitive ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3067 Thanks @thenatog! This has been merged to master. ---
[GitHub] nifi pull request #3069: NIFI-4806: Bumping to tika 1.19.1
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3069 NIFI-4806: Bumping to tika 1.19.1 NIFI-4806: - Bumping to tika 1.19.1. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-4806 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3069.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3069 commit bba58ee1cdfc7bcd36484e6849401af08cd6f23e Author: Matt Gilman Date: 2018-10-12T18:30:34Z NIFI-4806: - Bumping to tika 1.19.1. ---
[GitHub] nifi issue #3067: NIFI-5665 - Setting zookeeper's io.netty:netty transitive ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3067 Will review... ---
[GitHub] nifi issue #3066: NIFI-5691: Overriding the version of jackson in aws java s...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3066 It appears that the nifi-aws-nar was already including the newer versions of jackson. However, the nifi-aws-service-api-nar was not. There is no implementations in the service api, however the interfaces in the api contain classes which are part of the aws-java-sdk so the artifacts get bundled there too. ---
[GitHub] nifi issue #3066: NIFI-5691: Overriding the version of jackson in aws java s...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3066 Attaching to files showing the effects the proposed pom changes has on the artifacts bundled. [5691-sorted.txt](https://github.com/apache/nifi/files/2473454/5691-sorted.txt) [master-sorted.txt](https://github.com/apache/nifi/files/2473455/master-sorted.txt) ---
[GitHub] nifi pull request #3066: NIFI-5691: Overriding the version of jackson in aws...
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3066 NIFI-5691: Overriding the version of jackson in aws java sdk NIFI-5691: - Overriding the version of jackson in aws java sdk. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-5691 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3066.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3066 commit 72b6cf4f422f127b4dea167364f4b107e83c08d3 Author: Matt Gilman Date: 2018-10-12T14:23:47Z NIFI-5691: - Overriding the version of jackson in aws java sdk. ---
[GitHub] nifi issue #3064: NIFI-5688: Ensure that when we map our flow to a Versioned...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3064 Thanks @markap14! This has been merged to master. ---
[GitHub] nifi issue #3064: NIFI-5688: Ensure that when we map our flow to a Versioned...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3064 Will review.. ---
[GitHub] nifi pull request #3063: NIFI-5661: Always showing load balance settings
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3063 NIFI-5661: Always showing load balance settings NIFI-5661: - Allowing the load balance configuration to be shown/edited in both clustered and standalone mode. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-5661 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3063.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3063 commit 79c03caf4f57a1273cbe5856ec7b6fc007f00a3e Author: Matt Gilman Date: 2018-10-11T16:23:53Z NIFI-5661: - Allowing the load balance configuration to be shown/edited in both clustered and standalone mode. ---
[GitHub] nifi issue #3034: NIFI-5479 - Fixed up dependencies to remove the WARNs caus...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3034 Thanks @thenatog @joewitt. This has been merged to master. ---
[GitHub] nifi issue #3034: NIFI-5479 - Fixed up dependencies to remove the WARNs caus...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3034 @thenatog @joewitt Happy to continue reviewing here... ---
[GitHub] nifi issue #3046: NIFI-5661: Adding Load Balance config UI
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3046 Thanks @ijokarumawak! This has been merged to master. ---
[GitHub] nifi issue #3037: NIFI-5645: Auto reconnect ConsumeWindowsEventLog
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3037 Thanks @ijokarumawak! This has been merged to master. ---
[GitHub] nifi pull request #3037: NIFI-5645: Auto reconnect ConsumeWindowsEventLog
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/3037#discussion_r224205359 --- Diff: nifi-nar-bundles/nifi-windows-event-log-bundle/nifi-windows-event-log-processors/src/main/java/org/apache/nifi/processors/windows/event/log/ConsumeWindowsEventLog.java --- @@ -199,6 +230,8 @@ private String subscribe(ProcessContext context) throws URISyntaxException { subscriptionHandle = wEvtApi.EvtSubscribe(null, null, channel, query, null, null, evtSubscribeCallback, WEvtApi.EvtSubscribeFlags.SUBSCRIBE_TO_FUTURE | WEvtApi.EvtSubscribeFlags.EVT_SUBSCRIBE_STRICT); +lastActivityTimestamp = System.currentTimeMillis(); --- End diff -- Just wanted to clarify the logic here... Do we want to update the `lastActivityTimestamp` when the client was unable to subscribe? Could that lead to premature reconnection later? ---
[GitHub] nifi issue #3037: NIFI-5645: Auto reconnect ConsumeWindowsEventLog
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3037 Will review... ---
[GitHub] nifi issue #3046: NIFI-5661: Adding Load Balance config UI
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3046 Thanks for the update. We do not want to reference another module directly via `nf.`. This would only be working currently because we have not enabled a module loader yet. However, the codebase currently supports one. This can be seen at the top of any of the JS files. If a module loader was turned on, this would fail because the `nf.` namespace would no longer be in global scope. If you want to reference something in another module, we'll need to import it. Alternatively, you could import it in a module that is already imported in both. In the policy case, the combo options are in `nfCommon` and a utility function was added to access them. ---
[GitHub] nifi issue #3054: NIFI-5672: Do not compare Load Balancing address/port for ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3054 Thanks @markap14. This has been merged to master. ---
[GitHub] nifi issue #3054: NIFI-5672: Do not compare Load Balancing address/port for ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3054 Will review... ---
[GitHub] nifi issue #3052: NIFI-5666 Updated all usages of Spring, beanutils, collect...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3052 Thanks @joewitt. This has been merged to master. ---
[GitHub] nifi issue #3021: NIFI-5600 - Restore node location display on queue listing
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3021 Provided an alternate solution here: https://github.com/apache/nifi/pull/3055 ---
[GitHub] nifi pull request #3055: NIFI-5600: Fixing columns in queue listing and comp...
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/3055 NIFI-5600: Fixing columns in queue listing and component state NIFI-5600: - Recalculating the available columns for the queue listing and component state because they contain conditions which need to be re-evaluated. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-5600 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/3055.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3055 commit 4d8145d964b150ce3b159936955fd0a386fb5829 Author: Matt Gilman Date: 2018-10-09T16:49:31Z NIFI-5600: - Recalculating the available columns for the queue listing and component state because they contain conditions which need to be re-evaluated. ---
[GitHub] nifi issue #3021: NIFI-5600 - Restore node location display on queue listing
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3021 @pvillard31 Thanks for the PR! I can see what is happening here and I think a slightly different solution is required. With the introduction of NIFI-5208 we should be watching the cluster status actively so we can track the disconnected state. Because this state can change, I think we need to reconsider the columns we show when the listing is opened (not when the UI is bootstrapped). We actually have the same issue showing component state. I am going to open up another PR with this alternate solution. ---
[GitHub] nifi pull request #3046: NIFI-5661: Adding Load Balance config UI
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/3046#discussion_r223727659 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/nf-connection-details.js --- @@ -521,6 +524,27 @@ nfCommon.populateField('read-only-flow-file-expiration', connection.flowFileExpiration); nfCommon.populateField('read-only-back-pressure-object-threshold', connection.backPressureObjectThreshold); nfCommon.populateField('read-only-back-pressure-data-size-threshold', connection.backPressureDataSizeThreshold); + nfCommon.populateField('read-only-load-balance-strategy', nfCommon.getComboOptionText($('#load-balance-strategy-combo'), connection.loadBalanceStrategy)); + nfCommon.populateField('read-only-load-balance-partition-attribute', connection.loadBalancePartitionAttribute); + nfCommon.populateField('read-only-load-balance-compression', nfCommon.getComboOptionText($('#load-balance-compression-combo'), connection.loadBalanceCompression)); --- End diff -- I don't think we can access the combo's here and above on line 527. Opening the connection details dialog from the canvas (when the source is running for instance) probably works because the combos exist someplace else on the page. However, opening the connection details dialog from the Summary table fails because those combos do not exist in the DOM. If you're looking to avoid duplicating code, I would probably move the combo configuration into a shared place and access that from both places. I believe we did something similar with the policies combo. ---
[GitHub] nifi issue #3021: NIFI-5600 - Restore node location display on queue listing
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3021 Sorry for missing the mention... Will review ---
[GitHub] nifi issue #3052: NIFI-5666 Updated all usages of Spring, beanutils, collect...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3052 Will review... ---
[GitHub] nifi pull request #3046: NIFI-5661: Adding Load Balance config UI
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/3046#discussion_r223116414 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/css/connection-configuration.css --- @@ -122,4 +122,16 @@ div.undefined { line-height: 32px; height: 32px; padding: 0px 10px; -} \ No newline at end of file +} + +.multi-column-settings { --- End diff -- I don't think we need to duplicate this styles here. The styles from the `connection-details.css` should be in scope. ---
[GitHub] nifi pull request #3046: NIFI-5661: Adding Load Balance config UI
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/3046#discussion_r223115867 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-connection.js --- @@ -1341,6 +1363,52 @@ } }); +// determine whether or not to show the load-balance icon + connectionLabelContainer.select('text.load-balance-icon') +.classed('hidden', function () { +if (d.permissions.canRead) { +return !isLoadBalanceConfigured(d.component); +} else { +return true; +} +}).classed('load-balance-icon-active fa-rotate-90', function (d) { +return d.permissions.canRead && d.component.loadBalanceStatus === 'LOAD_BALANCE_ACTIVE'; + +}).classed('load-balance-icon-184', function() { +return isExpirationConfigured(d.component); + +}).classed('load-balance-icon-200', function() { +return !isExpirationConfigured(d.component); + +}).attr('x', function() { +return isExpirationConfigured(d.component) ? 184 : 200; --- End diff -- When the user does not have permission to the connection, the `component` here will not be set. We need to check this and handle accordingly anywhere we are invoking `isExpirationConfigured` (see the above 3 cases) since it requires the `component`. ---
[GitHub] nifi issue #3046: NIFI-5661: Adding Load Balance config UI
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3046 Will review... ---
[GitHub] nifi issue #3044: NIFI-5581: Fix replicate request timeout
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3044 Thanks @ijokarumawak! This has been merged to master. ---
[GitHub] nifi issue #3044: NIFI-5581: Fix replicate request timeout
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3044 Will review... ---
[GitHub] nifi issue #3018: NIFI-5622 Updated test resource keystores and truststores ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3018 Thanks @alopresto! This has been merged to master. ---
[GitHub] nifi issue #3028: Nifi 4806
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3028 Thanks @joewitt! This has been merged to master. ---
[GitHub] nifi issue #3028: Nifi 4806
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3028 Will review... ---
[GitHub] nifi issue #3035: NIFI-5628 Added content length check to OkHttpReplicationC...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3035 Thanks @alopresto! This has been merged to master. ---
[GitHub] nifi issue #3035: NIFI-5628 Added content length check to OkHttpReplicationC...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3035 Thanks @thenatog! I'll also have a look. ---
[GitHub] nifi issue #3030: NIFI-5634: When merging RPG entities, ensure that we only ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3030 Thanks @markap14! This has been merged to master. ---
[GitHub] nifi issue #3030: NIFI-5634: When merging RPG entities, ensure that we only ...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3030 Will review... ---
[GitHub] nifi issue #3035: NIFI-5628 Added content length check to OkHttpReplicationC...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/3035 Will review... ---
[GitHub] nifi-registry pull request #141: NIFIREG-186 - Referencing the correct paren...
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi-registry/pull/141 NIFIREG-186 - Referencing the correct parent artifact NIFIREG-186: - Referencing the correct parent artifact You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi-registry NIFIREG-186 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi-registry/pull/141.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #141 commit 72bec8135fcab7f1cc9c704d480996afa2bb8e45 Author: Matt Gilman Date: 2018-09-20T19:05:01Z NIFIREG-186 Referencing the correct parent artifact. ---
[GitHub] nifi issue #2990: NIFI-375: Added operation policy
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2990 @ijokarumawak Thanks this looks good. I'm going to squash and merged to master. I've created two new JIRAs that are related to this effort but separate from the initial concern. Thanks again for working through all the review comments! https://issues.apache.org/jira/browse/NIFI-5610 https://issues.apache.org/jira/browse/NIFI-5611 ---
[GitHub] nifi issue #2990: NIFI-375: Added operation policy
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2990 @ijokarumawak This is looking really good! Just seeing a couple minor things aside from the NPE I commented in the code. It looks like the `operatePermissions` were added to the `ComponentEntity`. This is good since this applied to the majority of components. However, I noticed that the `operatePermissions` was also added to the each of the concrete implementations (`ProcessorEntity`, `PortEntity`, etc). I do not believe we need the field and getter/setter methods within each concrete implementation. Is there a reason they are there? Also, I think we need to apply the new `Operation` policy checks in `FlowResource#activeControllerServices`. This is essentially like the corresponding `FlowResource#scheduleComponents` but for `ControllerServices`. This endpoint is not called directly by the NiFi UI but I think it should probably utilize the new policy. Thanks again! ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r218531396 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java --- @@ -1060,7 +1069,8 @@ public ProcessGroupStatusDTO createProcessGroupStatusDto(final ProcessGroup proc final Collection childRemoteProcessGroupStatusCollection = processGroupStatus.getRemoteProcessGroupStatus(); if (childRemoteProcessGroupStatusCollection != null) { for (final RemoteProcessGroupStatus childRemoteProcessGroupStatus : childRemoteProcessGroupStatusCollection) { -final RemoteProcessGroupStatusDTO childRemoteProcessGroupStatusDto = createRemoteProcessGroupStatusDto(childRemoteProcessGroupStatus); +final RemoteProcessGroup childRemoteProcessGroup = processGroup.getRemoteProcessGroup(childRemoteProcessGroupStatus.getId()); --- End diff -- The way this method is called recursively the `childRemoteProcessGroup` is not necessarily a child of `processGroup`. This can be seen by creating a `RemoteProcessGroup` in a nested `ProcessGroup`. Then open the Summary table. You'll see an NPE. I think the fix is pretty trivial. We are actually already getting the `remoteProcessGroup` one line below using the recursive `findXxx` method. Moving this line up and using identifier from the status and not the DTO should address the problem. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217816900 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-settings.js --- @@ -1045,29 +1041,38 @@ var reportingTaskActionFormatter = function (row, cell, value, columnDef, dataContext) { var markup = ''; -if (dataContext.permissions.canRead && dataContext.permissions.canWrite) { -if (dataContext.component.state === 'RUNNING') { +var canWrite = dataContext.permissions.canWrite; +var canRead = dataContext.permissions.canRead; +var canOperate = dataContext.operatePermissions.canWrite || canWrite; +var isStopped = dataContext.status.runStatus === 'STOPPED'; + +if (dataContext.status.runStatus === 'RUNNING') { +if (canOperate) { markup += ''; -} else if (dataContext.component.state === 'STOPPED' || dataContext.component.state === 'DISABLED') { -markup += ''; +} -// support starting when stopped and no validation errors -if (dataContext.component.state === 'STOPPED' && nfCommon.isEmpty(dataContext.component.validationErrors)) { -markup += ''; -} +} else if (isStopped || dataContext.status.runStatus === 'DISABLED') { -if (dataContext.component.multipleVersionsAvailable === true) { -markup += ''; -} +if (canRead && canWrite) { +markup += ''; +} -if (nfCommon.canModifyController()) { -markup += ''; -} +// support starting when stopped and no validation errors +if (canOperate && dataContext.status.runStatus === 'STOPPED' && dataContext.status.validationStatus === 'VALID') { +markup += ''; } -if (dataContext.component.persistsState === true) { -markup += ''; +if (canRead && canWrite && dataContext.component.multipleVersionsAvailable === true) { +markup += ''; } + +if (nfCommon.canModifyController()) { --- End diff -- Ah, I see what's changed. I was a little confused. The issue is that component removal requires permissions to `WRITE` both the component in question and it's parent. With the changes introduced here, we are no longer including `canWrite` when determining the visibility of the delete icons. When I was reviewing, I was seeing the delete icon when I shouldn't have been. I only had read and operate permissions to the component but the UI was presenting the delete icon (because I still had write permissions to the parent). However, when I attempted to actually delete it NiFi correctly told me I wasn't allowed. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217390883 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/OperationAuthorizable.java --- @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.authorization.resource; + +import org.apache.nifi.authorization.AccessDeniedException; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.Resource; +import org.apache.nifi.authorization.user.NiFiUser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Authorizable for a component that can be scheduled by operators. + */ +public class OperationAuthorizable implements Authorizable, EnforcePolicyPermissionsThroughBaseResource { +private static Logger logger = LoggerFactory.getLogger(OperationAuthorizable.class); +private final Authorizable baseAuthorizable; + +public OperationAuthorizable(final Authorizable baseAuthorizable) { +this.baseAuthorizable = baseAuthorizable; +} + +@Override +public Authorizable getParentAuthorizable() { +// Need to return parent operation authorizable. E.g. /operation/processor/ -> /operation/process-group/ -> /run-status/process-group/root +if (baseAuthorizable.getParentAuthorizable() == null) { +return null; +} else { +return new OperationAuthorizable(baseAuthorizable.getParentAuthorizable()); +} +} + +@Override +public Authorizable getBaseAuthorizable() { +return baseAuthorizable; +} + +@Override +public Resource getResource() { +return ResourceFactory.getOperationResource(baseAuthorizable.getResource()); +} + +/** + * Authorize the request action with the resource using base authorizable and operation authorizable combination. + * + * This method authorizes the request with the base authorizable first. If the request is allowed, then finish authorization. + * If base authorizable denies the request, then it checks if the user has WRITE permission for '/operation/{componentType}/{id}'. + */ +public static void authorize(final Authorizable baseAuthorizable, final Authorizer authorizer, final RequestAction requestAction, final NiFiUser user) { +try { +baseAuthorizable.authorize(authorizer, requestAction, user); +} catch (AccessDeniedException e) { +logger.debug("Authorization failed with {}. Try authorizing with OperationAuthorizable.", baseAuthorizable, e); +// Always use WRITE action for operation. +new OperationAuthorizable(baseAuthorizable).authorize(authorizer, RequestAction.WRITE, user); +} + +} + +/** + * Check if the request is authorized. + * + * @return True if the request is allowed by the base authorizable, or the user has WRITE permission for '/operation/{componentType}/id'. + */ +public static boolean isAuthorized(final Authorizable baseAuthorizable, final Authorizer authorizer, final RequestAction requestAction, final NiFiUser user) { --- End diff -- There is no time when someone should invoke this method with a `requestAction` of `WRITE`. Can we remove that parameter? Also, because of this and that the name is the same as the non-static version (which has admittedly confused me a number of times while reviewing) can we rename this method to more accurately depict its intention. Maybe something along the lines of `isOperationAuthorized`. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217390667 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/resource/OperationAuthorizable.java --- @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.authorization.resource; + +import org.apache.nifi.authorization.AccessDeniedException; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.Resource; +import org.apache.nifi.authorization.user.NiFiUser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Authorizable for a component that can be scheduled by operators. + */ +public class OperationAuthorizable implements Authorizable, EnforcePolicyPermissionsThroughBaseResource { +private static Logger logger = LoggerFactory.getLogger(OperationAuthorizable.class); +private final Authorizable baseAuthorizable; + +public OperationAuthorizable(final Authorizable baseAuthorizable) { +this.baseAuthorizable = baseAuthorizable; +} + +@Override +public Authorizable getParentAuthorizable() { +// Need to return parent operation authorizable. E.g. /operation/processor/ -> /operation/process-group/ -> /run-status/process-group/root +if (baseAuthorizable.getParentAuthorizable() == null) { +return null; +} else { +return new OperationAuthorizable(baseAuthorizable.getParentAuthorizable()); +} +} + +@Override +public Authorizable getBaseAuthorizable() { +return baseAuthorizable; +} + +@Override +public Resource getResource() { +return ResourceFactory.getOperationResource(baseAuthorizable.getResource()); +} + +/** + * Authorize the request action with the resource using base authorizable and operation authorizable combination. + * + * This method authorizes the request with the base authorizable first. If the request is allowed, then finish authorization. + * If base authorizable denies the request, then it checks if the user has WRITE permission for '/operation/{componentType}/{id}'. + */ +public static void authorize(final Authorizable baseAuthorizable, final Authorizer authorizer, final RequestAction requestAction, final NiFiUser user) { --- End diff -- There is no time when someone should invoke this method with a `requestAction` of `WRITE`. Can we remove that parameter? Also, because of this and that the name is the same as the non-static version (which has admittedly confused me a number of times while reviewing) can we rename this method to more accurately depict its intention. Maybe something along the lines of `authorizeOperation`. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217387530 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/RemoteProcessGroupResource.java --- @@ -434,6 +436,197 @@ public Response updateRemoteProcessGroupOutputPort( ); } +/** + * Updates the specified remote process group input port run status. + * + * @param httpServletRequest request + * @param id The id of the remote process group to update. + * @param portId The id of the input port to update. + * @param requestRemotePortRunStatusEntity The remoteProcessGroupPortRunStatusEntity + * @return A remoteProcessGroupPortEntity + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/input-ports/{port-id}/run-status") +@ApiOperation( +value = "Updates run status of a remote port", +notes = NON_GUARANTEED_ENDPOINT, +response = RemoteProcessGroupPortEntity.class, +authorizations = { +@Authorization(value = "Write - /remote-process-groups/{uuid} or /operation/remote-process-groups/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRemoteProcessGroupInputPortRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The remote process group id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The remote process group port id.", +required = true +) +@PathParam("port-id") final String portId, +@ApiParam( +value = "The remote process group port.", +required = true +) final RemotePortRunStatusEntity requestRemotePortRunStatusEntity) { + +if (requestRemotePortRunStatusEntity == null) { +throw new IllegalArgumentException("Remote process group port run status must be specified."); +} + +if (requestRemotePortRunStatusEntity.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRemotePortRunStatusEntity.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRemotePortRunStatusEntity); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRemotePortRunStatusEntity.isDisconnectedNodeAcknowledged()); +} + +final Revision requestRevision = getRevision(requestRemotePortRunStatusEntity.getRevision(), id); +return withWriteLock( +serviceFacade, +requestRemotePortRunStatusEntity, +requestRevision, +lookup -> { +final Authorizable remoteProcessGroup = lookup.getRemoteProcessGroup(id); +OperationAuthorizable.isAuthorized(remoteProcessGroup, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); --- End diff -- This should be calling `authorize` and not checking `isAuthorized`. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217415480 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-settings.js --- @@ -1045,29 +1041,38 @@ var reportingTaskActionFormatter = function (row, cell, value, columnDef, dataContext) { var markup = ''; -if (dataContext.permissions.canRead && dataContext.permissions.canWrite) { -if (dataContext.component.state === 'RUNNING') { +var canWrite = dataContext.permissions.canWrite; +var canRead = dataContext.permissions.canRead; +var canOperate = dataContext.operatePermissions.canWrite || canWrite; +var isStopped = dataContext.status.runStatus === 'STOPPED'; + +if (dataContext.status.runStatus === 'RUNNING') { +if (canOperate) { markup += ''; -} else if (dataContext.component.state === 'STOPPED' || dataContext.component.state === 'DISABLED') { -markup += ''; +} -// support starting when stopped and no validation errors -if (dataContext.component.state === 'STOPPED' && nfCommon.isEmpty(dataContext.component.validationErrors)) { -markup += ''; -} +} else if (isStopped || dataContext.status.runStatus === 'DISABLED') { -if (dataContext.component.multipleVersionsAvailable === true) { -markup += ''; -} +if (canRead && canWrite) { +markup += ''; +} -if (nfCommon.canModifyController()) { -markup += ''; -} +// support starting when stopped and no validation errors +if (canOperate && dataContext.status.runStatus === 'STOPPED' && dataContext.status.validationStatus === 'VALID') { +markup += ''; } -if (dataContext.component.persistsState === true) { -markup += ''; +if (canRead && canWrite && dataContext.component.multipleVersionsAvailable === true) { +markup += ''; } + +if (nfCommon.canModifyController()) { --- End diff -- I realize this isn't an issue with this PR but I believe this should be checking `canWrite` and not deferring to check the controller permission. The controller permission will have already been considered when determining a value for `canWrite` server-side since it should be the parent resource for every reporting task. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217401802 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-remote-process-group.js --- @@ -741,8 +740,8 @@ }) .attr('font-family', function (d) { var family = ''; -if (d.permissions.canRead) { -if (hasIssues(d) || d.component.transmitting) { +if (d.permissions.canRead || d.operatePermissions.canWrite) { --- End diff -- These checks (here and below) also need to consider if the user has `d.permission.canWrite`. If the user only has operate permissions they can see the status. But if the user does not have operate but does have write to the RPG they currently cannot see the status icon. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217387691 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/RemoteProcessGroupResource.java --- @@ -434,6 +436,197 @@ public Response updateRemoteProcessGroupOutputPort( ); } +/** + * Updates the specified remote process group input port run status. + * + * @param httpServletRequest request + * @param id The id of the remote process group to update. + * @param portId The id of the input port to update. + * @param requestRemotePortRunStatusEntity The remoteProcessGroupPortRunStatusEntity + * @return A remoteProcessGroupPortEntity + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/input-ports/{port-id}/run-status") +@ApiOperation( +value = "Updates run status of a remote port", +notes = NON_GUARANTEED_ENDPOINT, +response = RemoteProcessGroupPortEntity.class, +authorizations = { +@Authorization(value = "Write - /remote-process-groups/{uuid} or /operation/remote-process-groups/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRemoteProcessGroupInputPortRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The remote process group id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The remote process group port id.", +required = true +) +@PathParam("port-id") final String portId, +@ApiParam( +value = "The remote process group port.", +required = true +) final RemotePortRunStatusEntity requestRemotePortRunStatusEntity) { + +if (requestRemotePortRunStatusEntity == null) { +throw new IllegalArgumentException("Remote process group port run status must be specified."); +} + +if (requestRemotePortRunStatusEntity.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRemotePortRunStatusEntity.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRemotePortRunStatusEntity); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRemotePortRunStatusEntity.isDisconnectedNodeAcknowledged()); +} + +final Revision requestRevision = getRevision(requestRemotePortRunStatusEntity.getRevision(), id); +return withWriteLock( +serviceFacade, +requestRemotePortRunStatusEntity, +requestRevision, +lookup -> { +final Authorizable remoteProcessGroup = lookup.getRemoteProcessGroup(id); +OperationAuthorizable.isAuthorized(remoteProcessGroup, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); +}, +() -> serviceFacade.verifyUpdateRemoteProcessGroupInputPort(id, createPortDTOWithDesiredRunStatus(portId, id, requestRemotePortRunStatusEntity)), +(revision, remotePortRunStatusEntity) -> { +// update the specified remote process group +final RemoteProcessGroupPortEntity controllerResponse = serviceFacade.updateRemoteProcessGroupInputPort(revis
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217405219 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-remote-process-group.js --- @@ -741,8 +740,8 @@ }) .attr('font-family', function (d) { var family = ''; -if (d.permissions.canRead) { -if (hasIssues(d) || d.component.transmitting) { +if (d.permissions.canRead || d.operatePermissions.canWrite) { --- End diff -- Actually, for determining which icon we show and the appropriate style it appears that we are now only using the status. This is available to everyone so I don't think we need any permission checks here. We should only need them when adding the tooltips. This should be very similar to the Processor Invalid icon and the tooltips for the validation errors. ---
[GitHub] nifi issue #2990: NIFI-375: Added operation policy
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2990 @ijokarumawak I see. You're correct that the existing codebase also exhibits this behavior. However, I think that its an issue we should consider fixing separately from this. Generally speaking, I think the UI should not allow the user to change something that they cannot change back (assuming the same permissions). Let's proceed here and we can address this concern in a future effort. Thanks... I'll continue reviewing! ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217134368 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerServiceResource.java --- @@ -741,6 +743,88 @@ public Response removeControllerService( ); } +/** + * Updates the operational status for the specified controller service with the specified values. + * + * @param httpServletRequest request + * @param id The id of the controller service to update. + * @param requestRunStatusA runStatusEntity. + * @return A controllerServiceEntity. + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/run-status") +@ApiOperation( +value = "Updates run status of a controller service", +response = ControllerServiceEntity.class, +authorizations = { +@Authorization(value = "Write - /controller-services/{uuid} or /operation/controller-services/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRunStatus( +@Context HttpServletRequest httpServletRequest, +@ApiParam( +value = "The controller service id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The controller service run status.", +required = true +) final ControllerServiceRunStatusEntity requestRunStatus) { + +if (requestRunStatus == null) { +throw new IllegalArgumentException("Controller service run status must be specified."); +} + +if (requestRunStatus.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRunStatus.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRunStatus); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRunStatus.isDisconnectedNodeAcknowledged()); +} + +// handle expects request (usually from the cluster manager) +final Revision requestRevision = getRevision(requestRunStatus.getRevision(), id); +// Create DTO to verify if it can be updated. +final ControllerServiceDTO controllerServiceDTO = new ControllerServiceDTO(); +controllerServiceDTO.setId(id); +controllerServiceDTO.setState(requestRunStatus.getState()); +return withWriteLock( +serviceFacade, +requestRunStatus, +requestRevision, +lookup -> { +// authorize the service +final Authorizable authorizable = lookup.getControllerService(id).getAuthorizable(); +OperationAuthorizable.authorize(authorizable, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); +}, +() -> serviceFacade.verifyUpdateControllerService(controllerServiceDTO), +(revision, runStatusEntity) -> { +// update the controller service +final ControllerServiceEntity entity = serviceFacade.updateControllerService(revision, controllerServiceDTO); --- End diff -- We need to recreate this `controllerServiceDTO` using the `runStatusEntity` due to how we authorize/cache requests during our two phase commit. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217134851 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/OutputPortResource.java --- @@ -315,6 +319,90 @@ public Response removeOutputPort( ); } + +/** + * Updates the operational status for the specified input port with the specified values. + * + * @param httpServletRequest request + * @param id The id of the port to update. + * @param requestRunStatusA portRunStatusEntity. + * @return A portEntity. + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("/{id}/run-status") +@ApiOperation( +value = "Updates run status of an output-port", +response = ProcessorEntity.class, +authorizations = { +@Authorization(value = "Write - /output-ports/{uuid} or /operation/output-ports/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The port id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The port run status.", +required = true +) final PortRunStatusEntity requestRunStatus) { + +if (requestRunStatus == null) { +throw new IllegalArgumentException("Port run status must be specified."); +} + +if (requestRunStatus.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRunStatus.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRunStatus); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRunStatus.isDisconnectedNodeAcknowledged()); +} + +// handle expects request (usually from the cluster manager) +final Revision requestRevision = getRevision(requestRunStatus.getRevision(), id); +// Create port DTO to verify if it can be updated. +final PortDTO portDTO = new PortDTO(); +portDTO.setId(id); +portDTO.setState(requestRunStatus.getState()); + +return withWriteLock( +serviceFacade, +requestRunStatus, +requestRevision, +lookup -> { +final NiFiUser user = NiFiUserUtils.getNiFiUser(); + +final Authorizable authorizable = lookup.getOutputPort(id); +OperationAuthorizable.authorize(authorizable, authorizer, RequestAction.WRITE, user); +}, +() -> serviceFacade.verifyUpdateOutputPort(portDTO), +(revision, runStatusEntity) -> { +// update the input port +final PortEntity entity = serviceFacade.updateOutputPort(revision, portDTO); --- End diff -- We need to recreate this `portDTO` using the `runStatusEntity` due to how we authorize/cache requests during our two phase commit. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217135024 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessorResource.java --- @@ -668,6 +670,91 @@ public Response deleteProcessor( ); } +/** + * Updates the operational status for the specified processor with the specified values. + * + * @param httpServletRequest request + * @param id The id of the processor to update. + * @param requestRunStatusA processorEntity. + * @return A processorEntity. + * @throws InterruptedException if interrupted + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("/{id}/run-status") +@ApiOperation( +value = "Updates run status of a processor", +response = ProcessorEntity.class, +authorizations = { +@Authorization(value = "Write - /processors/{uuid} or /operation/processors/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The processor id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The processor run status.", +required = true +) final ProcessorRunStatusEntity requestRunStatus) { + +if (requestRunStatus == null) { +throw new IllegalArgumentException("Processor run status must be specified."); +} + +if (requestRunStatus.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRunStatus.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRunStatus); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRunStatus.isDisconnectedNodeAcknowledged()); +} + +// handle expects request (usually from the cluster manager) +final Revision requestRevision = getRevision(requestRunStatus.getRevision(), id); +// Create processor DTO to verify if it can be updated. +final ProcessorDTO requestProcessorDTO = new ProcessorDTO(); +requestProcessorDTO.setId(id); +requestProcessorDTO.setState(requestRunStatus.getState()); + +return withWriteLock( +serviceFacade, +requestRunStatus, +requestRevision, +lookup -> { +final NiFiUser user = NiFiUserUtils.getNiFiUser(); + +final Authorizable authorizable = lookup.getProcessor(id).getAuthorizable(); +OperationAuthorizable.authorize(authorizable, authorizer, RequestAction.WRITE, user); +}, +() -> serviceFacade.verifyUpdateProcessor(requestProcessorDTO), +(revision, runStatusEntity) -> { +// update the processor +final ProcessorEntity entity = serviceFacade.updateProcessor(revision, requestProcessorDTO); --- End diff -- We need to recreate this `requestProcessorDTO` using the `runStatusEntity` due to how we authorize/cache requests during our two phase commit. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217140684 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/ComponentEntityMerger.java --- @@ -68,10 +75,13 @@ default void merge(final EntityType clientEntity, final Map MAX_BULLETINS_PER_COMPONENT) { clientEntity.setBulletins(clientEntity.getBulletins().subList(0, MAX_BULLETINS_PER_COMPONENT)); } +} else { +clientEntity.setBulletins(null); --- End diff -- I think we need to continue to set the component to null when `canRead` is false. It may have been changed to accommodate an earlier iteration of this PR. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217134678 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/InputPortResource.java --- @@ -315,6 +319,90 @@ public Response removeInputPort( ); } +/** + * Updates the operational status for the specified input port with the specified values. + * + * @param httpServletRequest request + * @param id The id of the port to update. + * @param requestRunStatusA portRunStatusEntity. + * @return A portEntity. + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("/{id}/run-status") +@ApiOperation( +value = "Updates run status of an input-port", +response = ProcessorEntity.class, +authorizations = { +@Authorization(value = "Write - /input-ports/{uuid} or /operation/input-ports/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The port id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The port run status.", +required = true +) final PortRunStatusEntity requestRunStatus) { + +if (requestRunStatus == null) { +throw new IllegalArgumentException("Port run status must be specified."); +} + +if (requestRunStatus.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRunStatus.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRunStatus); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRunStatus.isDisconnectedNodeAcknowledged()); +} + +// handle expects request (usually from the cluster manager) +final Revision requestRevision = getRevision(requestRunStatus.getRevision(), id); +// Create port DTO to verify if it can be updated. +final PortDTO portDTO = new PortDTO(); +portDTO.setId(id); +portDTO.setState(requestRunStatus.getState()); + +return withWriteLock( +serviceFacade, +requestRunStatus, +requestRevision, +lookup -> { +final NiFiUser user = NiFiUserUtils.getNiFiUser(); + +final Authorizable authorizable = lookup.getInputPort(id); +OperationAuthorizable.authorize(authorizable, authorizer, RequestAction.WRITE, user); +}, +() -> serviceFacade.verifyUpdateInputPort(portDTO), +(revision, runStatusEntity) -> { +// update the input port +final PortEntity entity = serviceFacade.updateInputPort(revision, portDTO); --- End diff -- We need to recreate this `portDTO` using the `runStatusEntity` due to how we authorize/cache requests during our two phase commit. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217135735 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/RemoteProcessGroupResource.java --- @@ -557,6 +750,90 @@ public Response updateRemoteProcessGroup( ); } +/** + * Updates the operational status for the specified remote process group with the specified value. + * + * @param httpServletRequest request + * @param id The id of the remote process group to update. + * @param requestRemotePortRunStatusEntity A remotePortRunStatusEntity. + * @return A remoteProcessGroupEntity. + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/run-status") +@ApiOperation( +value = "Updates run status of a remote process group", +response = RemoteProcessGroupEntity.class, +authorizations = { +@Authorization(value = "Write - /remote-process-groups/{uuid} or /operation/remote-process-groups/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRemoteProcessGroupRunStatus( +@Context HttpServletRequest httpServletRequest, +@ApiParam( +value = "The remote process group id.", +required = true +) +@PathParam("id") String id, +@ApiParam( +value = "The remote process group run status.", +required = true +) final RemotePortRunStatusEntity requestRemotePortRunStatusEntity) { + +if (requestRemotePortRunStatusEntity == null) { +throw new IllegalArgumentException("Remote process group run status must be specified."); +} + +if (requestRemotePortRunStatusEntity.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRemotePortRunStatusEntity.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRemotePortRunStatusEntity); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRemotePortRunStatusEntity.isDisconnectedNodeAcknowledged()); +} + +// handle expects request (usually from the cluster manager) +final Revision requestRevision = getRevision(requestRemotePortRunStatusEntity.getRevision(), id); +final RemoteProcessGroupDTO remoteProcessGroupDTO = new RemoteProcessGroupDTO(); +remoteProcessGroupDTO.setId(id); + remoteProcessGroupDTO.setTransmitting(shouldTransmit(requestRemotePortRunStatusEntity)); +return withWriteLock( +serviceFacade, +requestRemotePortRunStatusEntity, +requestRevision, +lookup -> { +Authorizable authorizable = lookup.getRemoteProcessGroup(id); +OperationAuthorizable.authorize(authorizable, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); +}, +() -> serviceFacade.verifyUpdateRemoteProcessGroup(remoteProcessGroupDTO), +(revision, remoteProcessGroupEntity) -> { +// update the specified remote process group +final RemoteProcessGroupEntity entity = serviceFacade.updateRemoteProcessGroup(revision, remoteProcessGroupDTO); --- End diff -- We need to recreate this `remoteProcessGroupDTO` using the `remoteProcessGroupEntity` due to how we authorize/cache requests during our two phase commit. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217135560 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/RemoteProcessGroupResource.java --- @@ -434,6 +436,197 @@ public Response updateRemoteProcessGroupOutputPort( ); } +/** + * Updates the specified remote process group input port run status. + * + * @param httpServletRequest request + * @param id The id of the remote process group to update. + * @param portId The id of the input port to update. + * @param requestRemotePortRunStatusEntity The remoteProcessGroupPortRunStatusEntity + * @return A remoteProcessGroupPortEntity + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/input-ports/{port-id}/run-status") +@ApiOperation( +value = "Updates run status of a remote port", +notes = NON_GUARANTEED_ENDPOINT, +response = RemoteProcessGroupPortEntity.class, +authorizations = { +@Authorization(value = "Write - /remote-process-groups/{uuid} or /operation/remote-process-groups/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRemoteProcessGroupInputPortRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The remote process group id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The remote process group port id.", +required = true +) +@PathParam("port-id") final String portId, +@ApiParam( +value = "The remote process group port.", +required = true +) final RemotePortRunStatusEntity requestRemotePortRunStatusEntity) { + +if (requestRemotePortRunStatusEntity == null) { +throw new IllegalArgumentException("Remote process group port run status must be specified."); +} + +if (requestRemotePortRunStatusEntity.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRemotePortRunStatusEntity.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRemotePortRunStatusEntity); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRemotePortRunStatusEntity.isDisconnectedNodeAcknowledged()); +} + +final Revision requestRevision = getRevision(requestRemotePortRunStatusEntity.getRevision(), id); +final RemoteProcessGroupPortDTO remoteProcessGroupPort = new RemoteProcessGroupPortDTO(); +remoteProcessGroupPort.setId(portId); +remoteProcessGroupPort.setGroupId(id); + remoteProcessGroupPort.setTransmitting(shouldTransmit(requestRemotePortRunStatusEntity)); + +return withWriteLock( +serviceFacade, +requestRemotePortRunStatusEntity, +requestRevision, +lookup -> { +final Authorizable remoteProcessGroup = lookup.getRemoteProcessGroup(id); +OperationAuthorizable.isAuthorized(remoteProcessGroup, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); +}, +() -> serviceFacade.verifyUpdateRemoteProcessGroupInputPort(id, remoteProcessGroupPort), +
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217135375 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/RemoteProcessGroupResource.java --- @@ -434,6 +436,197 @@ public Response updateRemoteProcessGroupOutputPort( ); } +/** + * Updates the specified remote process group input port run status. + * + * @param httpServletRequest request + * @param id The id of the remote process group to update. + * @param portId The id of the input port to update. + * @param requestRemotePortRunStatusEntity The remoteProcessGroupPortRunStatusEntity + * @return A remoteProcessGroupPortEntity + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/input-ports/{port-id}/run-status") +@ApiOperation( +value = "Updates run status of a remote port", +notes = NON_GUARANTEED_ENDPOINT, +response = RemoteProcessGroupPortEntity.class, +authorizations = { +@Authorization(value = "Write - /remote-process-groups/{uuid} or /operation/remote-process-groups/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRemoteProcessGroupInputPortRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The remote process group id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The remote process group port id.", +required = true +) +@PathParam("port-id") final String portId, +@ApiParam( +value = "The remote process group port.", +required = true +) final RemotePortRunStatusEntity requestRemotePortRunStatusEntity) { + +if (requestRemotePortRunStatusEntity == null) { +throw new IllegalArgumentException("Remote process group port run status must be specified."); +} + +if (requestRemotePortRunStatusEntity.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRemotePortRunStatusEntity.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRemotePortRunStatusEntity); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRemotePortRunStatusEntity.isDisconnectedNodeAcknowledged()); +} + +final Revision requestRevision = getRevision(requestRemotePortRunStatusEntity.getRevision(), id); +final RemoteProcessGroupPortDTO remoteProcessGroupPort = new RemoteProcessGroupPortDTO(); +remoteProcessGroupPort.setId(portId); +remoteProcessGroupPort.setGroupId(id); + remoteProcessGroupPort.setTransmitting(shouldTransmit(requestRemotePortRunStatusEntity)); + +return withWriteLock( +serviceFacade, +requestRemotePortRunStatusEntity, +requestRevision, +lookup -> { +final Authorizable remoteProcessGroup = lookup.getRemoteProcessGroup(id); +OperationAuthorizable.isAuthorized(remoteProcessGroup, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); +}, +() -> serviceFacade.verifyUpdateRemoteProcessGroupInputPort(id, remoteProcessGroupPort), +
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217136120 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ReportingTaskResource.java --- @@ -542,6 +547,88 @@ public Response removeReportingTask( ); } +/** + * Updates the operational status for the specified ReportingTask with the specified values. + * + * @param httpServletRequest request + * @param id The id of the reporting task to update. + * @param requestRunStatus A runStatusEntity. + * @return A reportingTaskEntity. + */ +@PUT +@Consumes(MediaType.APPLICATION_JSON) +@Produces(MediaType.APPLICATION_JSON) +@Path("{id}/run-status") +@ApiOperation( +value = "Updates run status of a reporting task", +response = ReportingTaskEntity.class, +authorizations = { +@Authorization(value = "Write - /reporting-tasks/{uuid} or or /operation/reporting-tasks/{uuid}") +} +) +@ApiResponses( +value = { +@ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), +@ApiResponse(code = 401, message = "Client could not be authenticated."), +@ApiResponse(code = 403, message = "Client is not authorized to make this request."), +@ApiResponse(code = 404, message = "The specified resource could not be found."), +@ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") +} +) +public Response updateRunStatus( +@Context final HttpServletRequest httpServletRequest, +@ApiParam( +value = "The reporting task id.", +required = true +) +@PathParam("id") final String id, +@ApiParam( +value = "The reporting task run status.", +required = true +) final ReportingTaskRunStatusEntity requestRunStatus) { + +if (requestRunStatus == null) { +throw new IllegalArgumentException("Reporting task run status must be specified."); +} + +if (requestRunStatus.getRevision() == null) { +throw new IllegalArgumentException("Revision must be specified."); +} + +requestRunStatus.validateState(); + +if (isReplicateRequest()) { +return replicate(HttpMethod.PUT, requestRunStatus); +} else if (isDisconnectedFromCluster()) { + verifyDisconnectedNodeModification(requestRunStatus.isDisconnectedNodeAcknowledged()); +} + +// handle expects request (usually from the cluster manager) +final Revision requestRevision = getRevision(requestRunStatus.getRevision(), id); +// Create DTO to verify if it can be updated. +final ReportingTaskDTO reportingTaskDTO = new ReportingTaskDTO(); +reportingTaskDTO.setId(id); +reportingTaskDTO.setState(requestRunStatus.getState()); +return withWriteLock( +serviceFacade, +requestRunStatus, +requestRevision, +lookup -> { +// authorize reporting task +final Authorizable authorizable = lookup.getReportingTask(id).getAuthorizable(); +OperationAuthorizable.authorize(authorizable, authorizer, RequestAction.WRITE, NiFiUserUtils.getNiFiUser()); +}, +() -> serviceFacade.verifyUpdateReportingTask(reportingTaskDTO), +(revision, reportingTaskEntity) -> { +// update the reporting task +final ReportingTaskEntity entity = serviceFacade.updateReportingTask(revision, reportingTaskDTO); --- End diff -- We need to recreate this `reportingTaskDTO` using the `reportingTaskEntity` due to how we authorize/cache requests during our two phase commit. ---
[GitHub] nifi pull request #2990: NIFI-375: Added operation policy
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2990#discussion_r217121570 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/ControllerServiceEntityMerger.java --- @@ -137,7 +138,9 @@ public static void mergeControllerServiceReferences(final Set
[GitHub] nifi issue #2990: NIFI-375: Added operation policy
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2990 @ijokarumawak Thanks for the update. I'm still in the process of reviewing but one thing that concerns me is where we've identified Service Only in the scenarios above. Currently (before the PR) the Enable case we allow the user to specify if they want to enable just this service or this service and all components that reference it (including other services and their referencing components). During the Disable case, we require that the user disables this service and all referencing components. This is because the referencing components require this service's availability to continue running. The issue that we're hitting now is that a user with permissions outlined above with Service Only will be able to Enable this service but will be unable to subsequently disable it. Because of this, I'm wondering if we need to be even more strict to prevent this cases via the UI. I don't think its too restrictive as this is more of a corner case. The more common use case here will be granting operators permissions to the read policies and operation policies for these components. Thoughts? ---
[GitHub] nifi issue #2990: NIFI-375: Added operation policy
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2990 @ijokarumawak Thanks for the additional commits! Will review... ---
[GitHub] nifi issue #2996: NIFI-5581: Disable connection pooling for OkHttpReplicatio...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2996 Will review... ---
[GitHub] nifi issue #2990: NIFI-375: Added operation policy
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2990 Will review... ---
[GitHub] nifi issue #2981: NIFI-5282: GCPProcessor with HTTP Proxy with Authenticatio...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2981 Thanks @ijokarumawak! This has been merged to master. ---
[GitHub] nifi issue #2981: NIFI-5282: GCPProcessor with HTTP Proxy with Authenticatio...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2981 Reviewing... ---
[GitHub] nifi issue #2957: NIFI-5543: fix connection path is not visualizing as selec...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2957 Thanks @markobean! This has been merged to master. ---
[GitHub] nifi pull request #2961: NIFI-5479: Upgrade Jetty
GitHub user mcgilman opened a pull request: https://github.com/apache/nifi/pull/2961 NIFI-5479: Upgrade Jetty Upgrading Jetty. Including a commit that @joewitt had provided in PR #2933. I have provided an additional commit that addresses a behavioral change in Jetty's SslContextFactory. I have verified various NiFi deployments including (un)secured standalone and clustered instances using both certificate based authentication and our JWT authentication using OpenID Connect. Once this PR is closed, we can also close #2933. @markap14 Can you help verify some of our processors that leverage the bundled Jetty server? @ijokarumawak Can you help verify our web socket processors? Would also appreciate anyone else willing to deploy this proposed change to ensure there aren't any other regressions. You can merge this pull request into a Git repository by running: $ git pull https://github.com/mcgilman/nifi NIFI-5479 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/2961.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2961 commit 0364cf584b55a617c6eded719f0bc5a8baf91c99 Author: joewitt Date: 2018-08-02T20:29:23Z NIFI-5479 Upgraded Jetty. Moved where we unpack bundled deps to so we can avoid a new jetty bug with META-INF loading logic. WIP for testing/eval. Not ready for merge commit 39e537c8c172e0675eb1e3a9918c3329aea4d9cd Author: Matt Gilman Date: 2018-08-23T18:21:28Z NIFI-5479: - Using the SUN provider when the keystore type is JKS. ---
[GitHub] nifi issue #2957: NIFI-5543: fix connection path is not visualizing as selec...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2957 @markobean I've replicated the behavior and believe there is a bug here. However, I don't think the proposed solution is the proper way to address it. The points in question are `visibility: hidden` to ensure they remain clickable. I believe the issue is the order the drag behavior is added to the points. Can you please try moving the drag behavior for the `midpoints` and the `endpoints` to below where the `mousedown` listeners are registered and see if that addresses the issue. ---
[GitHub] nifi-registry issue #135: [NIFIREG-193] upgrade superagent
Github user mcgilman commented on the issue: https://github.com/apache/nifi-registry/pull/135 Thanks @scottyaslan! This has been merged to master. ---
[GitHub] nifi-registry issue #135: [NIFIREG-193] upgrade superagent
Github user mcgilman commented on the issue: https://github.com/apache/nifi-registry/pull/135 Will review... ---
[GitHub] nifi issue #2899: NIFI-4535 Only update Page Title to root flow name when us...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2899 Thanks @patricker! This has been merged to master. ---
[GitHub] nifi issue #2941: [NIFI-5499] upgrade AngularJS to v1.7.2
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2941 Thanks @scottyaslan! This has been merged to master. ---
[GitHub] nifi issue #2941: [NIFI-5499] upgrade AngularJS to v1.7.2
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2941 @scottyaslan I believe we also need to update the package-lock.json that is checked into source control. I noticed the package-lock.json is updated in the front end working directory. ---
[GitHub] nifi issue #2941: [NIFI-5499] upgrade AngularJS to v1.7.2
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2941 Will review... ---
[GitHub] nifi issue #2935: NIFI-5476 Allow TLS toolkit to use externally-signed CA in...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2935 Thanks for the contributions @alopresto and @pepov! I'm also a +1 on the changes proposed here. ---
[GitHub] nifi issue #2933: NIFI-5479 Upgraded Jetty. Moved where we unpack bundled de...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2933 @joewitt Thanks for the PR! When starting up in secure mode using a configuration that works with current master branch, I received some stack traces regarding the initialization of the `SSLContext`. There appears to be a runtime difference introduced here that affects the loading of providers. > 1305 Caused by: java.security.NoSuchAlgorithmException: no such algorithm: JKS for provider BC 1306 at sun.security.jca.GetInstance.getService(GetInstance.java:87) 1307 at sun.security.jca.GetInstance.getInstance(GetInstance.java:206) 1308 at java.security.Security.getImpl(Security.java:698) 1309 at java.security.KeyStore.getInstance(KeyStore.java:896) 1310 ... 21 common frames omitted ---
[GitHub] nifi issue #2934: NIFI-5484: Fixed PutHive3Streaming to use the Hive Metasto...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2934 Thanks @mattyb149! This has been merged to master. ---
[GitHub] nifi pull request #2934: NIFI-5484: Fixed PutHive3Streaming to use the Hive ...
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2934#discussion_r207559164 --- Diff: nifi-commons/nifi-utils/src/main/java/org/apache/nifi/processor/util/StandardValidators.java --- @@ -390,6 +391,19 @@ public ValidationResult validate(final String subject, final String input, final } }; +public static final Validator URI_LIST_VALIDATOR = (subject, input, context) -> { + +if (context.isExpressionLanguageSupported(subject) && context.isExpressionLanguagePresent(input)) { +return new ValidationResult.Builder().subject(subject).input(input).explanation("Expression Language Present").valid(true).build(); +} +Optional invalidUri = Arrays.stream(input.split(",")) --- End diff -- I believe the way this stream is set up that an input of empty string would be considered valid. ---
[GitHub] nifi issue #2934: NIFI-5484: Fixed PutHive3Streaming to use the Hive Metasto...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2934 Will review... ---
[GitHub] nifi issue #2932: NIFI-5480: Use FlowController's maps of components in orde...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2932 Sounds good thanks! ---
[GitHub] nifi issue #2932: NIFI-5480: Use FlowController's maps of components in orde...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2932 Will review... ---
[GitHub] nifi issue #2928: NIFI-5475: Upgraded Hive 3 bundle to Apache Hive 3.1.0
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2928 Thanks @mattyb149! This has been merged to master. ---
[GitHub] nifi issue #2928: NIFI-5475: Upgraded Hive 3 bundle to Apache Hive 3.1.0
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2928 Will review... ---
[GitHub] nifi issue #2908: NIFI-5442 Get X-ProxyContextPath value from request attrib...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2908 Thanks again for the PR and reviews! This has been merged to master. ---
[GitHub] nifi issue #2908: NIFI-5442 Get X-ProxyContextPath value from request attrib...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2908 @alopresto I'm a +1 with the addition of the most recent commit. I'll wait until tomorrow to merge it in just in case anyone else engaged here [@danfike @mcg30005] wants to check it out. ---
[GitHub] nifi issue #2908: NIFI-5442 Get X-ProxyContextPath value from request attrib...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2908 Thanks for the PR! I'm happy to help get this merged in. I tried out the proposed patch locally and it does address the issue in the case identified. However, I tried to run in some of the other context and the filter was not running as expected. Adding a dispatcher type of FORWARD on the `SanitizeContextPathFilter` resolved the issue locally. With this update I'll be a +1. ---
[GitHub] nifi issue #2899: NIFI-4535 Only update Page Title to root flow name when us...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2899 @patricker Just had another look. I think there is one case that still hasn't been addressed. When a user has been granted permissions to the root group, the document title does update to the root group name. A check has been introduced that will only change the document title if the user has access to the root group. However, if the user does have access to the root group and those permissions are removed the document title does not change back to the previous value. It retains the name of the root group when they last loaded the page. We should be restoring the default value. The default value is loaded through the about endpoint [1]. It would probably make sense to store that value when the page loads so we can restore it in an else clause in your PR. Let me know if you any assistance here. I'm happy to help. [1] https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/controllers/nf-ng-canvas-global-menu-controller.js#L318 ---
[GitHub] nifi issue #2914: NIFI-5448 Added failure relationship to UpdateAttributes t...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2914 Looking at the changeset I see that a property was added to allow the user to opt-in to the new behavior. That should alleviate my concerns. Sorry for the confusion. Only other suggestion would be to change the name of "Stop Processing" `AllowableValue` to maybe "Penalize" as that is what the current behavior should be doing. ---
[GitHub] nifi issue #2914: NIFI-5448 Added failure relationship to UpdateAttributes t...
Github user mcgilman commented on the issue: https://github.com/apache/nifi/pull/2914 I would be very careful taking that option. Yes, it would ensure that users flows remain valid. However, I would question if that is the behavior that those users actually want. If the new relationship were auto-terminated that would mean that any data routed there would be DROPped. Folks upgrading may not realize this and it could lead to data loss. This concern was brought up in an old JIRA [1]. At that time, the solution was to penalize the flowfile and rollback the session. Since this processor is so widely used, it may warrant a DISCUSS thread so the community can consider the options and when they may be appropriate to introduce. [1] https://issues.apache.org/jira/browse/NIFI-11 ---
[GitHub] nifi pull request #2899: NIFI-4535 Only update Page Title to root flow name ...
Github user mcgilman commented on a diff in the pull request: https://github.com/apache/nifi/pull/2899#discussion_r202814854 --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/canvas/nf-canvas.js --- @@ -167,12 +167,14 @@ nfNgBridge.injector.get('breadcrumbsCtrl').resetScrollPosition(); // set page title to the name of the root processor group -var rootBreadcrumb = breadcrumb; -while(rootBreadcrumb.parentBreadcrumb != null) { -rootBreadcrumb = rootBreadcrumb.parentBreadcrumb -} +if (breadcrumb.permissions.canRead) { +var rootBreadcrumb = breadcrumb; +while(rootBreadcrumb.parentBreadcrumb != null) { +rootBreadcrumb = rootBreadcrumb.parentBreadcrumb +} -document.title = rootBreadcrumb.breadcrumb.name; +document.title = rootBreadcrumb.breadcrumb.name; --- End diff -- @patricker Thanks for the PR! I haven't run this yet, but I think there may still be an issue. We need to ensure we have permissions to the `rootBreadcrumb` here. If the user navigated to a sub-Process Group but did not have permissions to the root Process Group I think we'd still have the error. Additionally, if the user navigated to a sub-Process Group which they didn't have access to but they did have access to the root Process Group we would unnecessarily avoid setting the document title. I don't believe there was an issue with identifying the `rootBreadcrumb`. The issue was attempting to extract the breadcrumb's name through the `breadcrumb` field which is `null`ed out when a user doesn't have permissions to the corresponding Process Group. ---