Skip to content

Commit

Permalink
NIFI-2655 Implemented logic check for missing -n or -l arguments.
Browse files Browse the repository at this point in the history 
Added and updated unit tests.
Cleaned up unnecessary and commented code.
Removed ignored unit tests.
Cleaned up commented code.

This closes apache#1262.

Signed-off-by: Andy LoPresto <alopresto@apache.org>
  • Loading branch information
@alopresto @dstreev
alopresto authored and dstreev committed Dec 9, 2016
1 parent bf36c7f commit 5fc13d9
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 103 deletions.
22 changes: 3 additions & 19 deletions ...kit-encrypt-config/src/main/groovy/org/apache/nifi/properties/ConfigEncryptionTool.groovy
View file
Expand Up @@ -284,10 +284,9 @@ class ConfigEncryptionTool {
logger.info("(dest) flow.xml.gz: \t\t\t\t\t${outputFlowXmlPath}")
}

// TODO: Implement in NIFI-2655
// if (!commandLine.hasOption(NIFI_PROPERTIES_ARG) && !commandLine.hasOption(LOGIN_IDENTITY_PROVIDERS_ARG)) {
// printUsageAndThrow("One of '-n'/'--${NIFI_PROPERTIES_ARG}' or '-l'/'--${LOGIN_IDENTITY_PROVIDERS_ARG}' must be provided", ExitCode.INVALID_ARGS)
// }
if (!commandLine.hasOption(NIFI_PROPERTIES_ARG) && !commandLine.hasOption(LOGIN_IDENTITY_PROVIDERS_ARG) && !commandLine.hasOption(DO_NOT_ENCRYPT_NIFI_PROPERTIES_ARG)) {
printUsageAndThrow("One or both of '-n'/'--${NIFI_PROPERTIES_ARG}' or '-l'/'--${LOGIN_IDENTITY_PROVIDERS_ARG}' must be provided unless '-x'/--'${DO_NOT_ENCRYPT_NIFI_PROPERTIES_ARG}' is specified", ExitCode.INVALID_ARGS)
}

if (commandLine.hasOption(MIGRATION_ARG)) {
migration = true
Expand Down Expand Up @@ -1037,21 +1036,6 @@ class ConfigEncryptionTool {
fileToWrite && ((!fileToWrite.exists() && fileToWrite.absoluteFile.parentFile.canWrite()) || (fileToWrite.exists() && fileToWrite.canWrite()))
}

private static String determineDefaultBootstrapConfPath() {
String niFiToolkitPath = System.getenv(NIFI_TOOLKIT_HOME) ?: ""
"${niFiToolkitPath ? niFiToolkitPath + "/" : ""}conf/bootstrap.conf"
}

private static String determineDefaultNiFiPropertiesPath() {
String niFiToolkitPath = System.getenv(NIFI_TOOLKIT_HOME) ?: ""
"${niFiToolkitPath ? niFiToolkitPath + "/" : ""}conf/nifi.properties"
}

private static String determineDefaultLoginIdentityProvidersPath() {
String niFiToolkitPath = System.getenv(NIFI_TOOLKIT_HOME) ?: ""
"${niFiToolkitPath ? niFiToolkitPath + "/" : ""}conf/login-identity-providers.xml"
}

private static String deriveKeyFromPassword(String password) {
password = password?.trim()
if (!password || password.length() < MIN_PASSWORD_LENGTH) {
Expand Down
127 changes: 43 additions & 84 deletions ...encrypt-config/src/test/groovy/org/apache/nifi/properties/ConfigEncryptionToolTest.groovy
View file
Expand Up @@ -207,7 +207,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {

// Act
flags.each { String arg ->
tool.parse([arg, bootstrapPath] as String[])
tool.parse([arg, bootstrapPath, "-n", "nifi.properties"] as String[])
logger.info("Parsed bootstrap.conf location: ${tool.bootstrapConfPath}")

// Assert
Expand All @@ -233,22 +233,6 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
}
}

// TODO: Remove as part of NIFI-2655
@Ignore("Remove as part of NIFI-2655")
@Test
void testParseShouldPopulateDefaultNiFiPropertiesArgument() {
// Arrange
String niFiPropertiesPath = "conf/nifi.properties"
ConfigEncryptionTool tool = new ConfigEncryptionTool()

// Act
tool.parse([] as String[])
logger.info("Parsed nifi.properties location: ${tool.niFiPropertiesPath}")

// Assert
assert new File(tool.niFiPropertiesPath).getPath() == new File(niFiPropertiesPath).getPath()
}

@Test
void testShouldParseOutputNiFiPropertiesArgument() {
// Arrange
Expand All @@ -266,22 +250,6 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
}
}

// TODO: Remove as part of NIFI-2655
@Ignore("Remove as part of NIFI-2655")
@Test
void testParseShouldPopulateDefaultOutputNiFiPropertiesArgument() {
// Arrange
String niFiPropertiesPath = "conf/nifi.properties"
ConfigEncryptionTool tool = new ConfigEncryptionTool()

// Act
tool.parse([] as String[])
logger.info("Parsed output nifi.properties location: ${tool.outputNiFiPropertiesPath}")

// Assert
assert new File(tool.outputNiFiPropertiesPath).getPath() == new File(niFiPropertiesPath).getPath()
}

@Test
void testParseShouldWarnIfNiFiPropertiesWillBeOverwritten() {
// Arrange
Expand Down Expand Up @@ -316,22 +284,6 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
}
}

// TODO: Remove as part of NIFI-2655
@Ignore("Remove as part of NIFI-2655")
@Test
void testParseShouldPopulateDefaultLoginIdentityProvidersArgument() {
// Arrange
String loginIdentityProvidersPath = "conf/login-identity-providers.xml"
ConfigEncryptionTool tool = new ConfigEncryptionTool()

// Act
tool.parse([] as String[])
logger.info("Parsed login-identity-providers.xml location: ${tool.loginIdentityProvidersPath}")

// Assert
assert new File(tool.loginIdentityProvidersPath).getPath() == new File(loginIdentityProvidersPath).getPath()
}

@Test
void testShouldParseOutputLoginIdentityProvidersArgument() {
// Arrange
Expand All @@ -349,22 +301,6 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
}
}

