HADOOP-16615. Add password check for credential provider #1614

hddong · 2019-10-08T07:27:50Z

When we use hadoop credential provider to store password, we can not sure if the password is the same as what we remembered.

So, I think we need a check tool.

hadoop-yetus · 2019-10-08T09:53:50Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	1036	Docker mode activated.
		_ Prechecks _
+1	dupname	0	No case conflicting files found.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 1 new or modified test files.
		_ trunk Compile Tests _
+1	mvninstall	1234	trunk passed
-1	compile	205	root in trunk failed.
+1	checkstyle	36	trunk passed
+1	mvnsite	78	trunk passed
+1	shadedclient	963	branch has no errors when building and testing our client artifacts.
+1	javadoc	96	trunk passed
0	spotbugs	134	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	132	trunk passed
		_ Patch Compile Tests _
+1	mvninstall	54	the patch passed
-1	compile	210	root in the patch failed.
-1	javac	210	root in the patch failed.
-0	checkstyle	34	hadoop-common-project/hadoop-common: The patch generated 11 new + 23 unchanged - 0 fixed = 34 total (was 23)
+1	mvnsite	70	the patch passed
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedclient	851	patch has no errors when building and testing our client artifacts.
+1	javadoc	86	the patch passed
+1	findbugs	138	the patch passed
		_ Other Tests _
-1	unit	552	hadoop-common in the patch failed.
+1	asflicense	35	The patch does not generate ASF License warnings.
		5854

Reason	Tests
Failed junit tests	hadoop.util.TestReadWriteDiskValidator

Subsystem	Report/Notes
Docker	Client=19.03.2 Server=19.03.2 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/artifact/out/Dockerfile
GITHUB PR	#1614
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux b7b849b49a07 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `4fdf016`
Default Java	1.8.0_222
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/artifact/out/branch-compile-root.txt
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/artifact/out/patch-compile-root.txt
javac	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/artifact/out/patch-compile-root.txt
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
unit	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/testReport/
Max. process+thread count	1368 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/2/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

hddong · 2019-10-09T01:46:16Z

@steveloughran HADOOP-16615 patch here.

...on-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java

steveloughran · 2019-10-10T17:30:50Z

...mmon-project/hadoop-common/src/test/java/org/apache/hadoop/security/alias/TestCredShell.java

    rc = shell.run(args2);
    assertEquals(0, rc);
+    assertTrue(outContent.toString().contains("Password match success for credential1."));


Add an error message here which includes the failing string if the conditions not met. FWIW we are moving to using AssertJ assertions -if you are happy with them, feel free to use them over classic junit asserts

An error check added.

steveloughran · 2019-10-10T17:32:02Z

oops, deleted the wrong yetus comment. once you update the patch it will kick off again.

Overall -patch LGTM; a little bit of tuning and it's ready

hadoop-yetus · 2019-10-11T11:09:15Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	1794	Docker mode activated.
		_ Prechecks _
+1	dupname	0	No case conflicting files found.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 1 new or modified test files.
		_ trunk Compile Tests _
+1	mvninstall	1067	trunk passed
+1	compile	1016	trunk passed
+1	checkstyle	51	trunk passed
+1	mvnsite	86	trunk passed
+1	shadedclient	881	branch has no errors when building and testing our client artifacts.
+1	javadoc	92	trunk passed
0	spotbugs	123	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	120	trunk passed
		_ Patch Compile Tests _
+1	mvninstall	49	the patch passed
+1	compile	968	the patch passed
+1	javac	968	the patch passed
-0	checkstyle	51	hadoop-common-project/hadoop-common: The patch generated 12 new + 22 unchanged - 0 fixed = 34 total (was 22)
+1	mvnsite	80	the patch passed
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedclient	718	patch has no errors when building and testing our client artifacts.
+1	javadoc	94	the patch passed
+1	findbugs	132	the patch passed
		_ Other Tests _
+1	unit	545	hadoop-common in the patch passed.
+1	asflicense	55	The patch does not generate ASF License warnings.
		7897

Subsystem	Report/Notes
Docker	Client=19.03.3 Server=19.03.3 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/3/artifact/out/Dockerfile
GITHUB PR	#1614
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 9f23c3ae07b4 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `f267917`
Default Java	1.8.0_222
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/3/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/3/testReport/
Max. process+thread count	1374 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/3/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2019-10-11T11:28:28Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	83	Docker mode activated.
		_ Prechecks _
+1	dupname	1	No case conflicting files found.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 1 new or modified test files.
		_ trunk Compile Tests _
