From 21c6f01831dd3a63c46c2cbb94289206a6239166 Mon Sep 17 00:00:00 2001 From: yliu Date: Mon, 5 Jan 2015 06:55:08 +0800 Subject: [PATCH] HADOOP-11455. KMS and Credential CLI should request confirmation for deletion by default. (Charles Lamb via yliu) --- .../hadoop-common/CHANGES.txt | 3 +++ .../apache/hadoop/crypto/key/KeyShell.java | 18 +++++++++-------- .../security/alias/CredentialShell.java | 20 ++++++++++--------- .../hadoop/crypto/key/TestKeyShell.java | 3 ++- .../hadoop/security/alias/TestCredShell.java | 7 ++++--- 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index d7ebeac0e2..ec75e8de8e 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -452,6 +452,9 @@ Release 2.7.0 - UNRELEASED HADOOP-11399. Java Configuration file and .xml files should be automatically cross-compared (rchiang via rkanter) + HADOOP-11455. KMS and Credential CLI should request confirmation for + deletion by default. (Charles Lamb via yliu) + OPTIMIZATIONS HADOOP-11323. WritableComparator#compare keeps reference to byte array. diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java index e0ca6240e3..4c72d06d06 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java @@ -47,7 +47,7 @@ public class KeyShell extends Configured implements Tool { " [" + ListCommand.USAGE + "]\n"; private static final String LIST_METADATA = "keyShell.list.metadata"; - private boolean interactive = false; + private boolean interactive = true; private Command command = null; /** allows stdout to be captured if necessary */ @@ -169,8 +169,8 @@ public class KeyShell extends Configured implements Tool { getConf().set(KeyProviderFactory.KEY_PROVIDER_PATH, args[++i]); } else if ("-metadata".equals(args[i])) { getConf().setBoolean(LIST_METADATA, true); - } else if ("-i".equals(args[i]) || ("-interactive".equals(args[i]))) { - interactive = true; + } else if ("-f".equals(args[i]) || ("-force".equals(args[i]))) { + interactive = false; } else if ("-help".equals(args[i])) { printKeyShellUsage(); return 1; @@ -367,11 +367,13 @@ public class KeyShell extends Configured implements Tool { } private class DeleteCommand extends Command { - public static final String USAGE = "delete [-provider ] [-help]"; + public static final String USAGE = + "delete [-provider ] [-f] [-help]"; public static final String DESC = "The delete subcommand deletes all versions of the key\n" + "specified by the argument from within the\n" + - "provider specified -provider."; + "provider specified -provider. The command asks for\n" + + "user confirmation unless -f is specified."; String keyName = null; boolean cont = true; @@ -397,10 +399,10 @@ public class KeyShell extends Configured implements Tool { try { cont = ToolRunner .confirmPrompt("You are about to DELETE all versions of " - + " key: " + keyName + " from KeyProvider " - + provider + ". Continue?:"); + + " key " + keyName + " from KeyProvider " + + provider + ". Continue? "); if (!cont) { - out.println("Nothing has been be deleted."); + out.println(keyName + " has not been deleted."); } return cont; } catch (IOException e) { diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java index 6d9c6af263..f39740309e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java @@ -44,7 +44,7 @@ public class CredentialShell extends Configured implements Tool { " [" + DeleteCommand.USAGE + "]\n" + " [" + ListCommand.USAGE + "]\n"; - private boolean interactive = false; + private boolean interactive = true; private Command command = null; /** allows stdout to be captured if necessary */ @@ -116,8 +116,8 @@ public class CredentialShell extends Configured implements Tool { userSuppliedProvider = true; getConf().set(CredentialProviderFactory.CREDENTIAL_PROVIDER_PATH, args[++i]); - } else if (args[i].equals("-i") || (args[i].equals("-interactive"))) { - interactive = true; + } else if (args[i].equals("-f") || (args[i].equals("-force"))) { + interactive = false; } else if (args[i].equals("-v") || (args[i].equals("-value"))) { value = args[++i]; } else if (args[i].equals("-help")) { @@ -236,11 +236,13 @@ public class CredentialShell extends Configured implements Tool { } private class DeleteCommand extends Command { - public static final String USAGE = "delete [-provider] [-help]"; + public static final String USAGE = + "delete [-provider] [-f] [-help]"; public static final String DESC = - "The delete subcommand deletes the credenital\n" + + "The delete subcommand deletes the credential\n" + "specified as the argument from within the provider\n" + - "indicated through the -provider argument"; + "indicated through the -provider argument. The command asks for\n" + + "confirmation unless the -f option is specified."; String alias = null; boolean cont = true; @@ -267,9 +269,9 @@ public class CredentialShell extends Configured implements Tool { if (interactive) { try { cont = ToolRunner - .confirmPrompt("You are about to DELETE the credential: " + + .confirmPrompt("You are about to DELETE the credential " + alias + " from CredentialProvider " + provider.toString() + - ". Continue?:"); + ". Continue? "); if (!cont) { out.println("Nothing has been deleted."); } @@ -293,7 +295,7 @@ public class CredentialShell extends Configured implements Tool { provider.flush(); printProviderWritten(); } catch (IOException e) { - out.println(alias + "has NOT been deleted."); + out.println(alias + " has NOT been deleted."); throw e; } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java index 3407eb7cec..fe887182a2 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java @@ -75,7 +75,8 @@ public class TestKeyShell { private void deleteKey(KeyShell ks, String keyName) throws Exception { int rc; outContent.reset(); - final String[] delArgs = {"delete", keyName, "-provider", jceksProvider}; + final String[] delArgs = + {"delete", keyName, "-f", "-provider", jceksProvider}; rc = ks.run(delArgs); assertEquals(0, rc); assertTrue(outContent.toString().contains(keyName + " has been " + diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java index c89036272b..7ba4bc17e2 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java @@ -47,6 +47,7 @@ public class TestCredShell { System.setOut(new PrintStream(outContent)); System.setErr(new PrintStream(errContent)); final Path jksPath = new Path(tmpDir.toString(), "keystore.jceks"); + new File(jksPath.toString()).delete(); jceksProvider = "jceks://file" + jksPath.toUri(); } @@ -71,7 +72,7 @@ public class TestCredShell { assertTrue(outContent.toString().contains("credential1")); outContent.reset(); - String[] args4 = {"delete", "credential1", "-provider", + String[] args4 = {"delete", "credential1", "-f", "-provider", jceksProvider}; rc = cs.run(args4); assertEquals(0, rc); @@ -113,7 +114,7 @@ public class TestCredShell { assertTrue(outContent.toString().contains("WARNING: you are modifying a " + "transient provider.")); - String[] args2 = {"delete", "credential1", "-provider", "user:///"}; + String[] args2 = {"delete", "credential1", "-f", "-provider", "user:///"}; rc = cs.run(args2); assertEquals(outContent.toString(), 0, rc); assertTrue(outContent.toString().contains("credential1 has been successfully " + @@ -167,7 +168,7 @@ public class TestCredShell { assertTrue(outContent.toString().contains("credential1 has been successfully " + "created.")); - String[] args2 = {"delete", "credential1", "-provider", + String[] args2 = {"delete", "credential1", "-f", "-provider", jceksProvider}; rc = shell.run(args2); assertEquals(0, rc);