capistrant commented on code in PR #12950:
URL: https://github.com/apache/druid/pull/12950#discussion_r956139880
##########
server/src/main/java/org/apache/druid/server/StatusResource.java:
##########
@@ -75,7 +78,29 @@ public Map<String, String> getProperties()
{
Map<String, String> allProperties = Maps.fromProperties(properties);
Set<String> hidderProperties = druidServerConfig.getHiddenProperties();
- return Maps.filterEntries(allProperties, (entry) ->
!hidderProperties.contains(entry.getKey()));
+ return filterHiddenProperties(hidderProperties, allProperties);
+ }
+
+ /**
+ * filter out entries from allProperties with key containing elements in
hidderProperties (case insensitive)
+ *
+ * @return map of properties that are not filtered out.
+ */
+ @Nonnull
+ private Map<String, String> filterHiddenProperties(
+ Set<String> hidderProperties,
Review Comment:
should this be `hiddenProperties` instead of `hidderProperties`?
##########
server/src/main/java/org/apache/druid/client/DruidServerConfig.java:
##########
@@ -84,4 +84,5 @@ public Set<String> getHiddenProperties()
return hiddenProperties;
}
+
Review Comment:
accidental newline?
##########
docs/configuration/index.md:
##########
@@ -787,6 +787,14 @@ All Druid components can communicate with each other over
HTTP.
|`druid.global.http.unusedConnectionTimeout`|The timeout for idle connections
in connection pool. The connection in the pool will be closed after this
timeout and a new one will be established. This timeout should be less than
`druid.global.http.readTimeout`. Set this timeout = ~90% of
`druid.global.http.readTimeout`|`PT4M`|
|`druid.global.http.numMaxThreads`|Maximum number of I/O worker
threads|`max(10, ((number of cores * 17) / 16 + 2) + 30)`|
+### Common endpoints Configuration
+
+This section contains the configuration options for endpoints that are
supported by all processes.
+
+|Property| Description
|Default|
+|--------|----------------------------------------------------------------------------------------------------------------------------------------------|-------|
+|`druid.server.hiddenProperties`| If property names or substring of property
names (case insensitive) is in this list, responses of the `/status/properties`
endpoint do not show these properties
|`["druid.s3.accessKey","druid.s3.secretKey","druid.metadata.storage.connector.password"]`|
Review Comment:
I want to raise the idea of adding new default generic property substrings
here such as `password` and `key` to help ensure redaction of any future
sensitive properties. I just checked a local deploy and there is definitely
other sensitive properties that are being exposed due to the restricted default
set + hidden config (until now)
--
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: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
