Add proxy username and password settings for Azure repository plugin #2098

willyborankin · 2022-02-12T21:44:18Z

Description

Now to configure Azure plugin to use any proxy you need to set username/password using standard process variables:

Such settings have 2 main problems:

Storing password this way is insecure
In case of password leaking it is necessary to restart all nodes in the cluster

To avoid of a such problem username and password security settings were added
Storing username/password in security settings is more secure compare to process settings and in case of any problem with username and password such security settings could be reloaded without restart of each node in the cluster.

Re-loadable security settings:

azure.client.*.proxy.username - Proxy user name
azure.client.*.proxy.password - Proxy user password

Issues Resolved

[List any issues this PR will resolve]

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

opensearch-ci-bot · 2022-02-12T21:46:02Z

Can one of the admins verify this patch?

opensearch-ci-bot · 2022-02-12T22:26:27Z

✅ Gradle Check success 76c49fa37417aec92265cc02e11db34176dd337b
Log 2361

Reports 2361

opensearch-ci-bot · 2022-02-13T11:54:03Z

✅ Gradle Check success b09a27a234ca91554d7d47017c61e283635b6e53
Log 2365

Reports 2365

opensearch-ci-bot · 2022-02-13T12:28:22Z

❌ Gradle Check failure b32ccb3c436168052086cc470c1f1bf50ec75de2
Log 2366

Reports 2366

opensearch-ci-bot · 2022-02-13T15:51:51Z

✅ Gradle Check success d5c6238497f1e789c9c1e9886480289bc9ed23ae
Log 2369

Reports 2369

reta · 2022-02-14T15:58:59Z

...ns/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageService.java

+                    }
+                });
+            }
+            final Type azureProxyType = proxySettings.getType() == Proxy.Type.SOCKS ? Type.SOCKS5 : Type.HTTP;


I am not sure we should discard the possibility of Type.SOCKS4 usage since we don't know all possible deployment scenarios. May be it makes sense to widen the proxy types (detach from enumeration) to DIRECT, HTTP, SOCKS (default to SOCKS4 to support compatibility since this is what is being picked right now), SOCKS4, SOCKS5?

That is the problem that Azure SDK wraps up Proxy.Tpe and the real type for both - is Proxy.Type.SOCKS. We can set any in this case, since JDK takes care about SOCKS version

Now I get what do you mean. Let's use Azure SDK ProxyType explicitly

done. I had had to add ProxySettings.ProxyType. It wraps Azure SDK Type and use NONE as a default value, since settings cannot be null, and we need to check if ProxyType was set or not.

reta · 2022-02-14T19:59:58Z

...s/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java

        AZURE_CLIENT_PREFIX_KEY,
        "proxy.type",
-        (key) -> new Setting<>(key, "direct", s -> Proxy.Type.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope),
+        (key) -> new Setting<>(key, "none", s -> ProxySettings.ProxyType.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope),


Why not keep direct as the default? It seems like none is really not needed

reta · 2022-02-14T20:01:02Z

plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/ProxySettings.java

+
+        SOCKS5(ProxyOptions.Type.SOCKS5.name()),
+
+        NONE("NONE");


We better keep DIRECT instead of none since people may use this value (and it was a default)

reta · 2022-02-14T20:01:58Z

plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/ProxySettings.java

