NIFI-4256 Adds AWS Encryption Controller Service#3574
NIFI-4256 Adds AWS Encryption Controller Service#3574natural wants to merge 2 commits intoapache:masterfrom
Conversation
|
Reviewing... |
alopresto
left a comment
There was a problem hiding this comment.
Reviewed the code and made minor comments. Ran a local smoke test where NiFi listed & fetched items from a plain bucket and then used client side CMK encryption and put them back in a new bucket. A different flow then listed & fetched from the encrypted bucket and printed the plaintext data locally.
Will build with checkstyle and run tests locally, but once changes are made, +1.
...-processors/src/main/java/org/apache/nifi/processors/aws/s3/AbstractS3EncryptionService.java
Outdated
Show resolved
Hide resolved
...-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/PutS3Object.java
Show resolved
Hide resolved
...-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/PutS3Object.java
Show resolved
Hide resolved
.../main/java/org/apache/nifi/processors/aws/s3/encryption/ClientSideCMKEncryptionStrategy.java
Outdated
Show resolved
Hide resolved
...ocessors/src/main/java/org/apache/nifi/processors/aws/s3/encryption/S3EncryptionService.java
Outdated
Show resolved
Hide resolved
...ocessors/src/main/java/org/apache/nifi/processors/aws/s3/encryption/S3EncryptionService.java
Outdated
Show resolved
Hide resolved
| @Override | ||
| protected Collection<ValidationResult> customValidate(final ValidationContext validationContext) { | ||
| Collection<ValidationResult> validationResults = new ArrayList<>(); | ||
| validationResults.add(encryptionStrategy.validateKey(validationContext.getProperty(ENCRYPTION_VALUE).getValue())); |
There was a problem hiding this comment.
Add dependent validation for key ID or material value depending on encryption strategy (i.e. if CMK, when the Base64-encoded key material is entered, decode it and verify it is 16, 24, 32 bytes).
There was a problem hiding this comment.
I believe this is already done by the custom property validation.
| } | ||
|
|
||
| @Override | ||
| public ValidationResult validateKey(String keyValue) { |
There was a problem hiding this comment.
This method is repeated. I think it can be refactored.
...undle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/s3/ITPutS3Object.java
Show resolved
Hide resolved
|
Merged. Thanks for this contribution, Troy. |
NIFI-4256 Adds AWS S3 FlowFile encryption attributes, more javadocs, better names. This closes apache#3574. Signed-off-by: Andy LoPresto <alopresto@apache.org>
NIFI-4256 Adds AWS S3 FlowFile encryption attributes, more javadocs, better names. This closes apache#3574. Signed-off-by: Andy LoPresto <alopresto@apache.org>
Description of PR
The code in this change-set adds an
S3EncryptionServicecontroller service to manage and apply encryption settings to various S3 processors. Specifically:S3EncryptionServicefor encrypting and decrypting S3 operationsENCRYPTION_SERVICEproperty in theAbstractS3ProcessorclassPutS3ObjectandFetchS3ObjectprocessorsFor all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
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.
Has your PR been rebased against the latest commit within the target branch (typically
master)?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
squashor use--forcewhen pushing to allow for clean monitoring of changes.For code changes:
mvn -Pcontrib-check clean installat the rootnififolder?LICENSEfile, including the mainLICENSEfile undernifi-assembly?NOTICEfile, including the mainNOTICEfile found undernifi-assembly?.displayNamein addition to .name (programmatic access) for each of the new properties?For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.