// TODO: Remove as part of NIFI-2655
@Ignore("Remove as part of NIFI-2655")
@Test
void testParseShouldPopulateDefaultOutputLoginIdentityProvidersArgument() {
// Arrange
String loginIdentityProvidersPath = "conf/login-identity-providers.xml"
ConfigEncryptionTool tool = new ConfigEncryptionTool()

// Act
tool.parse([] as String[])
logger.info("Parsed output login-identity-providers.xml location: ${tool.outputLoginIdentityProvidersPath}")

// Assert
assert new File(tool.outputLoginIdentityProvidersPath).getPath() == new File(loginIdentityProvidersPath).getPath()
}

@Test
void testParseShouldWarnIfLoginIdentityProvidersWillBeOverwritten() {
// Arrange
Expand All @@ -391,7 +327,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {

// Act
flags.each { String arg ->
tool.parse([arg, KEY_HEX] as String[])
tool.parse([arg, KEY_HEX, "-n", "nifi.properties"] as String[])
logger.info("Parsed key: ${tool.keyHex}")

// Assert
Expand All @@ -406,14 +342,47 @@ class ConfigEncryptionToolTest extends GroovyTestCase {

// Act
def msg = shouldFail {
tool.parse("-m -e oldKey -w oldPassword".split(" ") as String[])
tool.parse("-m -n nifi.properties -e oldKey -w oldPassword".split(" ") as String[])
}
logger.expected(msg)

// Assert
assert msg =~ "Only one of '-w'/'--oldPassword' and '-e'/'--oldKey' can be used"
}

@Test
void testParseShouldFailIfNiFiPropertiesAndLoginIdentityProviderBothMissing() {
// Arrange
ConfigEncryptionTool tool = new ConfigEncryptionTool()

def invalidArgs = [
"-v -m",
"-v -s password",
"-n",
"-l",
"-o output-nifi.properties -i output-login-identity-providers.xml",
"-f flow.xml.gz",
]

final String NO_NFP_OR_LIP = "One or both of '-n'/'--${ConfigEncryptionTool.NIFI_PROPERTIES_ARG}' or '-l'/'--${ConfigEncryptionTool.LOGIN_IDENTITY_PROVIDERS_ARG}' must be provided unless '-x'/--'${ConfigEncryptionTool.DO_NOT_ENCRYPT_NIFI_PROPERTIES_ARG}' is specified"
final String MISSING_NFP_ARGUMENT = "Error parsing command line. (Missing argument for option: n)"
final String MISSING_LIP_ARGUMENT = "Error parsing command line. (Missing argument for option: l)"
final String MIGRATE_NEEDS_NFP = "In order to migrate a flow.xml.gz, a nifi.properties file must also be specified via '-n'/'--niFiProperties'."

def ACCEPTABLE_ERROR_MSGS = [NO_NFP_OR_LIP, MISSING_NFP_ARGUMENT, MISSING_LIP_ARGUMENT, MIGRATE_NEEDS_NFP]

// Act
invalidArgs.each { String badArgs ->
def msg = shouldFail {
tool.parse(badArgs.split(" ") as String[])
}
logger.expected(msg)

// Assert
assert ACCEPTABLE_ERROR_MSGS.contains(msg)
}
}

@Test
void testParseShouldIgnoreMigrationKeyAndPasswordIfMigrationNotEnabled() {
// Arrange
Expand Down Expand Up @@ -505,7 +474,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
@Test
void testShouldReadPasswordFromConsoleIfNoKeyPresent() {
// Arrange
def args = [] as String[]
def args = ["-n", "nifi.properties"] as String[]
ConfigEncryptionTool tool = new ConfigEncryptionTool()
tool.parse(args)
logger.info("Using password flag: ${tool.usingPassword}")
Expand All @@ -529,7 +498,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
@Test
void testShouldReadKeyFromConsoleIfFlagProvided() {
// Arrange
def args = ["-r"] as String[]
def args = ["-r", "-n", "nifi.properties"] as String[]
ConfigEncryptionTool tool = new ConfigEncryptionTool()
tool.parse(args)
logger.info("Using password flag: ${tool.usingPassword}")
Expand All @@ -553,7 +522,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
@Test
void testShouldIgnoreRawKeyFlagIfKeyProvided() {
// Arrange
def args = ["-r", "-k", KEY_HEX] as String[]
def args = ["-r", "-k", KEY_HEX, "-n", "nifi.properties"] as String[]
ConfigEncryptionTool tool = new ConfigEncryptionTool()

// Act
Expand All @@ -576,7 +545,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
@Test
void testShouldIgnoreRawKeyFlagIfPasswordProvided() {
// Arrange
def args = ["-r", "-p", PASSWORD] as String[]
def args = ["-r", "-p", PASSWORD, "-n", "nifi.properties"] as String[]
ConfigEncryptionTool tool = new ConfigEncryptionTool()

// Act
Expand Down Expand Up @@ -952,7 +921,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
final String EXPECTED_KEY_LINE = ConfigEncryptionTool.BOOTSTRAP_KEY_PREFIX + KEY_HEX

ConfigEncryptionTool tool = new ConfigEncryptionTool()
String[] args = ["-b", workingFile.path, "-k", KEY_HEX]
String[] args = ["-b", workingFile.path, "-k", KEY_HEX, "-n", "nifi.properties"]
tool.parse(args)

// Act
Expand Down Expand Up @@ -983,7 +952,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
logger.info("Set POSIX permissions to ${getFilePermissions(workingFile)}")

ConfigEncryptionTool tool = new ConfigEncryptionTool()
String[] args = ["-b", workingFile.path, "-k", KEY_HEX]
String[] args = ["-b", workingFile.path, "-k", KEY_HEX, "-n", "nifi.properties"]
tool.parse(args)

// Act
Expand Down Expand Up @@ -1012,7 +981,7 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
logger.info("Set POSIX permissions to ${getFilePermissions(workingFile)}")

ConfigEncryptionTool tool = new ConfigEncryptionTool()
String[] args = ["-b", workingFile.path, "-k", KEY_HEX]
String[] args = ["-b", workingFile.path, "-k", KEY_HEX, "-n", "nifi.properties"]
tool.parse(args)

// Act
Expand Down Expand Up @@ -3128,14 +3097,6 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
String newFlowPassword = passwordProgression[i + 1]
logger.info("Migrating from ${existingFlowPassword} to ${newFlowPassword}")

// Set up assertions for this iteration
// exit.expectSystemExitWithStatus(0)
// exit.checkAssertionAfterwards(new Assertion() {
// public void checkAssertion() {

// }
// });

// 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]

Expand All @@ -3148,8 +3109,6 @@ class ConfigEncryptionToolTest extends GroovyTestCase {
// Assert
// Get the updated nifi.properties and check the sensitive key
final List<String> updatedPropertiesLines = workingNiFiPropertiesFile.readLines()
// logger.info("Updated nifi.properties:")
// logger.info("\n" * 2 + updatedPropertiesLines.join("\n"))
String updatedSensitiveKeyLine = updatedPropertiesLines.find { it.startsWith(NiFiProperties.SENSITIVE_PROPS_KEY) }
logger.info("Updated key line: ${updatedSensitiveKeyLine}")

Expand Down

0 comments on commit 5fc13d9

Please sign in to comment.