+    public static enum ProxyType {
+        HTTP(ProxyOptions.Type.HTTP.name()),
+
+        SOCKS4(ProxyOptions.Type.SOCKS4.name()),


We probably need SOCKS(ProxyOptions.Type.SOCKS4.name()) since it was a legit proxy value before (but mark is as @Deprecated, referring to SOCKS4)

opensearch-ci-bot · 2022-02-14T20:09:58Z

❌ Gradle Check failure 3341e652c0ce03374c9c3ba84aaba7f2c5443fb5
Log 2381

Reports 2381

willyborankin · 2022-02-14T20:40:48Z

@reta fixed your comments

reta · 2022-02-14T20:51:27Z

plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/ProxySettings.java

+            + username
+            + '\''
+            + ", password='"
+            + (Strings.isNullOrEmpty(password) ? "NO_EMPTY" : "EMPTY")


Super minor: NOT_EMPTY (or *****)

opensearch-ci-bot · 2022-02-14T21:20:03Z

✅ Gradle Check success d1cdcae982e5be7d6c2b98dbfca7f0c025782110
Log 2383

Reports 2383

dblock · 2022-02-14T21:25:28Z

~~- Help me understand where username/password are actually stored?~~

Are they stored in plain text or something else?

Are these values logged anywhere?

~~Is blank a valid password?~~

I see secureString and what happens below, thanks.

dblock · 2022-02-14T21:27:56Z

Please open related issues to document this plugin and its settings in https://github.com/opensearch-project/documentation-website.

dblock · 2022-02-14T21:28:09Z

@kartg Care to give this another look and merge?

plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/ProxySettings.java

...pository-azure/src/test/java/org/opensearch/repositories/azure/AzureStorageServiceTests.java

opensearch-ci-bot · 2022-02-15T15:04:38Z

✅ Gradle Check success 13eadeed3dafb7b7b0830543810009f2909aa9c1
Log 2399

Reports 2399

willyborankin · 2022-02-15T16:08:28Z

...ns/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageService.java

+        final ProxySettings proxySettings = azureStorageSettings.getProxySettings();
+        if (proxySettings != ProxySettings.NO_PROXY_SETTINGS) {
+            if (proxySettings.isAuthenticated()) {
+                Authenticator.setDefault(new Authenticator() {


it must be wrapped up with SocketAccess

Added username/password proxy settings for Azure repository. Security settings: - azure.client.*.proxy.username - Proxy user name - azure.client.*.proxy.password - Proxy user password Signed-off-by: Andrey Pleskach <[email protected]>

opensearch-ci-bot · 2022-02-15T17:00:14Z

✅ Gradle Check success 36df329
Log 2401

Reports 2401

Added username/password proxy settings for Azure repository. Security settings: - azure.client.*.proxy.username - Proxy user name - azure.client.*.proxy.password - Proxy user password Signed-off-by: Andrey Pleskach <[email protected]> (cherry picked from commit 62361ce)

dblock · 2022-02-16T17:00:20Z

...s/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java

@@ -325,7 +327,7 @@ public String toString() {
        sb.append(", timeout=").append(timeout);
        sb.append(", endpointSuffix='").append(endpointSuffix).append('\'');
        sb.append(", maxRetries=").append(maxRetries);
-        sb.append(", proxy=").append(proxy);
+        sb.append(", proxySettings=").append(proxySettings != ProxySettings.NO_PROXY_SETTINGS ? "PROXY_SET" : "PROXY_NOT_SET");


This is a nitpick, but things like PROXY_SET imply some kind of variable. Since this is a log, maybe just use "set" and "not set"?

…#2108) Added username/password proxy settings for Azure repository. Security settings: - azure.client.*.proxy.username - Proxy user name - azure.client.*.proxy.password - Proxy user password Signed-off-by: Andrey Pleskach <[email protected]> (cherry picked from commit 62361ce) Co-authored-by: Andrey Pleskach <[email protected]>

willyborankin · 2022-02-21T10:23:42Z

documen

done opensearch-project/documentation-website#417

willyborankin marked this pull request as ready for review February 12, 2022 22:30

willyborankin requested a review from a team as a code owner February 12, 2022 22:30

willyborankin force-pushed the azure-proxy-support branch from 76c49fa to b09a27a Compare February 13, 2022 11:12

willyborankin force-pushed the azure-proxy-support branch from b09a27a to b32ccb3 Compare February 13, 2022 12:05

willyborankin force-pushed the azure-proxy-support branch from b32ccb3 to d5c6238 Compare February 13, 2022 15:08

willyborankin changed the title ~~Add proxy username and password settings for Azure repository~~ Add proxy username and password settings for Azure repository plugin Feb 14, 2022

reta reviewed Feb 14, 2022

View reviewed changes

dblock requested a review from kartg February 14, 2022 17:08

willyborankin force-pushed the azure-proxy-support branch from d5c6238 to 3341e65 Compare February 14, 2022 19:29

reta reviewed Feb 14, 2022

View reviewed changes

willyborankin force-pushed the azure-proxy-support branch from 3341e65 to d1cdcae Compare February 14, 2022 20:40

reta reviewed Feb 14, 2022

View reviewed changes

reta approved these changes Feb 14, 2022

View reviewed changes

dblock approved these changes Feb 14, 2022

View reviewed changes

dblock added the backport 1.x label Feb 14, 2022

kartg reviewed Feb 15, 2022

View reviewed changes

plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/ProxySettings.java Outdated Show resolved Hide resolved

...pository-azure/src/test/java/org/opensearch/repositories/azure/AzureStorageServiceTests.java Outdated Show resolved Hide resolved

willyborankin force-pushed the azure-proxy-support branch from d1cdcae to 13eadee Compare February 15, 2022 14:26

willyborankin commented Feb 15, 2022

View reviewed changes

willyborankin force-pushed the azure-proxy-support branch from 13eadee to 36df329 Compare February 15, 2022 16:16

kartg approved these changes Feb 15, 2022

View reviewed changes

kartg merged commit 62361ce into opensearch-project:main Feb 15, 2022

opensearch-trigger-bot bot mentioned this pull request Feb 15, 2022

[Backport 1.x] Add proxy username and password settings for Azure repository plugin #2108

Merged

dblock reviewed Feb 16, 2022

View reviewed changes

kartg mentioned this pull request Feb 16, 2022

Add proxy settings for GCS repository plugin #2096

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add proxy username and password settings for Azure repository plugin #2098

Add proxy username and password settings for Azure repository plugin #2098

willyborankin commented Feb 12, 2022 •

edited

Loading

opensearch-ci-bot commented Feb 12, 2022

opensearch-ci-bot commented Feb 12, 2022

opensearch-ci-bot commented Feb 13, 2022

opensearch-ci-bot commented Feb 13, 2022

opensearch-ci-bot commented Feb 13, 2022

reta Feb 14, 2022

willyborankin Feb 14, 2022 •

edited

Loading

willyborankin Feb 14, 2022

willyborankin Feb 14, 2022 •

edited

Loading

reta Feb 14, 2022

reta Feb 14, 2022

reta Feb 14, 2022

reta Feb 14, 2022 •

edited

Loading

opensearch-ci-bot commented Feb 14, 2022

willyborankin commented Feb 14, 2022

reta Feb 14, 2022

opensearch-ci-bot commented Feb 14, 2022

dblock commented Feb 14, 2022 •

edited

Loading

dblock commented Feb 14, 2022

dblock commented Feb 14, 2022

opensearch-ci-bot commented Feb 15, 2022

willyborankin Feb 15, 2022

opensearch-ci-bot commented Feb 15, 2022

dblock Feb 16, 2022

willyborankin commented Feb 21, 2022

Add proxy username and password settings for Azure repository plugin #2098

Add proxy username and password settings for Azure repository plugin #2098

Conversation

willyborankin commented Feb 12, 2022 • edited Loading

Description

Issues Resolved

Check List

opensearch-ci-bot commented Feb 12, 2022

opensearch-ci-bot commented Feb 12, 2022

opensearch-ci-bot commented Feb 13, 2022

opensearch-ci-bot commented Feb 13, 2022

opensearch-ci-bot commented Feb 13, 2022

Choose a reason for hiding this comment

willyborankin Feb 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willyborankin Feb 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta Feb 14, 2022 • edited Loading

Choose a reason for hiding this comment

opensearch-ci-bot commented Feb 14, 2022

willyborankin commented Feb 14, 2022

Choose a reason for hiding this comment

opensearch-ci-bot commented Feb 14, 2022

dblock commented Feb 14, 2022 • edited Loading

dblock commented Feb 14, 2022

dblock commented Feb 14, 2022

opensearch-ci-bot commented Feb 15, 2022

Choose a reason for hiding this comment

opensearch-ci-bot commented Feb 15, 2022

Choose a reason for hiding this comment

willyborankin commented Feb 21, 2022

willyborankin commented Feb 12, 2022 •

edited

Loading

willyborankin Feb 14, 2022 •

edited

Loading

willyborankin Feb 14, 2022 •

edited

Loading

reta Feb 14, 2022 •

edited

Loading

dblock commented Feb 14, 2022 •

edited

Loading