+1	mvninstall	1226	trunk passed
+1	compile	1082	trunk passed
+1	checkstyle	45	trunk passed
+1	mvnsite	80	trunk passed
+1	shadedclient	938	branch has no errors when building and testing our client artifacts.
+1	javadoc	87	trunk passed
0	spotbugs	128	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	126	trunk passed
		_ Patch Compile Tests _
+1	mvninstall	48	the patch passed
+1	compile	1026	the patch passed
+1	javac	1026	the patch passed
-0	checkstyle	46	hadoop-common-project/hadoop-common: The patch generated 12 new + 22 unchanged - 0 fixed = 34 total (was 22)
+1	mvnsite	78	the patch passed
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedclient	806	patch has no errors when building and testing our client artifacts.
+1	javadoc	85	the patch passed
+1	findbugs	130	the patch passed
		_ Other Tests _
+1	unit	596	hadoop-common in the patch passed.
+1	asflicense	48	The patch does not generate ASF License warnings.
		6614

Subsystem	Report/Notes
Docker	Client=19.03.3 Server=19.03.3 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/4/artifact/out/Dockerfile
GITHUB PR	#1614
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux b101a7cec088 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `f267917`
Default Java	1.8.0_222
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/4/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/4/testReport/
Max. process+thread count	1378 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/4/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2019-10-11T12:31:58Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	91	Docker mode activated.
		_ Prechecks _
+1	dupname	0	No case conflicting files found.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	0	The patch appears to include 1 new or modified test files.
		_ trunk Compile Tests _
+1	mvninstall	1313	trunk passed
+1	compile	1184	trunk passed
+1	checkstyle	42	trunk passed
+1	mvnsite	79	trunk passed
+1	shadedclient	930	branch has no errors when building and testing our client artifacts.
+1	javadoc	93	trunk passed
0	spotbugs	141	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	138	trunk passed
		_ Patch Compile Tests _
+1	mvninstall	63	the patch passed
+1	compile	1102	the patch passed
+1	javac	1102	the patch passed
-0	checkstyle	49	hadoop-common-project/hadoop-common: The patch generated 12 new + 23 unchanged - 0 fixed = 35 total (was 23)
+1	mvnsite	87	the patch passed
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedclient	831	patch has no errors when building and testing our client artifacts.
+1	javadoc	87	the patch passed
+1	findbugs	142	the patch passed
		_ Other Tests _
+1	unit	587	hadoop-common in the patch passed.
+1	asflicense	45	The patch does not generate ASF License warnings.
		6958

Subsystem	Report/Notes
Docker	Client=19.03.3 Server=19.03.3 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/5/artifact/out/Dockerfile
GITHUB PR	#1614
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 75c6b5958cb3 4.15.0-54-generic #58-Ubuntu SMP Mon Jun 24 10:55:24 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `4a700c2`
Default Java	1.8.0_222
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/5/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/5/testReport/
Max. process+thread count	1338 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/5/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

steveloughran · 2019-10-11T14:21:57Z

code is good, you just need to deal with those checkstyles, which are mostly indentation and a couple of minor line lengths

hadoop-yetus · 2019-10-12T10:37:27Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
0	reexec	41	Docker mode activated.
		_ Prechecks _
+1	dupname	1	No case conflicting files found.
+1	@author	0	The patch does not contain any @author tags.
+1	test4tests	1	The patch appears to include 1 new or modified test files.
		_ trunk Compile Tests _
+1	mvninstall	1069	trunk passed
+1	compile	1023	trunk passed
+1	checkstyle	52	trunk passed
+1	mvnsite	84	trunk passed
+1	shadedclient	890	branch has no errors when building and testing our client artifacts.
+1	javadoc	94	trunk passed
0	spotbugs	123	Used deprecated FindBugs config; considering switching to SpotBugs.
+1	findbugs	119	trunk passed
		_ Patch Compile Tests _
+1	mvninstall	48	the patch passed
+1	compile	971	the patch passed
+1	javac	971	the patch passed
-0	checkstyle	49	hadoop-common-project/hadoop-common: The patch generated 1 new + 22 unchanged - 0 fixed = 23 total (was 22)
+1	mvnsite	80	the patch passed
+1	whitespace	0	The patch has no whitespace issues.
+1	shadedclient	732	patch has no errors when building and testing our client artifacts.
+1	javadoc	94	the patch passed
+1	findbugs	131	the patch passed
		_ Other Tests _
+1	unit	545	hadoop-common in the patch passed.
+1	asflicense	53	The patch does not generate ASF License warnings.
		6178

Subsystem	Report/Notes
Docker	Client=19.03.3 Server=19.03.3 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/6/artifact/out/Dockerfile
GITHUB PR	#1614
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux cf69b3d98101 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `6e5cd52`
Default Java	1.8.0_222
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/6/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/6/testReport/
Max. process+thread count	1370 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1614/6/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.10.0 http://yetus.apache.org

