OpenSearch: HTTP proxy support for plugin installation #11723
Conversation
dc81ac1 to
048f622
Compare
e75f0f1 to
3794647
Compare
| :param store_passwd: store password to use. | ||
| :return: path to the truststore file. | ||
| """ | ||
| store_path = new_tmp_file(suffix=".jks") |
There was a problem hiding this comment.
Q: I don't know how expensive these operations are, and if this is worth it, but could we cache this in config.dirs.tmp (but that depends on what the store depends on). Right now we are generating the same trust store for the installation of every single plugin, right?
There was a problem hiding this comment.
Yes, currently it's one trust store used for a given version of Opensearch/Elasticsearch installation, reused for all its plugins.
The JRE and thus keytool executable changes based on the Opensearch/Elasticsearch version (we use the bundled JRE), it may make sense not to re-use the trust store across versions.
| for scheme, default_port, var in [ | ||
| ("http", "80", config.OUTBOUND_HTTP_PROXY), | ||
| ("https", "443", config.OUTBOUND_HTTPS_PROXY), | ||
| ]: | ||
| if var: | ||
| netloc = urlparse(var).netloc | ||
| url = netloc.split(":") | ||
| if len(url) == 2: | ||
| hostname, port = url | ||
| else: | ||
| hostname, port = url[0], default_port | ||
|
|
||
| props[f"{scheme}.proxyHost"] = hostname | ||
| props[f"{scheme}.proxyPort"] = port |
There was a problem hiding this comment.
I would suggest renaming var to what it really is (proxy_url?).
And parsing the urls is always a bit complicated (I guess because the URL spec is complicated?), but urlparse should do the hostname and port parsing for you already:
| for scheme, default_port, var in [ | |
| ("http", "80", config.OUTBOUND_HTTP_PROXY), | |
| ("https", "443", config.OUTBOUND_HTTPS_PROXY), | |
| ]: | |
| if var: | |
| netloc = urlparse(var).netloc | |
| url = netloc.split(":") | |
| if len(url) == 2: | |
| hostname, port = url | |
| else: | |
| hostname, port = url[0], default_port | |
| props[f"{scheme}.proxyHost"] = hostname | |
| props[f"{scheme}.proxyPort"] = port | |
| for scheme, default_port, proxy_url in [ | |
| ("http", "80", config.OUTBOUND_HTTP_PROXY), | |
| ("https", "443", config.OUTBOUND_HTTPS_PROXY), | |
| ]: | |
| if var: | |
| parsed_proxy_url = urlparse(proxy_url) | |
| props[f"{scheme}.proxyHost"] = parsed_proxy_url.hostname | |
| props[f"{scheme}.proxyPort"] = parsed_proxy_url.port |
If the URL does not have a port, parsed_proxy_url.port will be None
There was a problem hiding this comment.
Thanks, it's slightly simpler now with 6eeb953
| # | ||
|
|
||
|
|
||
| def system_properties_to_cli_args(properties: dict[str, str]) -> list[str]: |
There was a problem hiding this comment.
Q: It's a pity you can't set system properties via environment variables, otherwise this could be easily integrated generally for our Java packages by adding the properties to JavaInstallerMixin.get_java_env_vars.
ElasticSearch and OpenSearch don't use the Java installation from the package installer, but their own, but what do you think would it take to get the proxy supported for all backends using Java in the future?
There was a problem hiding this comment.
We have the package/installer abstractions which handle the installation aspect. Personally I think we should look into introducing some abstractions for the execution/runtime aspect. Because strictly speaking, determining and setting the proxy config falls in that area. We have the JavaInstallerMixin which kind of blurs these responsibilities. But clearly demarcating the responsibilities would let us harmonise things just like the installers.
Already there are some common patterns that have emerged, e.g. KinesisServerManager which handle multiple instances of Kinesis-Mock to allow account/region namespacing, MqttBrokerManager — same for Mosquitto, etc. It just needs fleshing out. I'll put this in the backlog.
Background
LocalStack provides the
OUTBOUND_HTTP_PROXYandOUTBOUND_HTTPS_PROXYenv config options to set the HTTP/HTTPS proxy. These config options are mostly obeyed within the codebase, but certain external invocations do not respect them. Notable among them is ElasticSearch/OpenSearch. These are Java applications and require separate configuration.Changes
This PR ensures that the relevant ElasticSearch/OpenSearch components that directly access the internet are passed the proper proxy settings.
The values in
OUTBOUND_HTTP_PROXYandOUTBOUND_HTTPS_PROXYare translated to Java network system properties and passed to invocations of plugin managers.Furthermore, the PEM CA bundle if specified in
REQUESTS_CA_BUNDLEis converted to a JKS TrustStore and passed to the invocations. This is to ensure that SSL connections between the plugin managers and the proxy server are successful.The execution of ElasticSearch/OpenSearch is not changed, I'm assuming that they will not connect to the internet in normal operations.
Tests
This PR includes unit-tests for the code related to system properties.
The following methodology was used to test the actual proxy scenario:
http://127.0.0.1:8080OUTBOUND_HTTP_PROXY=http://localhost:8080OUTBOUND_HTTPS_PROXY=http://localhost:8080REQUESTS_CA_BUNDLE=/home/viren/.mitmproxy/mitmproxy-ca-cert.pembecause mitmproxy uses its own self-signed certificates (this config option is actually part of the Requests Python library but we piggyback on it)Elasticsearch_7.10andOpenSearch_1.1as engine versions). The step where ElasticSearch/OpenSearch installs the plugins is recorded in the mitmproxy terminal. The download of ElasticSearch/OpenSearch itself is not be seen in the screenshot because it was cached.