dpol1 commented on code in PR #2173:
URL: https://github.com/apache/stormcrawler/pull/2173#discussion_r4080082937


##########
docs/src/main/asciidoc/configuration.adoc:
##########
@@ -441,9 +441,10 @@ See the 
link:https://github.com/apache/stormcrawler/tree/main/external/opensearc
 |===
 | key | default value | description
 
-| opensearch.addresses | - | OpenSearch server address(es).
-| opensearch.user | - | Username for authentication (optional).
+| opensearch.addresses | - | OpenSearch server address(es). An address without 
a scheme is contacted over plain http, on port 9200 if none is given; use 
`https://` when credentials are configured.
+| opensearch.user | - | Username for Basic authentication (optional). The 
credentials are only sent to the hosts and ports listed in the addresses, not 
to nodes discovered by sniffing under another address; list those nodes in the 
addresses or disable `opensearch.<type>.sniff`, which is on by default. A 
warning is logged when sniffing is enabled together with credentials, and if an 
address other than a loopback one uses plain http.

Review Comment:
   `opensearch.<type>.sniff` isn't in the table, can you add a row?



##########
external/opensearch/src/main/java/org/apache/stormcrawler/opensearch/OpenSearchConnection.java:
##########
@@ -306,6 +378,21 @@ public static OpenSearchConnection getConnection(
                     ConfUtils.getBoolean(
                             stormConf, Constants.PARAMPREFIX, dottedType, 
"sniff", true);
             if (sniff) {
+                if (hasCredentials(
+                        ConfUtils.getString(stormConf, Constants.PARAMPREFIX, 
dottedType, "user"),
+                        ConfUtils.getString(
+                                stormConf, Constants.PARAMPREFIX, dottedType, 
"password"))) {
+                    LOG.warn(
+                            "Sniffing is enabled for {} and OpenSearch 
credentials are configured. "
+                                    + "The credentials are only sent to the 
configured addresses: "
+                                    + "requests to a node the sniffer finds 
under another host or "
+                                    + "port are sent without them and fail if 
the cluster requires "
+                                    + "authentication. List every node in 
opensearch.{}.addresses "

Review Comment:
   this doesn't work with https addresses: the sniffer registers the nodes it 
finds as `Scheme.HTTP`, so after the first sniff every request goes over http. 
shall we pass `Scheme.HTTPS` to `OpenSearchNodesSniffer` when the addresses are 
https?



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

Reply via email to