NIFI-2640 Fixed Windows encrypt-config.bat script to correctly invoke ConfigEncryptionTool. - Resolved failing Windows tests by adding OS-agnostic file permission read/write methods and new regex for different date formats. - Ignored tests that fail on Windows due to file permission and line ending issues. These are captured in NIFI-2644.

This closes #925

Signed-off-by: jpercivall <joepercivall@yahoo.com>

nifi-toolkit/nifi-toolkit-assembly/src/main/resources/bin/encrypt-config.bat

@@ -33,7 +33,7 @@ goto startConfig
 :startConfig
 set LIB_DIR=%~dp0..\classpath;%~dp0..\lib
 
-SET JAVA_PARAMS=-cp %LIB_DIR%\* -Xms12m -Xmx24m %JAVA_ARGS% org.apache.nifi.util.config.ConfigEncryptionTool
+SET JAVA_PARAMS=-cp %LIB_DIR%\* -Xms128m -Xmx256m org.apache.nifi.properties.ConfigEncryptionTool
 
 cmd.exe /C "%JAVA_EXE%" %JAVA_PARAMS% %*
 

nifi-toolkit/nifi-toolkit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy

@@ -242,12 +242,6 @@ class ConfigEncryptionTool {
     private NiFiProperties loadNiFiProperties() throws IOException {
         File niFiPropertiesFile
         if (niFiPropertiesPath && (niFiPropertiesFile = new File(niFiPropertiesPath)).exists()) {
-            String oldNiFiPropertiesPath = System.getProperty(NiFiProperties.PROPERTIES_FILE_PATH)
-            logger.debug("Saving existing NiFiProperties file path ${oldNiFiPropertiesPath}")
-
-            System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, niFiPropertiesFile.absolutePath)
-            logger.debug("Temporarily set NiFiProperties file path to ${niFiPropertiesFile.absolutePath}")
-
             NiFiProperties properties
             try {
                 properties = NiFiPropertiesLoader.withKey(keyHex).load(niFiPropertiesFile)
@@ -258,14 +252,6 @@ class ConfigEncryptionTool {
                     logger.error("Encountered an error", e)
                 }
                 throw new IOException("Cannot load NiFiProperties from [${niFiPropertiesPath}]", e)
-            } finally {
-                // Can't set a system property to null
-                if (oldNiFiPropertiesPath) {
-                    System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, oldNiFiPropertiesPath)
-                } else {
-                    System.clearProperty(NiFiProperties.PROPERTIES_FILE_PATH)
-                }
-                logger.debug("Restored system variable ${NiFiProperties.PROPERTIES_FILE_PATH} to ${oldNiFiPropertiesPath}")
             }
         } else {
             printUsageAndThrow("Cannot load NiFiProperties from [${niFiPropertiesPath}]", ExitCode.ERROR_READING_NIFI_PROPERTIES)

nifi-toolkit/nifi-toolkit-encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy

@@ -19,6 +19,7 @@ package org.apache.nifi.properties
 import ch.qos.logback.classic.spi.LoggingEvent
 import ch.qos.logback.core.AppenderBase
 import org.apache.commons.codec.binary.Hex
+import org.apache.commons.lang3.SystemUtils
 import org.apache.nifi.toolkit.tls.commandLine.CommandLineParseException
 import org.apache.nifi.util.NiFiProperties
 import org.apache.nifi.util.console.TextDevice
@@ -26,8 +27,10 @@ import org.apache.nifi.util.console.TextDevices
 import org.bouncycastle.crypto.generators.SCrypt
 import org.bouncycastle.jce.provider.BouncyCastleProvider
 import org.junit.After
+import org.junit.Assume
 import org.junit.Before
 import org.junit.BeforeClass
+import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import org.junit.contrib.java.lang.system.Assertion
@@ -92,6 +95,38 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
         }
     }
 
