goiri commented on code in PR #4908:
URL: https://github.com/apache/hadoop/pull/4908#discussion_r974648275
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java:
##########
@@ -59,6 +59,8 @@ public class TestConfServlet {
new HashMap<String, String>();
private static final Map<String, String> TEST_FORMATS =
new HashMap<String, String>();
+ private static final Map<String, String> MASK_PROPERTIES =
Review Comment:
Single line
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java:
##########
@@ -214,6 +215,12 @@ public final class HttpServer2 implements FilterContainer {
private StatisticsHandler statsHandler;
private HttpServer2Metrics metrics;
+ private static final String MASK = "******";
+ public static final String FEDERATION_STATESTORE_SQL_USERNAME =
+ "yarn.federation.state-store.sql.username";
+ public static final String FEDERATION_STATESTORE_SQL_PASSWROD =
Review Comment:
Should we make this more generic and mas any conf key with password on it?
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java:
##########
@@ -247,4 +254,67 @@ public void testBadFormat() throws Exception {
}
assertEquals("", sw.toString());
}
+
+ private void verifyReplaceProperty(Configuration conf, String format,
+ String propertyName) throws Exception {
+ StringWriter sw = null;
+ PrintWriter pw = null;
+ ConfServlet service = null;
+ try {
+ service = new ConfServlet();
+ ServletConfig servletConf = mock(ServletConfig.class);
+ ServletContext context = mock(ServletContext.class);
+ service.init(servletConf);
+ when(context.getAttribute(HttpServer2.CONF_CONTEXT_ATTRIBUTE))
+ .thenReturn(conf);
+ when(service.getServletContext())
+ .thenReturn(context);
+
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getHeader(HttpHeaders.ACCEPT))
+ .thenReturn(TEST_FORMATS.get(format));
+ when(request.getParameter("name"))
+ .thenReturn(propertyName);
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ sw = new StringWriter();
+ pw = new PrintWriter(sw);
+ when(response.getWriter()).thenReturn(pw);
+
+ // response request
+ service.doGet(request, response);
+ String result = sw.toString().trim();
+
+ // For example, for the property
yarn.federation.state-store.sql.username,
+ // we set the value to test-user,
+ // which should be replaced by a mask, which should be ******
+ // MASK_PROPERTIES.get("property
yarn.federation.state-store.sql.username")
+ // is the value before replacement, test-user
+ // result contains the replaced value, which should be ******
+ assertTrue(result.contains(propertyName) &&
Review Comment:
Make it two separate assertTrue()
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java:
##########
@@ -214,6 +215,12 @@ public final class HttpServer2 implements FilterContainer {
private StatisticsHandler statsHandler;
private HttpServer2Metrics metrics;
+ private static final String MASK = "******";
+ public static final String FEDERATION_STATESTORE_SQL_USERNAME =
Review Comment:
In a more general sense, does it make sense for passwords to be in the
configuration file itself?
Do we have other mechanisms to specify these?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/ConfServlet.java:
##########
@@ -43,13 +48,18 @@ public class ConfServlet extends HttpServlet {
protected static final String FORMAT_JSON = "json";
protected static final String FORMAT_XML = "xml";
+
/**
* Return the Configuration of the daemon hosting this servlet.
* This is populated when the HttpServer starts.
*/
private Configuration getConfFromContext() {
Configuration conf = (Configuration)getServletContext().getAttribute(
HttpServer2.CONF_CONTEXT_ATTRIBUTE);
+ List<String> props = new ArrayList<>();
+ props.add(FEDERATION_STATESTORE_SQL_USERNAME);
Review Comment:
I think this should be more general.
We should have a generic part to set particular keys as passwords to not be
shown and then in the YARN router part to actually specify the key.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfServlet.java:
##########
@@ -247,4 +254,67 @@ public void testBadFormat() throws Exception {
}
assertEquals("", sw.toString());
}
+
+ private void verifyReplaceProperty(Configuration conf, String format,
+ String propertyName) throws Exception {
+ StringWriter sw = null;
+ PrintWriter pw = null;
+ ConfServlet service = null;
+ try {
+ service = new ConfServlet();
+ ServletConfig servletConf = mock(ServletConfig.class);
+ ServletContext context = mock(ServletContext.class);
+ service.init(servletConf);
+ when(context.getAttribute(HttpServer2.CONF_CONTEXT_ATTRIBUTE))
+ .thenReturn(conf);
+ when(service.getServletContext())
+ .thenReturn(context);
+
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ when(request.getHeader(HttpHeaders.ACCEPT))
Review Comment:
Single line
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