This message was automatically generated.

hddong · 2019-10-18T01:04:31Z

retest this, please

hddong · 2019-10-22T01:29:24Z

retest this please

hddong · 2019-10-22T01:30:37Z

@steveloughran please help to retest this.

steveloughran · 2019-10-23T16:27:14Z

I like all tests in our code to provide enough diagnostics on failure that we can work out what went wrong purely from the Jenkins logs.

AssertJ is really good here, which is why we are adopting it in new code.

For your tests, I'm going to propose a new assertion which can be used to validate the output of the new command. If the condition is not met,
The exception raised will include all of the output.

private void assertOutputContains(String expected) {
  Assertions.assertThat(outContent.toString())
    .contains(expected));
}

Yetus probably isn't retesting the code, as it thinks the last change was a documentation only change. If you change the test case things may be different.

Use assertj

hddong · 2019-10-24T09:27:31Z

@steveloughran thanks for your explanation. Change to use assertj.

steveloughran · 2019-10-24T17:45:11Z

thanks. code LGTM.

+1

steveloughran · 2019-10-24T17:51:30Z

+1, merged to trunk. Thanks!

(if you do a PR with this cherry picked back to 3.2/3.1 then I'll pull back there too. It's low risk and beneficial to all)

hddong · 2019-10-28T01:47:38Z

@steveloughran thanks for your help and review.

hddong and others added 4 commits October 8, 2019 15:25

HADOOP-16615 Add password check for credential provider

91a5ea6

Update CredentialShell.java

dfff1e9

Update CommandsManual.md

de6e75e

Update CommandsManual.md

8cf3b20

apache deleted a comment from hadoop-yetus Oct 10, 2019

steveloughran reviewed Oct 10, 2019

View reviewed changes

...on-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java Outdated Show resolved Hide resolved

steveloughran reviewed Oct 10, 2019

View reviewed changes

...on-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java Show resolved Hide resolved

steveloughran reviewed Oct 10, 2019

View reviewed changes

hddong added 4 commits October 11, 2019 16:10

Update CredentialShell.java

6a4d257

Add failed test

966115e

Update CredentialShell.java

d3f90ca

Update CredentialShell.java

e19324b

hddong added 2 commits October 12, 2019 16:35

checkstyle

2eb3d3e

checkstyle

12caecb

Update CredentialShell.java

4dadefc

Update CommandsManual.md

8b41d45

Change testPromptForCredential

41cb0bd

Use assertj

steveloughran closed this Oct 31, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-16615. Add password check for credential provider #1614

HADOOP-16615. Add password check for credential provider #1614

hddong commented Oct 8, 2019

hadoop-yetus commented Oct 8, 2019

hddong commented Oct 9, 2019

steveloughran Oct 10, 2019

hddong Oct 11, 2019

steveloughran commented Oct 10, 2019

hadoop-yetus commented Oct 11, 2019

hadoop-yetus commented Oct 11, 2019

hadoop-yetus commented Oct 11, 2019

steveloughran commented Oct 11, 2019

hadoop-yetus commented Oct 12, 2019

hddong commented Oct 18, 2019

hddong commented Oct 22, 2019

hddong commented Oct 22, 2019

steveloughran commented Oct 23, 2019

hddong commented Oct 24, 2019

steveloughran commented Oct 24, 2019 •

edited

Loading

steveloughran commented Oct 24, 2019

hddong commented Oct 28, 2019

HADOOP-16615. Add password check for credential provider #1614

HADOOP-16615. Add password check for credential provider #1614

Conversation

hddong commented Oct 8, 2019

hadoop-yetus commented Oct 8, 2019

hddong commented Oct 9, 2019

steveloughran Oct 10, 2019

Choose a reason for hiding this comment

hddong Oct 11, 2019

Choose a reason for hiding this comment

steveloughran commented Oct 10, 2019

hadoop-yetus commented Oct 11, 2019

hadoop-yetus commented Oct 11, 2019

hadoop-yetus commented Oct 11, 2019

steveloughran commented Oct 11, 2019

hadoop-yetus commented Oct 12, 2019

hddong commented Oct 18, 2019

hddong commented Oct 22, 2019

hddong commented Oct 22, 2019

steveloughran commented Oct 23, 2019

hddong commented Oct 24, 2019

steveloughran commented Oct 24, 2019 • edited Loading

steveloughran commented Oct 24, 2019

hddong commented Oct 28, 2019

steveloughran commented Oct 24, 2019 •

edited

Loading