+    /**
+     * OS-agnostic method for setting file permissions. On POSIX-compliant systems, accurately sets the provided permissions. On Windows, sets the corresponding permissions for the file owner only.
+     *
+     * @param file the file to modify
+     * @param permissions the desired permissions
+     */
+    private static void setFilePermissions(File file, List<PosixFilePermission> permissions = []) {
+        if (SystemUtils.IS_OS_WINDOWS) {
+            file?.setReadable(permissions.contains(PosixFilePermission.OWNER_READ))
+            file?.setWritable(permissions.contains(PosixFilePermission.OWNER_WRITE))
+            file?.setExecutable(permissions.contains(PosixFilePermission.OWNER_EXECUTE))
+        } else {
+            Files.setPosixFilePermissions(file?.toPath(), permissions as Set)
+        }
+    }
+
+    /**
+     * OS-agnostic method for getting file permissions. On POSIX-compliant systems, accurately gets the existing permissions. On Windows, gets the corresponding permissions for the file owner only.
+     *
+     * @param file the file to check
+     * @return a Set of (String, PosixFilePermissions) containing the permissions
+     */
+    private static Set getFilePermissions(File file) {
+        if (SystemUtils.IS_OS_WINDOWS) {
+           return [file.canRead() ? "OWNER_READ" : "",
+            file.canWrite() ? "OWNER_WRITE" : "",
+            file.canExecute() ? "OWNER_EXECUTE" : ""].findAll { it } as Set
+        } else {
+            return Files.getPosixFilePermissions(file?.toPath())
+        }
+    }
+
     @Test
     void testShouldPrintHelpMessage() {
         // Arrange
@@ -603,14 +638,47 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
     @Test
     void testLoadNiFiPropertiesShouldHandleReadFailure() {
         // Arrange
+        Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+
         File inputPropertiesFile = new File("src/test/resources/nifi_with_sensitive_properties_unprotected.properties")
         File workingFile = new File("tmp_nifi.properties")
         workingFile.delete()
 
         Files.copy(inputPropertiesFile.toPath(), workingFile.toPath())
         // Empty set of permissions
-        Files.setPosixFilePermissions(workingFile.toPath(), [] as Set)
+        setFilePermissions(workingFile, [])
-        logger.info("Set POSIX permissions to ${Files.getPosixFilePermissions(workingFile.toPath())}")
+        logger.info("Set POSIX permissions to ${getFilePermissions(workingFile)}")
+
+        ConfigEncryptionTool tool = new ConfigEncryptionTool()
+        String[] args = ["-n", workingFile.path, "-k", KEY_HEX]
+        tool.parse(args)
+
+        // Act
+        def msg = shouldFail(IOException) {
+            tool.loadNiFiProperties()
+            logger.info("Read nifi.properties")
+        }
+        logger.expected(msg)
+
+        // Assert
+        assert msg == "Cannot load NiFiProperties from [${workingFile.path}]".toString()
+
+        workingFile.deleteOnExit()
+    }
+
+    @Ignore("Setting the Windows file permissions fails in the test harness, so the test does not throw the expected exception")
+    @Test
+    void testLoadNiFiPropertiesShouldHandleReadFailureOnWindows() {
+        // Arrange
+        Assume.assumeTrue("Test only runs on Windows", SystemUtils.IS_OS_WINDOWS)
+
+        File inputPropertiesFile = new File("src/test/resources/nifi_with_sensitive_properties_unprotected.properties")
+        File workingFile = new File("tmp_nifi.properties")
+        workingFile.delete()
+
+        Files.copy(inputPropertiesFile.toPath(), workingFile.toPath())
+        // Empty set of permissions
+        workingFile.setReadable(false)
 
         ConfigEncryptionTool tool = new ConfigEncryptionTool()
         String[] args = ["-n", workingFile.path, "-k", KEY_HEX]
@@ -793,8 +861,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         Files.copy(emptyKeyFile.toPath(), workingFile.toPath())
         // Empty set of permissions
-        Files.setPosixFilePermissions(workingFile.toPath(), [] as Set)
+        setFilePermissions(workingFile, [])
-        logger.info("Set POSIX permissions to ${Files.getPosixFilePermissions(workingFile.toPath())}")
+        logger.info("Set POSIX permissions to ${getFilePermissions(workingFile)}")
 
         ConfigEncryptionTool tool = new ConfigEncryptionTool()
         String[] args = ["-b", workingFile.path, "-k", KEY_HEX]
@@ -822,8 +890,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         Files.copy(emptyKeyFile.toPath(), workingFile.toPath())
         // Read-only set of permissions
-        Files.setPosixFilePermissions(workingFile.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ] as Set)
+        setFilePermissions(workingFile, [PosixFilePermission.OWNER_READ, PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ])
-        logger.info("Set POSIX permissions to ${Files.getPosixFilePermissions(workingFile.toPath())}")
+        logger.info("Set POSIX permissions to ${getFilePermissions(workingFile)}")
 
         ConfigEncryptionTool tool = new ConfigEncryptionTool()
         String[] args = ["-b", workingFile.path, "-k", KEY_HEX]
@@ -886,6 +954,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
     @Test
     void testShouldSerializeNiFiProperties() {
         // Arrange
+        Assume.assumeTrue("Test only runs on *nix because Windows line endings are different", !SystemUtils.IS_OS_WINDOWS)
+
         Properties rawProperties = [key: "value", key2: "value2"] as Properties
         NiFiProperties properties = new StandardNiFiProperties(rawProperties)
         logger.info("Loaded ${properties.size()} properties")
@@ -899,7 +969,9 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         // The serialization could have occurred > 1 second ago, causing a rolling date/time mismatch, so use regex
         // Format -- #Fri Aug 19 16:51:16 PDT 2016
-        String datePattern = /#\w{3} \w{3} \d{2} \d{2}:\d{2}:\d{2} \w{3} \d{4}/
+        // Alternate format -- #Fri Aug 19 16:51:16 GMT-05:00 2016
+        // \u0024 == '$' to avoid escaping
+        String datePattern = /^#\w{3} \w{3} \d{2} \d{2}:\d{2}:\d{2} \w{3}([\-+]\d{2}:\d{2})? \d{4}\u0024/
 
         // One extra line for the date
         assert lines.size() == properties.size() + 1
@@ -1127,8 +1199,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         Files.copy(inputPropertiesFile.toPath(), workingFile.toPath())
         // Read-only set of permissions
-        Files.setPosixFilePermissions(workingFile.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ] as Set)
+        setFilePermissions(workingFile, [PosixFilePermission.OWNER_READ, PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ])
-        logger.info("Set POSIX permissions to ${Files.getPosixFilePermissions(workingFile.toPath())}")
+        logger.info("Set POSIX permissions to ${getFilePermissions(workingFile)}")
 
         ConfigEncryptionTool tool = new ConfigEncryptionTool()
         String[] args = ["-n", inputPropertiesFile.path, "-o", workingFile.path, "-k", KEY_HEX]
@@ -1153,6 +1225,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
     @Test
     void testWriteNiFiPropertiesShouldHandleWriteFailureWhenFileDoesNotExist() {
         // Arrange
+        Assume.assumeTrue("Test only runs on *nix", !SystemUtils.IS_OS_WINDOWS)
+
         File inputPropertiesFile = new File("src/test/resources/nifi_with_sensitive_properties_unprotected.properties")
         File tmpDir = new File("target/tmp/")
         tmpDir.mkdirs()
@@ -1160,8 +1234,8 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
         workingFile.delete()
 
         // Read-only set of permissions
-        Files.setPosixFilePermissions(tmpDir.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ] as Set)
+        setFilePermissions(tmpDir, [PosixFilePermission.OWNER_READ, PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ])
-        logger.info("Set POSIX permissions on parent directory to ${Files.getPosixFilePermissions(tmpDir.toPath())}")
+        logger.info("Set POSIX permissions to ${getFilePermissions(tmpDir)}")
 
         ConfigEncryptionTool tool = new ConfigEncryptionTool()
         String[] args = ["-n", inputPropertiesFile.path, "-o", workingFile.path, "-k", KEY_HEX]
@@ -1181,7 +1255,44 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
         assert msg == "The nifi.properties file at ${workingFile.path} must be writable by the user running this tool".toString()
 
         workingFile.deleteOnExit()
-        Files.setPosixFilePermissions(tmpDir.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE] as Set)
+        setFilePermissions(tmpDir, [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE])
+        tmpDir.deleteOnExit()
+    }
+
+    @Ignore("Setting the Windows file permissions fails in the test harness, so the test does not throw the expected exception")
+    @Test
+    void testWriteNiFiPropertiesShouldHandleWriteFailureWhenFileDoesNotExistOnWindows() {
+        // Arrange
+        Assume.assumeTrue("Test only runs on Windows", SystemUtils.IS_OS_WINDOWS)
+
+        File inputPropertiesFile = new File("src/test/resources/nifi_with_sensitive_properties_unprotected.properties")
+        File tmpDir = new File("target/tmp/")
+        tmpDir.mkdirs()
+        File workingFile = new File("target/tmp/tmp_nifi.properties")
+        workingFile.delete()
+
+        // Read-only set of permissions
+        tmpDir.setWritable(false)
+
+        ConfigEncryptionTool tool = new ConfigEncryptionTool()
+        String[] args = ["-n", inputPropertiesFile.path, "-o", workingFile.path, "-k", KEY_HEX]
+        tool.parse(args)
+        NiFiProperties niFiProperties = tool.loadNiFiProperties()
+        tool.@niFiProperties = niFiProperties
+        logger.info("Loaded ${niFiProperties.size()} properties from ${inputPropertiesFile.path}")
+
+        // Act
+        def msg = shouldFail(IOException) {
+            tool.writeNiFiProperties()
+            logger.info("Wrote to ${workingFile.path}")
+        }
+        logger.expected(msg)
+
+        // Assert
+        assert msg == "The nifi.properties file at ${workingFile.path} must be writable by the user running this tool".toString()
+
+        workingFile.deleteOnExit()
+        setFilePermissions(tmpDir, [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE])
         tmpDir.deleteOnExit()
     }
 
