You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/12/07 20:39:30 UTC
[GitHub] [nifi] exceptionfactory opened a new pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
exceptionfactory opened a new pull request #4712:
URL: https://github.com/apache/nifi/pull/4712
#### Description of PR
NIFI-7885 Introduces a Boolean environment variable named NIFI_HDFS_DENY_LOCAL_FILE_SYSTEM_ACCESS that be set to _true_ in order to deny access from HDFS Processors to the Local File System. This change impacts all Processors that extend AbstractHadoopProcessor. The evaluation occurs during Processor scheduling when retrieving the initial FileSystem object based on the provided Hadoop Configuration. Setting the environment variable to _true_ will caused an AccessDeniedException to be thrown if an operator attempts to configure access to the Local File System.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [X] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [X] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
- [X] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
### For code changes:
- [X] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
- [X] Have you written or updated unit tests to verify your changes?
- [ ] Have you verified that the full build is successful on JDK 8?
- [X] Have you verified that the full build is successful on JDK 11?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
- [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
- [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende merged pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
bbende merged pull request #4712:
URL: https://github.com/apache/nifi/pull/4712
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
bbende commented on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-741806785
Yea we need to look into that test and figure out why it is sometimes failing, but that is a separate issue. Will merge shortly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-741050871
Thanks for the confirmation. I refactored the approach to perform the check in `customValidate()` with the `FileSystem.getDefaultUri()` method, which performs some normalization to ensure that if the `fs.defaultFS` value equals `local`, it will be normalized to start with `file:///`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
bbende commented on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-740996110
Ah that is a good point that I had not considered. I think we can say that for all the processors we provide we are ensuring none of them circumvent the check in `preProcessConfiguration`. If someone implements a custom processor, well then all bets are off, they didn't even have to extend from `AbstractHadoopProcessor `and can do anything they want at that point.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-741487912
Thanks! It looks like the MacOS build failed on an unrelated unit test that is also impacting other recent builds.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory edited a comment on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
exceptionfactory edited a comment on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-740983198
@bbende Thanks for the review and feedback. The `resetHDFSResources()` method handles Kerberos setup options prior to calling `getFileSystemAsUser()`, which includes calling `preProcessConfiguration()` that subclasses can override to adjust the `Configuration`. Existing subclasses do not appear to do anything that would influence the file system type, so as long as that doesn't seem like a concern, changing the behavior to fail validation instead of throwing the AccessDeniedException during scheduling sounds like a better approach.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bbende commented on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
bbende commented on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-740930522
Code looks good, also built and tested this and seems to be working as described.
With the check inside `getFileSystemAsUser`, that first gets called during `onScheduled` so the error is produced when the user starts the processor in the UI. We then get an error like:
```
2020-12-08 19:38:21,329 ERROR org.apache.nifi.processors.hadoop.PutHDFS: PutHDFS[id=4388cbe3-0176-1000-ffff-ffffcb860909] HDFS Configuration error - java.nio.file.AccessDeniedException: file:///: LFS Access Denied according to Environment Variable [NIFI_HDFS_DENY_LOCAL_FILE_SYSTEM_ACCESS]: java.nio.file.AccessDeniedException: file:///: LFS Access Denied according to Environment Variable [NIFI_HDFS_DENY_LOCAL_FILE_SYSTEM_ACCESS]
java.n
2020-12-08 19:38:21,331 ERROR org.apache.nifi.processors.hadoop.PutHDFS: PutHDFS[id=4388cbe3-0176-1000-ffff-ffffcb860909] Failed to properly initialize Processor. If still scheduled to run, NiFi will attempt to initialize and run the Processor again after the 'Administrative Yield Duration' has elapsed. Failure is due to java.nio.file.AccessDeniedException: file:///: LFS Access Denied according to Environment Variable [NIFI_HDFS_DENY_LOCAL_FILE_SYSTEM_ACCESS]: java.nio.file.AccessDeniedException: file:///: LFS Access Denied according to Environment Variable [NIFI_HDFS_DENY_LOCAL_FILE_SYSTEM_ACCESS]
java.nio.file.AccessDeniedException: file:///: LFS Access Denied according to Environment Variable [NIFI_HDFS_DENY_LOCAL_FILE_SYSTEM_ACCESS]
```
At this point if flow files are sent into the processor they remain in the incoming queue since the processor technically isn't running yet, and the framework continues trying to call onScheduled after yielding.
I'm wondering if a better user experience would be to perform the check in `customValidate` so that the processor is not valid and can't be started and avoids all of the continuous errors above. We already have the `Configuration` object available and just need to get the value of `fs.defaultFS` and then use your same matcher against that value, thoughts?
https://github.com/apache/nifi/blob/main/nifi-nar-bundles/nifi-extension-utils/nifi-hadoop-utils/src/main/java/org/apache/nifi/processors/hadoop/AbstractHadoopProcessor.java#L219
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #4712: NIFI-7885 Added Environment Variable to deny LFS access using Hadoop
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4712:
URL: https://github.com/apache/nifi/pull/4712#issuecomment-740983198
@bbende Thanks for the review and feedback. The `resetHDFSResources()` method handles Kerberos setup options prior to calling `getFileSystemAsUser()`, which includes calling `preProcessConfiguration()` that subclasses can override to adjust the `Configuration`. Existing subclasses do not appear to do anything that would influence the file system type, so as long as that doesn't seem like a concern, changing the behavior to fail validation instead of throwing the AccessDeniedException during scheduling sounds ike a better approach.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org