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 23:35:55 UTC
[GitHub] [nifi] thenatog opened a new pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
thenatog opened a new pull request #4715:
URL: https://github.com/apache/nifi/pull/4715
…d tests.
NIFI-6999 - Slight change to test to check for WARN message.
NIFI-6999 - Removed very large flow file and test that uses it. This test ran for about 2 minutes so was excessive to keep in. The other changed tests to handle streams proves the functionality. A large file can be used on the command line to manually test large flow files. Some other cleanup.
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
#### Description of PR
_Enables X functionality; fixes bug NIFI-YYYY._
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?
- [x] 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] exceptionfactory commented on pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#issuecomment-760194979
Thanks for making the changes @thenatog. Looks good!
----------------------------------------------------------------
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] thenatog commented on pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#issuecomment-764911205
Will merge.
----------------------------------------------------------------
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] thenatog closed pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog closed pull request #4715:
URL: https://github.com/apache/nifi/pull/4715
----------------------------------------------------------------
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] thenatog commented on pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#issuecomment-740937392
Updated PR to remove junk comments and improve readability etc. as per your recommendation.
----------------------------------------------------------------
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] thenatog commented on a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r538839172
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4062,158 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
- // Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated ${workingFlowXmlFile.path} was: ${decryptedValue}")
+ assert decryptedValue == PASSWORD || decryptedValue == ANOTHER_PASSWORD
+ }
+ }
+ })
+
+ // Act
+ ConfigEncryptionTool.main(args)
+ logger.info("Invoked #main with ${args.join(" ")}")
+
+ // Assert
+
+ // Assertions defined above
Review comment:
These comments relate to formatting tests in a format of Arrange, Act, and Assert sections. The way the test is designed in this case changes the layout of how that format looks, but I guess the intention is the same. I can remove these comments though.
----------------------------------------------------------------
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 a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r548222438
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4062,158 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
- // Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated ${workingFlowXmlFile.path} was: ${decryptedValue}")
+ assert decryptedValue == PASSWORD || decryptedValue == ANOTHER_PASSWORD
+ }
+ }
+ })
+
+ // Act
+ ConfigEncryptionTool.main(args)
+ logger.info("Invoked #main with ${args.join(" ")}")
+
+ // Assert
+
+ // Assertions defined above
Review comment:
Although these same comments are used in other methods, I recommend removing them here since they do not directly reference any lines of code.
----------------------------------------------------------------
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] thenatog commented on pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#issuecomment-740259543
This can be tested with the following files:
nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/bootstrap.conf
nifi-toolkit/nifi-toolkit-encrypt-config/src/test/resources/nifi_default.properties
and the attached flow.xml.gz file. [large-flow.xml.gz](https://github.com/apache/nifi/files/5655962/large-flow.xml.gz)
The attached file is 10mb gzipped, and ~1GB unzipped. It has a processor with an encrypted field repeated many times:
enc{2032416987A00D9FCD757528D7AE609D7E793CA5F956641DB53E14CDB9BFCD4037B73AC705CD3F5C1C1BDE18B8D7B281} whose value is "thisIsABadPassword"
The command to run is:
./encrypt-config.sh -n conf/nifi.properties -f conf/large-flow.xml.gz -b conf/bootstrap.conf -x -v -s aNewPassword
----------------------------------------------------------------
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] thenatog closed pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog closed pull request #4715:
URL: https://github.com/apache/nifi/pull/4715
----------------------------------------------------------------
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] thenatog commented on pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#issuecomment-759619902
@exceptionfactory Updated with latest request. Let me know if there's anything else.
----------------------------------------------------------------
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] thenatog commented on pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#issuecomment-764911205
Will merge.
----------------------------------------------------------------
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] thenatog commented on a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r538839172
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4062,158 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
- // Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated ${workingFlowXmlFile.path} was: ${decryptedValue}")
+ assert decryptedValue == PASSWORD || decryptedValue == ANOTHER_PASSWORD
+ }
+ }
+ })
+
+ // Act
+ ConfigEncryptionTool.main(args)
+ logger.info("Invoked #main with ${args.join(" ")}")
+
+ // Assert
+
+ // Assertions defined above
Review comment:
These comments relate to formatting tests in a format of Arrange, Act, and Assert sections. The way the test is designed in this case changes the layout of how that format looks, but I guess the intention is the same. Looks like most of the tests have these comments in there. Should I remove?
----------------------------------------------------------------
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] thenatog commented on a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
thenatog commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r538836447
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -64,8 +68,8 @@ class ConfigEncryptionTool {
public String outputLoginIdentityProvidersPath
public String authorizersPath
public String outputAuthorizersPath
- public String flowXmlPath
- public String outputFlowXmlPath
+ public static flowXmlPath
+ public static outputFlowXmlPath
Review comment:
These were changed to static to allow static access in static main() for calling loadFlowXml(flowXmlPath). I changed this slightly to take the path as a parameter to allow easier testing for the loadFlowXml method. Not sure how successful it was though.
----------------------------------------------------------------
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 a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r548225218
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -657,18 +661,16 @@ class ConfigEncryptionTool {
/**
* Loads the flow definition from the provided file path, handling the GZIP file compression. Unlike {@link #loadLoginIdentityProviders()} this method does not decrypt the content (for performance and separation of concern reasons).
*
- * @return the file content
+ * @param The path to the XML file
+ * @return The file content
* @throw IOException if the flow.xml.gz file cannot be read
*/
- private String loadFlowXml() throws IOException {
+ private InputStream loadFlowXml(String flowXmlPath) throws IOException {
Review comment:
The parameter `flowXmlPath` should probably be renamed since it shadows the static `flowXmlPath` variable declared in the class.
----------------------------------------------------------------
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 a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r538391803
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -64,8 +68,8 @@ class ConfigEncryptionTool {
public String outputLoginIdentityProvidersPath
public String authorizersPath
public String outputAuthorizersPath
- public String flowXmlPath
- public String outputFlowXmlPath
+ public static String flowXmlPath
+ public static String outputFlowXmlPath
Review comment:
Is there a reason for making these static?
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -93,6 +97,7 @@ class ConfigEncryptionTool {
private boolean handlingFlowXml = false
private boolean ignorePropertiesFiles = false
private boolean translatingCli = false
+ private boolean isflowXmlGZipped = false
Review comment:
Instead of setting and track this variable, is it possible to check the class type of the InputStream returned from loadFlowXml? That would avoid introducing another state variable into the class.
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -3806,19 +3812,23 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
// Verify the flow definition
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
- verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz content changed
assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+
+ // TODO: Nathan needs to change this findAll to search the updated flow xml file and stream search it.
+ def updatedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
+ //def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
Review comment:
Recommend removing this comment since the implementation has changed
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -3911,19 +3921,20 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
// Verify the flow definition
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
- verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream migratedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz cipher texts did change (new salt)
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ assert migratedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ def migratedFlowCipherTexts = findFieldsInStream(migratedFlowXmlContent, WFXCTR)
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated " + workingFlowXmlFile.path + " was: " + decryptedValue)
Review comment:
Recommend changing this log to use either a Groovy string or parameterized log string.
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -810,21 +815,52 @@ class ConfigEncryptionTool {
Cipher encryptCipher = generateFlowEncryptionCipher(newFlowPassword, encryptionSalt, newAlgorithm, newProvider)
int elementCount = 0
-
- // Scan the XML content and identify every encrypted element, decrypt it, and replace it with the re-encrypted value
- String migratedFlowXmlContent = flowXmlContent.replaceAll(WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX) { String wrappedCipherText ->
- String plaintext = decryptFlowElement(wrappedCipherText, existingFlowPassword, existingAlgorithm, existingProvider)
- byte[] cipherBytes = encryptCipher.doFinal(plaintext.bytes)
- byte[] saltAndCipherBytes = concatByteArrays(encryptionSalt, cipherBytes)
- elementCount++
- "enc{${Hex.encodeHex(saltAndCipherBytes)}}"
+ File tempFlowXmlFile = new File(getTemporaryFlowXmlFile(outputFlowXmlPath).toString())
+ BufferedWriter tempFlowXmlWriter = getFileOutputStream(tempFlowXmlFile, isflowXmlGZipped)
+
+ // Scan through XML content as a stream, decrypt and re-encrypt fields with a new flow password
+ final BufferedReader reader = new BufferedReader(new InputStreamReader(flowXmlContent))
+ String line;
+
+ while((line = reader.readLine()) != null) {
+ def matcher = line =~ WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX
+ if(matcher.find()) {
+ String plaintext = decryptFlowElement(matcher.getAt(0), existingFlowPassword, existingAlgorithm, existingProvider)
+ byte[] cipherBytes = encryptCipher.doFinal(plaintext.bytes)
+ byte[] saltAndCipherBytes = concatByteArrays(encryptionSalt, cipherBytes)
+ elementCount++
+ tempFlowXmlWriter.writeLine(line.replaceFirst(WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX, "enc{${Hex.encodeHex(saltAndCipherBytes)}}"))
+ } else {
+ tempFlowXmlWriter.writeLine(line)
+ }
}
+ tempFlowXmlWriter.flush()
+ tempFlowXmlWriter.close()
+
+ // Overwrite the original flow file with the migrated flow file
+ Files.move(tempFlowXmlFile.toPath(), Paths.get(outputFlowXmlPath), StandardCopyOption.ATOMIC_MOVE)
if (isVerbose) {
logger.info("Decrypted and re-encrypted ${elementCount} elements for flow.xml.gz")
}
- migratedFlowXmlContent
+ loadFlowXml(outputFlowXmlPath)
+ }
+
+ private BufferedWriter getFileOutputStream(File outputFlowXmlPath, boolean isFileGZipped) {
+ BufferedWriter migratedFlowXml
+ if(isFileGZipped) {
+ migratedFlowXml = new BufferedWriter(new OutputStreamWriter(new GZIPOutputStream(new FileOutputStream(outputFlowXmlPath))));
+ } else {
+ migratedFlowXml = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(outputFlowXmlPath)));
+ }
+ migratedFlowXml
+ }
+
+ // Create a temporary output file we can write the stream to
+ private Path getTemporaryFlowXmlFile(String originalOutputFlowXmlPath) {
+ String migratedFileName = "migrated-".concat(Paths.get(originalOutputFlowXmlPath).getFileName().toString())
Review comment:
For improved readability, it may be better to declare a separate variable for the original file path, then declare migratedFileName:
```suggestion
String migratedFileName = "migrated-${originalFileName}"
```
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -810,21 +815,52 @@ class ConfigEncryptionTool {
Cipher encryptCipher = generateFlowEncryptionCipher(newFlowPassword, encryptionSalt, newAlgorithm, newProvider)
int elementCount = 0
-
- // Scan the XML content and identify every encrypted element, decrypt it, and replace it with the re-encrypted value
- String migratedFlowXmlContent = flowXmlContent.replaceAll(WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX) { String wrappedCipherText ->
- String plaintext = decryptFlowElement(wrappedCipherText, existingFlowPassword, existingAlgorithm, existingProvider)
- byte[] cipherBytes = encryptCipher.doFinal(plaintext.bytes)
- byte[] saltAndCipherBytes = concatByteArrays(encryptionSalt, cipherBytes)
- elementCount++
- "enc{${Hex.encodeHex(saltAndCipherBytes)}}"
+ File tempFlowXmlFile = new File(getTemporaryFlowXmlFile(outputFlowXmlPath).toString())
+ BufferedWriter tempFlowXmlWriter = getFileOutputStream(tempFlowXmlFile, isflowXmlGZipped)
Review comment:
Instead of passing isflowXmlGZipped, the Boolean could be set using an instanceof check
```suggestion
BufferedWriter tempFlowXmlWriter = getFileOutputStream(tempFlowXmlFile, flowXmlContent instanceof GZipInputStream)
```
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -3806,19 +3812,23 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
// Verify the flow definition
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
- verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz content changed
assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+
+ // TODO: Nathan needs to change this findAll to search the updated flow xml file and stream search it.
Review comment:
Should this comment be removed?
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -3728,7 +3734,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
// TODO: Test reading sensitive props key from console
// TODO: All combo scenarios
@Test
- void testShouldPerformFullOperationOnFlowXmlWithoutEncryptedNiFiProperties() {
+ void testShouldPerformFullOperationOnFlowXmlWithoutEncryptedNiFiPropertiesa() {
Review comment:
Is this method name change intentional?
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -810,21 +815,52 @@ class ConfigEncryptionTool {
Cipher encryptCipher = generateFlowEncryptionCipher(newFlowPassword, encryptionSalt, newAlgorithm, newProvider)
int elementCount = 0
-
- // Scan the XML content and identify every encrypted element, decrypt it, and replace it with the re-encrypted value
- String migratedFlowXmlContent = flowXmlContent.replaceAll(WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX) { String wrappedCipherText ->
- String plaintext = decryptFlowElement(wrappedCipherText, existingFlowPassword, existingAlgorithm, existingProvider)
- byte[] cipherBytes = encryptCipher.doFinal(plaintext.bytes)
- byte[] saltAndCipherBytes = concatByteArrays(encryptionSalt, cipherBytes)
- elementCount++
- "enc{${Hex.encodeHex(saltAndCipherBytes)}}"
+ File tempFlowXmlFile = new File(getTemporaryFlowXmlFile(outputFlowXmlPath).toString())
+ BufferedWriter tempFlowXmlWriter = getFileOutputStream(tempFlowXmlFile, isflowXmlGZipped)
+
+ // Scan through XML content as a stream, decrypt and re-encrypt fields with a new flow password
+ final BufferedReader reader = new BufferedReader(new InputStreamReader(flowXmlContent))
+ String line;
+
+ while((line = reader.readLine()) != null) {
+ def matcher = line =~ WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX
+ if(matcher.find()) {
+ String plaintext = decryptFlowElement(matcher.getAt(0), existingFlowPassword, existingAlgorithm, existingProvider)
+ byte[] cipherBytes = encryptCipher.doFinal(plaintext.bytes)
+ byte[] saltAndCipherBytes = concatByteArrays(encryptionSalt, cipherBytes)
+ elementCount++
+ tempFlowXmlWriter.writeLine(line.replaceFirst(WRAPPED_FLOW_XML_CIPHER_TEXT_REGEX, "enc{${Hex.encodeHex(saltAndCipherBytes)}}"))
+ } else {
+ tempFlowXmlWriter.writeLine(line)
+ }
}
+ tempFlowXmlWriter.flush()
+ tempFlowXmlWriter.close()
+
+ // Overwrite the original flow file with the migrated flow file
+ Files.move(tempFlowXmlFile.toPath(), Paths.get(outputFlowXmlPath), StandardCopyOption.ATOMIC_MOVE)
if (isVerbose) {
logger.info("Decrypted and re-encrypted ${elementCount} elements for flow.xml.gz")
}
- migratedFlowXmlContent
+ loadFlowXml(outputFlowXmlPath)
+ }
+
+ private BufferedWriter getFileOutputStream(File outputFlowXmlPath, boolean isFileGZipped) {
+ BufferedWriter migratedFlowXml
+ if(isFileGZipped) {
+ migratedFlowXml = new BufferedWriter(new OutputStreamWriter(new GZIPOutputStream(new FileOutputStream(outputFlowXmlPath))));
+ } else {
+ migratedFlowXml = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(outputFlowXmlPath)));
Review comment:
Recommend adjusting this method to reduce some duplication along the lines of the following, also removing end of line semicolons:
```suggestion
OutputStream flowOutputStream = new FileOutputStream(outputFlowXmlPath)
if (isFileGZipped) {
flowOutputStream = new GZIPOutputStream(flowOutputStream)
}
return new BufferedWriter(new OutputStreamWriter(flowOutputStream))
```
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -5172,6 +5306,26 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
!diffSimilar.hasDifferences()
}
+ // TODO: Nathan make this method stream in a file and search it for the pattern.
+ static Set<String> findFieldsInStream(InputStream fileInputStream, String pattern) {
+ Set<String> fieldsFound = new HashSet<String>()
+ Reader reader = new BufferedReader(new InputStreamReader(fileInputStream))
+ String line
+ while((line = reader.readLine()) != null) {
+ def matcher = line =~ pattern
+ if(matcher.find()) {
+ fieldsFound.add(matcher.getAt(0))
+ }
+ }
+
+// fileInputStream.eachLine { line ->
+// def matcher = line =~ pattern
+// if(matcher.find()) {
+// fieldsFound.add(matcher.getAt(0))
+// } }
Review comment:
This comment block should be removed
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4064,164 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ // TODO: Nathan assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+
+ // TODO: Nathan needs to change this findAll to search the updated flow xml file and stream search it.
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ //def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated " + workingFlowXmlFile.path + " was: " + decryptedValue)
Review comment:
Recommend changing to parameterized log or Groovy string with variables.
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4064,164 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ // TODO: Nathan assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+
+ // TODO: Nathan needs to change this findAll to search the updated flow xml file and stream search it.
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ //def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated " + workingFlowXmlFile.path + " was: " + decryptedValue)
+ assert decryptedValue == PASSWORD || decryptedValue == ANOTHER_PASSWORD
+ }
+ }
+ })
+
+ // Act
+ ConfigEncryptionTool.main(args)
+ logger.info("Invoked #main with ${args.join(" ")}")
+
+ // Assert
+
+ // Assertions defined above
+ }
+
+ /**
+ * In this scenario, the nifi.properties file has a sensitive key value which is already encrypted. The goal is to provide a new provide a new sensitive key value, perform the migration of the flow.xml.gz, and update nifi.properties with a new encrypted sensitive key value without modifying any other nifi.properties values.
+ */
+ @Test
+ void testShouldPerformFullOperationOnAFlowXmlWithPreviouslyEncryptedNiFiProperties() {
+ // Arrange
+ exit.expectSystemExitWithStatus(0)
+
+ File tmpDir = setupTmpDir()
+
+ File passwordKeyFile = new File("src/test/resources/bootstrap_with_root_key_password_128.conf")
+ File bootstrapFile = new File("target/tmp/tmp_bootstrap.conf")
+ bootstrapFile.delete()
+
+ Files.copy(passwordKeyFile.toPath(), bootstrapFile.toPath())
+ final List<String> originalBootstrapLines = bootstrapFile.readLines()
+ String originalKeyLine = originalBootstrapLines.find {
+ it.startsWith(ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX)
+ }
+ final String EXPECTED_KEY_LINE = ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX + PASSWORD_KEY_HEX_128
+ logger.info("Original key line from bootstrap.conf: ${originalKeyLine}")
+ assert originalKeyLine == ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX + PASSWORD_KEY_HEX_128
+
+ // Not "handling" NFP, so update in place (not source test resource)
+ String niFiPropertiesTemplatePath = "src/test/resources/nifi_with_few_sensitive_properties_protected_aes_password_128.properties"
+ File niFiPropertiesFile = new File(niFiPropertiesTemplatePath)
+
+ File workingNiFiPropertiesFile = new File("target/tmp/tmp-nifi.properties")
+ workingNiFiPropertiesFile.delete()
+ Files.copy(niFiPropertiesFile.toPath(), workingNiFiPropertiesFile.toPath())
+
+ // Use a flow definition that was encrypted with the hard-coded default SP key
+ File flowXmlFile = new File("src/test/resources/flow.xml.gz")
+ File workingFlowXmlFile = new File("target/tmp/tmp-flow.xml.gz")
+ workingFlowXmlFile.delete()
+ Files.copy(flowXmlFile.toPath(), workingFlowXmlFile.toPath())
+
+ // Get the original ciphered fields to compare later
+ def verifyTool = new ConfigEncryptionTool()
+ verifyTool.isVerbose = true
+ def originalFlowCipherTexts = findFieldsInStream(verifyTool.loadFlowXml(flowXmlFile.path), WFXCTR)
+ final int CIPHER_TEXT_COUNT = originalFlowCipherTexts.size()
+
+ // Load both the encrypted and decrypted properties to compare later
+ NiFiPropertiesLoader niFiPropertiesLoader = NiFiPropertiesLoader.withKey(PASSWORD_KEY_HEX_128)
+ NiFiProperties inputProperties = niFiPropertiesLoader.load(workingNiFiPropertiesFile)
+ logger.info("Loaded ${inputProperties.size()} properties from input file")
+ ProtectedNiFiProperties protectedInputProperties = new ProtectedNiFiProperties(inputProperties)
+ def originalSensitiveValues = protectedInputProperties.getSensitivePropertyKeys().collectEntries { String key -> [(key): protectedInputProperties.getProperty(key)] }
+ logger.info("Original sensitive values: ${originalSensitiveValues}")
+
+
+ final String SENSITIVE_PROTECTION_KEY = ProtectedNiFiProperties.getProtectionKey(NiFiProperties.SENSITIVE_PROPS_KEY)
+ ProtectedNiFiProperties encryptedProperties = niFiPropertiesLoader.readProtectedPropertiesFromDisk(workingNiFiPropertiesFile)
+ def originalEncryptedValues = encryptedProperties.getSensitivePropertyKeys().collectEntries { String key -> [(key): encryptedProperties.getProperty(key)] }
+ logger.info("Original encrypted values: ${originalEncryptedValues}")
+ String originalSensitiveKeyProtectionScheme = encryptedProperties.getProperty(SENSITIVE_PROTECTION_KEY)
+ logger.info("Sensitive property key originally protected with ${originalSensitiveKeyProtectionScheme}")
+
+ String newFlowPassword = FLOW_PASSWORD
+
+ // Bootstrap path must be provided to decrypt nifi.properties to get SP key
+ String[] args = ["-n", workingNiFiPropertiesFile.path, "-f", workingFlowXmlFile.path, "-b", bootstrapFile.path, "-x", "-v", "-s", newFlowPassword]
+
+ exit.checkAssertionAfterwards(new Assertion() {
+ void checkAssertion() {
+ final List<String> updatedPropertiesLines = workingNiFiPropertiesFile.readLines()
+ logger.info("Updated nifi.properties:")
+ logger.info("\n" * 2 + updatedPropertiesLines.join("\n"))
+
+ AESSensitivePropertyProvider spp = new AESSensitivePropertyProvider(PASSWORD_KEY_HEX_128)
+
+ // Check that the output values for everything is the same except the sensitive props key
+ NiFiProperties updatedProperties = new NiFiPropertiesLoader().readProtectedPropertiesFromDisk(workingNiFiPropertiesFile)
+ assert updatedProperties.size() == inputProperties.size()
+ String newSensitivePropertyKey = updatedProperties.getProperty(NiFiProperties.SENSITIVE_PROPS_KEY)
+
+ // Check that the encrypted value changed
+ assert newSensitivePropertyKey != originalSensitiveValues.get(NiFiProperties.SENSITIVE_PROPS_KEY)
+
+ // Check that the decrypted value is the new password
+ assert spp.unprotect(newSensitivePropertyKey) == newFlowPassword
+
+ // Check that all other values stayed the same
+ originalEncryptedValues.every { String key, String originalValue ->
+ if (key != NiFiProperties.SENSITIVE_PROPS_KEY) {
+ assert updatedProperties.getProperty(key) == originalValue
+ }
+ }
+
+ // Check that all other (decrypted) values stayed the same
+ originalSensitiveValues.every { String key, String originalValue ->
+ if (key != NiFiProperties.SENSITIVE_PROPS_KEY) {
+ assert spp.unprotect(updatedProperties.getProperty(key)) == originalValue
+ }
+ }
+
+ // Check that the protection scheme did not change
+ String sensitiveKeyProtectionScheme = updatedProperties.getProperty(SENSITIVE_PROTECTION_KEY)
+ logger.info("Sensitive property key currently protected with ${sensitiveKeyProtectionScheme}")
+ assert sensitiveKeyProtectionScheme == originalSensitiveKeyProtectionScheme
+
+ // Check that bootstrap.conf did not change
+ final List<String> updatedBootstrapLines = bootstrapFile.readLines()
+ String updatedKeyLine = updatedBootstrapLines.find {
+ it.startsWith(ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX)
+ }
+ logger.info("Updated key line: ${updatedKeyLine}")
+
+ assert updatedKeyLine == EXPECTED_KEY_LINE
+ assert originalBootstrapLines.size() == updatedBootstrapLines.size()
+
+ // Verify the flow definition
+ verifyTool = new ConfigEncryptionTool()
+ verifyTool.isVerbose = true
+ InputStream migratedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
+
+ def migratedFlowCipherTexts = findFieldsInStream(migratedFlowXmlContent, WFXCTR)
+ logger.info("Migrated flow cipher texts for: " + workingFlowXmlFile.path)
+ //def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
Review comment:
This commented line should be removed
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4064,164 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ // TODO: Nathan assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+
+ // TODO: Nathan needs to change this findAll to search the updated flow xml file and stream search it.
Review comment:
Should these comments be removed?
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4064,164 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
// Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ // TODO: Nathan assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+
+ // TODO: Nathan needs to change this findAll to search the updated flow xml file and stream search it.
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ //def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated " + workingFlowXmlFile.path + " was: " + decryptedValue)
+ assert decryptedValue == PASSWORD || decryptedValue == ANOTHER_PASSWORD
+ }
+ }
+ })
+
+ // Act
+ ConfigEncryptionTool.main(args)
+ logger.info("Invoked #main with ${args.join(" ")}")
+
+ // Assert
+
+ // Assertions defined above
Review comment:
These comments should be removed
----------------------------------------------------------------
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 a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4715:
URL: https://github.com/apache/nifi/pull/4715#discussion_r538821132
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -4053,18 +4062,158 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
def verifyTool = new ConfigEncryptionTool()
verifyTool.isVerbose = true
verifyTool.flowXmlPath = workingFlowXmlFile.path
- String updatedFlowXmlContent = verifyTool.loadFlowXml()
+ InputStream updatedFlowXmlContent = verifyTool.loadFlowXml(workingFlowXmlFile.path)
- // Check that the flow.xml.gz content changed
- assert updatedFlowXmlContent != ORIGINAL_FLOW_XML_CONTENT
+ def migratedFlowCipherTexts = findFieldsInStream(updatedFlowXmlContent, WFXCTR)
// Verify that the cipher texts decrypt correctly
logger.info("Original flow.xml.gz cipher texts: ${originalFlowCipherTexts}")
- def flowCipherTexts = updatedFlowXmlContent.findAll(WFXCTR)
- logger.info("Updated flow.xml.gz cipher texts: ${flowCipherTexts}")
- assert flowCipherTexts.size() == CIPHER_TEXT_COUNT
- flowCipherTexts.every {
- assert ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword) == "thisIsABadPassword"
+ logger.info("Updated flow.xml.gz cipher texts: ${migratedFlowCipherTexts}")
+ assert migratedFlowCipherTexts.size() == CIPHER_TEXT_COUNT
+ migratedFlowCipherTexts.each {
+ String decryptedValue = ConfigEncryptionTool.decryptFlowElement(it, newFlowPassword)
+ logger.info("Decrypted value of migrated ${workingFlowXmlFile.path} was: ${decryptedValue}")
+ assert decryptedValue == PASSWORD || decryptedValue == ANOTHER_PASSWORD
+ }
+ }
+ })
+
+ // Act
+ ConfigEncryptionTool.main(args)
+ logger.info("Invoked #main with ${args.join(" ")}")
+
+ // Assert
+
+ // Assertions defined above
Review comment:
It looks like this comment and the one above should be removed.
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
##########
@@ -287,7 +291,10 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
// Assert
assert !TestAppender.events.isEmpty()
- assert TestAppender.events.first().message =~ "The source nifi.properties and destination nifi.properties are identical \\[.*\\] so the original will be overwritten"
+ assert TestAppender.events.stream().any() {
+ it.message =~ "The source nifi.properties and destination nifi.properties are identical \\[.*\\] so the original will be overwritten"
+ }
+ //assert TestAppender.events.first().message =~ "The source nifi.properties and destination nifi.properties are identical \\[.*\\] so the original will be overwritten"
Review comment:
It looks like this commented line should be removed.
##########
File path: nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
##########
@@ -64,8 +68,8 @@ class ConfigEncryptionTool {
public String outputLoginIdentityProvidersPath
public String authorizersPath
public String outputAuthorizersPath
- public String flowXmlPath
- public String outputFlowXmlPath
+ public static flowXmlPath
+ public static outputFlowXmlPath
Review comment:
Is there a reason these values are static?
----------------------------------------------------------------
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