repository-azure plugin: SAS authentication fails with endpoint_suffix

[raphlopez]

Describe the bug
Loading the repository-azure plugin fails when providing a SAS token rather than a Storage Account key for Azure authentication.

To Reproduce
Steps to reproduce the behavior:

1. Start an opensearch server with a keystore containing the following keys (any value should work to reproduce this bug):

• azure.client.default.account
• azure.client.default.sas_token
• azure.client.default.endpoint_suffix

2. Load the repository-azure plugin
3. Upon load, the following exception should occur

  java.lang.IllegalArgumentException: missing required setting [azure.client.default.key] for setting [azure.client.default.endpoint_suffix]
          at org.opensearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:606)
          at org.opensearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:530)
          at org.opensearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:500)
          at org.opensearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:470)
          at org.opensearch.common.settings.SettingsModule.<init>(SettingsModule.java:161)
          at org.opensearch.node.Node.<init>(Node.java:463)
          at org.opensearch.node.Node.<init>(Node.java:319)
          at org.opensearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:242)
          at org.opensearch.bootstrap.Bootstrap.setup(Bootstrap.java:242)
          at org.opensearch.bootstrap.Bootstrap.init(Bootstrap.java:412)
          at org.opensearch.bootstrap.OpenSearch.init(OpenSearch.java:178)
          at org.opensearch.bootstrap.OpenSearch.execute(OpenSearch.java:169)
          at org.opensearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:100)
          at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
          at org.opensearch.cli.Command.main(Command.java:101)
          at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:135)
          at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:101)

Expected behavior
According to the checks in buildConnectString() method, authentication can be done by providing either a storage account name and key, or a storage account name and a SAS token.

However, providing only a SAS_TOKEN_SETTING results in the following error on plugin load (from above trace):

java.lang.IllegalArgumentException: missing required setting [azure.client.default.key] for setting [azure.client.default.endpoint_suffix]

The error message above points to the ENDPOINT_SUFFIX_SETTING, which has a dependency on storage account name and key only (i.e. does not account for SAS token).

Given that SAS_TOKEN_SETTING can be set and is mutually exclusive with KEY_SETTING, the ENDPOINT_SUFFIX_SETTING's dependencies should account for this alternative authentication method.

Plugins
repository-azure

[reta]

@raphlopez could you please try without using azure.client.default.endpoint_suffix? Or it is required to be used?

[raphlopez]

@reta it does work with azure.client.default.endpoint_suffix removed, but there is a use-case for authenticating to other clouds in azure with different endpoint_suffix (e.g. core.chinacloudapi.cn for Azure China, or core.cloudapi.de for Azure Germany.)

Looking back, I could see this being categorized as a feature request more than a bug report - let me know if you'd like me to adjust the title/tags accordingly.

[raphlopez]

ENDPOINT_SUFFIX dependency on KEY_SETTING has been removed in #2485, and backports to 2.x and 2.0 respectively in #2807 and #2808

[reta]

@dblock @nknize since 2.0 release is a bit far from now, would you think we could backport this small change to 1.3.x (to get it as part of 1.3.2)? thank you!

[dblock]

I think that 1.3.2 ship just sailed, hasn't it?

[reta]

I think that 1.3.2 ship just sailed, hasn't it?

Yes, you are right, I discovered that yesterday at open hours, thanks!

[dblock]

Generally those releases are for security fixes only. While backporting other fixes and even features seems nice because one feels like they are doing a no-risk upgrade, it doesn't always work out that way.

[reta]

Generally those releases are for security fixes only. While backporting other fixes and even features seems nice because one feels like they are doing a no-risk upgrade, it doesn't always work out that way.

I agree, thank you