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/08 14:16:25 UTC

[GitHub] [nifi] exceptionfactory commented on a change in pull request #4715: NIFI-6999 - Made changes to load flow.xml files using streams. Update…

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