@@ -1192,7 +1303,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         File tmpDir = new File("target/tmp/")
         tmpDir.mkdirs()
-        Files.setPosixFilePermissions(tmpDir.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE] as Set)
+        setFilePermissions(tmpDir, [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE])
 
         File emptyKeyFile = new File("src/test/resources/bootstrap_with_empty_master_key.conf")
         File bootstrapFile = new File("target/tmp/tmp_bootstrap.conf")
@@ -1271,7 +1382,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         File tmpDir = new File("target/tmp/")
         tmpDir.mkdirs()
-        Files.setPosixFilePermissions(tmpDir.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE] as Set)
+        setFilePermissions(tmpDir, [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE])
 
         File emptyKeyFile = new File("src/test/resources/bootstrap_with_empty_master_key.conf")
         File bootstrapFile = new File("target/tmp/tmp_bootstrap.conf")
@@ -1353,7 +1464,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
 
         File tmpDir = new File("target/tmp/")
         tmpDir.mkdirs()
-        Files.setPosixFilePermissions(tmpDir.toPath(), [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE] as Set)
+        setFilePermissions(tmpDir, [PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE])
 
         File emptyKeyFile = new File("src/test/resources/bootstrap_with_empty_master_key.conf")
         File bootstrapFile = new File("target/tmp/tmp_bootstrap.conf")