Support Azure Workload Identity authentication for blob repository

[Richard-coco]

Description

Add support for Azure Workload Identity authentication in the Azure repository plugin.
Add configuration validation to prevent mixing Workload Identity with account key or SAS token.
Add corresponding unit tests.

Related Issues

Resolves #20789

Check List

• Functionality includes testing.
• API changes companion pull request, if applicable.
• Public documentation issue/PR, if applicable.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 84f4371)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Variable Scope The type variable is declared inside the if (Strings.hasText(tokenCredentialType)) block, but it is used in the lambda clientBuilder which is also inside that block. This is fine for the current structure, but the lambda captures type from the enclosing scope. Since type is a local variable used in a lambda, it must be effectively final — verify that the compiler does not complain and that the variable is not reassigned anywhere in the block. TokenCredentialType type = TokenCredentialType.valueOfType(tokenCredentialType); if (type == TokenCredentialType.WORKLOAD_IDENTITY) { if (Strings.hasText(key) || Strings.hasText(sasToken)) { throw new SettingsException("Workload Identity cannot be used with key or sas_token"); } } this.endpointBuilder = (logger) -> { String tokenCredentialEndpointSuffix = endpointSuffix; if (Strings.hasText(tokenCredentialEndpointSuffix) == false) { // Default to "core.windows.net". tokenCredentialEndpointSuffix = Constants.ConnectionStringConstants.DEFAULT_DNS; } final URI primaryBlobEndpoint = URI.create("https://" + account + ".blob." + tokenCredentialEndpointSuffix); final URI secondaryBlobEndpoint = URI.create("https://" + account + "-secondary.blob." + tokenCredentialEndpointSuffix); return new StorageEndpoint(primaryBlobEndpoint, secondaryBlobEndpoint); }; this.clientBuilder = (builder, executor, logger) -> { if (type == TokenCredentialType.WORKLOAD_IDENTITY) { return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); } else { return builder.credential(new ManagedIdentityCredentialBuilder() { @Override public ManagedIdentityCredential build() { // Use the privileged executor with IdentityClient instance CredentialBuilderBaseHelper.getClientOptions(this).setExecutorService(executor); return super.build(); } }.build()).endpoint(endpointBuilder.apply(logger).getPrimaryUri()); } }; Missing Validation The validation that prevents mixing Workload Identity with key or SAS token only applies when tokenCredentialType is WORKLOAD_IDENTITY. There is no validation preventing MANAGED_IDENTITY from being combined with key or SAS token, which may also be an unintended or insecure configuration. Consider whether similar validation should apply to managed identity as well. if (Strings.hasText(tokenCredentialType)) { TokenCredentialType type = TokenCredentialType.valueOfType(tokenCredentialType); if (type == TokenCredentialType.WORKLOAD_IDENTITY) { if (Strings.hasText(key) || Strings.hasText(sasToken)) { throw new SettingsException("Workload Identity cannot be used with key or sas_token"); } } Missing Test Coverage There is no test verifying that MANAGED_IDENTITY combined with a key or SAS token does NOT throw an exception (to confirm it is intentionally allowed), nor a test for an invalid/unknown token_credential_type value. Adding these would improve coverage and clarify intended behavior. public void testAzureStorageSettingsWithManagedIdentity() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("azure.client.default.account", "teststorage"); Settings settings = Settings.builder() .setSecureSettings(secureSettings) .put("azure.client.default.token_credential_type", "managed") .build(); AzureStorageSettings settingsObj = AzureStorageSettings.load(settings).get("default"); assertEquals("managed", settingsObj.getTokenCredentialType()); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 84f4371

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use valid Base64 key in test to avoid false failures The test uses "any_fake_key" as the key value, but Azure storage keys are expected to be Base64-encoded strings. If the AzureStorageSettings.load() method validates the key format before reaching the workload identity check, the test may throw a different exception than SettingsException (e.g., an IllegalArgumentException for invalid Base64), causing the test to fail for the wrong reason. Use a valid Base64-encoded string as the fake key value to ensure the test accurately validates the intended behavior. plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java [238-241] public void testWorkloadIdentityWithKeyThrowsException() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("azure.client.default.account", "teststorage"); - secureSettings.setString("azure.client.default.key", "any_fake_key"); + secureSettings.setString("azure.client.default.key", "dGVzdGtleQ=="); // Base64 encoded "testkey" Suggestion importance[1-10]: 5 __ Why: This is a valid concern — if the key is validated as Base64 before the workload identity check, the test could fail for the wrong reason. Using a valid Base64 string ensures the test accurately validates the intended SettingsException behavior.	Low
	Validate required account field for workload identity The validation for WORKLOAD_IDENTITY only checks if key or sasToken is present, but does not validate that the account field is set, which is required for constructing the blob endpoint URL. If account is empty or null, the URI construction "https://" + account + ".blob." + tokenCredentialEndpointSuffix will produce an invalid endpoint silently. Add a validation check to ensure account is present when using WORKLOAD_IDENTITY. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [282-288] if (type == TokenCredentialType.WORKLOAD_IDENTITY) { if (Strings.hasText(key) || Strings.hasText(sasToken)) { throw new SettingsException("Workload Identity cannot be used with key or sas_token"); } + if (!Strings.hasText(account)) { + throw new SettingsException("Account must be specified when using Workload Identity"); + } } Suggestion importance[1-10]: 3 __ Why: While the suggestion adds a validation for account, the account field is likely already validated elsewhere in the settings loading process, making this an edge case improvement with limited practical impact.	Low
Possible issue	Ensure lambda captures instance field correctly The variable type is declared inside the if block but is captured in the lambda clientBuilder. In Java, variables captured by lambdas must be effectively final. Since type is declared within the same if block scope and is not reassigned, this should work, but it's worth verifying that type is not reassigned anywhere. More critically, endpointBuilder is also referenced inside clientBuilder lambda, but endpointBuilder is assigned (this.endpointBuilder = ...) after the clientBuilder lambda is defined — this means the lambda captures the field reference correctly via this, but if endpointBuilder is a local variable or reassigned, it could cause issues. Ensure endpointBuilder used inside clientBuilder refers to the instance field and not a stale reference. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [300-314] this.clientBuilder = (builder, executor, logger) -> { if (type == TokenCredentialType.WORKLOAD_IDENTITY) { return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) - .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); + .endpoint(this.endpointBuilder.apply(logger).getPrimaryUri()); } else { return builder.credential(new ManagedIdentityCredentialBuilder() { - ... - }.build()).endpoint(endpointBuilder.apply(logger).getPrimaryUri()); + @Override + public ManagedIdentityCredential build() { + CredentialBuilderBaseHelper.getClientOptions(this).setExecutorService(executor); + return super.build(); + } + }.build()).endpoint(this.endpointBuilder.apply(logger).getPrimaryUri()); } }; Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about lambda variable capture, but endpointBuilder is already assigned as this.endpointBuilder before clientBuilder is defined, so the lambda captures the field reference correctly. The improved_code change to use this.endpointBuilder explicitly is a minor style improvement but doesn't fix a real bug in this context.	Low

---

Previous suggestions

Suggestions up to commit 37aa8d9

Category	Suggestion	Impact
General	Use valid Base64 key in test to avoid false positives The key setting in Azure storage is typically a Base64-encoded storage account key. Using a plain string like "any_fake_key" may cause a decoding error before the SettingsException for Workload Identity conflict is thrown, potentially making the test pass for the wrong reason or fail unexpectedly. Use a valid Base64-encoded value to ensure the exception is thrown due to the intended validation. plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java [238-241] public void testWorkloadIdentityWithKeyThrowsException() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("azure.client.default.account", "teststorage"); - secureSettings.setString("azure.client.default.key", "any_fake_key"); + secureSettings.setString("azure.client.default.key", Base64.getEncoder().encodeToString("any_fake_key".getBytes())); Suggestion importance[1-10]: 5 __ Why: This is a valid concern — if the key setting requires Base64 encoding and validation occurs before the Workload Identity check, the test could pass for the wrong reason. Using a properly Base64-encoded value ensures the test validates the intended behavior.	Low
	Validate required account field for Workload Identity The validation for WORKLOAD_IDENTITY only checks for key and sasToken, but does not validate that the account field is present. Workload Identity authentication requires a valid storage account name to construct the endpoint URL. If account is blank, the URI construction "https://" + account + ".blob." + tokenCredentialEndpointSuffix will produce an invalid URL without any error. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [282-288] if (type == TokenCredentialType.WORKLOAD_IDENTITY) { if (Strings.hasText(key) || Strings.hasText(sasToken)) { throw new SettingsException("Workload Identity cannot be used with key or sas_token"); } + if (!Strings.hasText(account)) { + throw new SettingsException("Workload Identity requires a valid storage account name"); + } } Suggestion importance[1-10]: 4 __ Why: Adding validation for the account field when using Workload Identity is a reasonable defensive check, but account is likely already validated elsewhere in the settings loading process, making this a minor improvement rather than a critical fix.	Low
Possible issue	Ensure lambda captures are effectively final The variable type is declared inside the if block but is captured in the lambda clientBuilder. In Java, variables captured by lambdas must be effectively final. Since type is declared inside the if block and the lambda is assigned to this.clientBuilder within the same block, this should work, but it's worth verifying that type is not reassigned anywhere. More critically, endpointBuilder is also referenced inside clientBuilder lambda, but endpointBuilder is assigned to this.endpointBuilder which is set just before — ensure the lambda captures the field reference correctly and not a stale value. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [300-314] +final TokenCredentialType credentialType = TokenCredentialType.valueOfType(tokenCredentialType); +if (credentialType == TokenCredentialType.WORKLOAD_IDENTITY) { + if (Strings.hasText(key) || Strings.hasText(sasToken)) { + throw new SettingsException("Workload Identity cannot be used with key or sas_token"); + } +} +this.endpointBuilder = (logger) -> { + ... +}; +final java.util.function.Function<ClientLogger, StorageEndpoint> capturedEndpointBuilder = this.endpointBuilder; this.clientBuilder = (builder, executor, logger) -> { - if (type == TokenCredentialType.WORKLOAD_IDENTITY) { + if (credentialType == TokenCredentialType.WORKLOAD_IDENTITY) { return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) - .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); + .endpoint(capturedEndpointBuilder.apply(logger).getPrimaryUri()); } else { return builder.credential(new ManagedIdentityCredentialBuilder() { - ... - }.build()).endpoint(endpointBuilder.apply(logger).getPrimaryUri()); + @Override + public ManagedIdentityCredential build() { + CredentialBuilderBaseHelper.getClientOptions(this).setExecutorService(executor); + return super.build(); + } + }.build()).endpoint(capturedEndpointBuilder.apply(logger).getPrimaryUri()); } }; Suggestion importance[1-10]: 4 __ Why: The concern about type being effectively final is valid in principle, but in the actual code type is declared once inside the if block and never reassigned, so it is already effectively final. The suggestion about endpointBuilder lambda capture is a minor concern since this.endpointBuilder is set just before and the lambda references it directly. The improved code adds unnecessary complexity without addressing a real bug.	Low

Suggestions up to commit 0fd7379

Category	Suggestion	Impact
Possible issue	Ensure lambda captures effectively final variable The type variable is declared inside the if (Strings.hasText(tokenCredentialType)) block but is referenced inside the lambda clientBuilder, which may cause a compilation error since type must be effectively final to be captured by the lambda. Ensure type is declared as a final local variable so it can be properly captured. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [300-303] -this.clientBuilder = (builder, executor, logger) -> { - if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) - .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); +final TokenCredentialType type = TokenCredentialType.valueOfType(tokenCredentialType); Suggestion importance[1-10]: 7 __ Why: The type variable is declared inside the if block and captured by a lambda, which requires it to be effectively final. In Java, a local variable used in a lambda must be effectively final - while type is only assigned once and is technically effectively final, making it explicitly final is a valid improvement for clarity and safety. This is a real potential concern worth addressing.	Medium
General	Add required field validation for Workload Identity The validation for WORKLOAD_IDENTITY only checks for key or sasToken being present, but does not validate that the account field is set, which is required for constructing the blob endpoint. Consider adding a check that account is non-empty when using Workload Identity, to provide a clear error message instead of a cryptic failure later. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [284-288] if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - if (Strings.hasText(key) || Strings.hasText(sasToken)) { - throw new SettingsException("Workload Identity cannot be used with key or sas_token"); - } - } + if (Strings.hasText(key) || Strings.hasText(sasToken)) { + throw new SettingsException("Workload Identity cannot be used with key or sas_token"); + } + if (!Strings.hasText(account)) { + throw new SettingsException("Account must be set when using Workload Identity"); + } +} Suggestion importance[1-10]: 4 __ Why: While adding an account validation is a reasonable defensive check, the account field is likely already validated elsewhere in the settings loading process, making this suggestion a minor improvement rather than a critical fix.	Low
	Use valid Base64 key in test The test uses "any_fake_key" as the key value, but Azure storage keys are expected to be Base64-encoded strings. If the AzureStorageSettings.load method validates or decodes the key before checking for Workload Identity conflicts, this could cause a different exception than SettingsException to be thrown, making the test unreliable. Use a valid Base64-encoded string as the fake key value. plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java [238-241] public void testWorkloadIdentityWithKeyThrowsException() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("azure.client.default.account", "teststorage"); - secureSettings.setString("azure.client.default.key", "any_fake_key"); + secureSettings.setString("azure.client.default.key", "dGVzdGtleXZhbHVlMTIzNDU2Nzg5MA=="); Suggestion importance[1-10]: 3 __ Why: The concern about Base64 encoding is valid in theory, but the test is specifically checking that a SettingsException is thrown due to the Workload Identity conflict check, which happens before any key decoding. The validation in AzureStorageSettings checks Strings.hasText(key) which would be true for any non-empty string, so the test should work correctly with "any_fake_key".	Low

Suggestions up to commit 57fc4ad

Category	Suggestion	Impact
Possible issue	Ensure lambda-captured variable is effectively final The type variable is declared inside the if (Strings.hasText(tokenCredentialType)) block but is referenced inside the lambda clientBuilder, which may cause a compilation error if type is not effectively final or if the lambda captures a variable from an enclosing scope incorrectly. Ensure type is declared as final to guarantee it is effectively final and safely captured by the lambda. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [300-303] -this.clientBuilder = (builder, executor, logger) -> { - if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) - .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); +final TokenCredentialType type = TokenCredentialType.valueOfType(tokenCredentialType); Suggestion importance[1-10]: 6 __ Why: The type variable is captured inside a lambda and must be effectively final in Java. While modern Java compilers may infer this, explicitly declaring it final is a valid improvement. However, the improved_code only shows the declaration line and doesn't reflect the full context of the change, making it slightly incomplete.	Low
General	Use valid Base64 key in test to avoid false failures The key value set in the test ("any_fake_key") is likely not a valid Base64-encoded key, which may cause the test to fail due to key parsing/decoding errors before reaching the SettingsException for Workload Identity conflict. Use a valid Base64-encoded string to avoid unintended failures. plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java [238-241] public void testWorkloadIdentityWithKeyThrowsException() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("azure.client.default.account", "teststorage"); - secureSettings.setString("azure.client.default.key", "any_fake_key"); + secureSettings.setString("azure.client.default.key", Base64.getEncoder().encodeToString("fakekey".getBytes())); Suggestion importance[1-10]: 5 __ Why: If the key field is decoded as Base64 during settings loading, using "any_fake_key" could cause a decoding error before the SettingsException for Workload Identity is thrown, making the test pass for the wrong reason or fail unexpectedly. Using a valid Base64 string would make the test more robust.	Low
	Validate required account field for Workload Identity The validation for WORKLOAD_IDENTITY only checks for key or sasToken but does not validate that the account field is present, which is required to construct the blob endpoint. If account is missing, the endpoint construction will silently produce a malformed URI. Add a check to ensure account is non-empty when using Workload Identity. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [284-288] if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - if (Strings.hasText(key) || Strings.hasText(sasToken)) { - throw new SettingsException("Workload Identity cannot be used with key or sas_token"); - } - } + if (Strings.hasText(key) || Strings.hasText(sasToken)) { + throw new SettingsException("Workload Identity cannot be used with key or sas_token"); + } + if (!Strings.hasText(account)) { + throw new SettingsException("Workload Identity requires a valid account name"); + } +} Suggestion importance[1-10]: 4 __ Why: Adding a check for the account field when using Workload Identity is a reasonable defensive measure, but account is likely already validated elsewhere in the settings loading process, making this a minor improvement rather than a critical fix.	Low

Suggestions up to commit 82854f0

Category	Suggestion	Impact
Possible issue	Ensure lambda-captured variable is effectively final The type variable is declared inside the if (Strings.hasText(tokenCredentialType)) block but is referenced inside the lambda clientBuilder, which may cause a compilation error if type is not effectively final or if the lambda captures a variable from an enclosing scope incorrectly. Ensure type is declared as final to guarantee it is effectively final and safely captured by the lambda. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [300-303] -this.clientBuilder = (builder, executor, logger) -> { - if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) - .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); +final TokenCredentialType type = TokenCredentialType.valueOfType(tokenCredentialType); Suggestion importance[1-10]: 6 __ Why: The type variable is used inside a lambda expression and must be effectively final in Java. While modern Java compilers may infer this, explicitly declaring it final is a good practice to prevent potential compilation issues and clarify intent. However, the improved_code only shows the declaration line and doesn't reflect the full context of the change needed.	Low
General	Use valid Base64 key in test to isolate exception cause The value "any_fake_key" is not a valid Base64-encoded key, which may cause the test to fail due to a key parsing/decoding error rather than the expected SettingsException about Workload Identity. Use a valid Base64-encoded string to ensure the exception is thrown for the correct reason. plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java [238-241] -public void testWorkloadIdentityWithKeyThrowsException() { - MockSecureSettings secureSettings = new MockSecureSettings(); - secureSettings.setString("azure.client.default.account", "teststorage"); - secureSettings.setString("azure.client.default.key", "any_fake_key"); +secureSettings.setString("azure.client.default.key", "dGVzdGtleQ=="); Suggestion importance[1-10]: 5 __ Why: This is a valid concern - if the key parsing fails before the Workload Identity validation check, the test would pass for the wrong reason. Using a valid Base64-encoded key ensures the SettingsException is thrown specifically due to the Workload Identity conflict check.	Low
	Handle unknown token credential types explicitly The validation only checks for WORKLOAD_IDENTITY, but there is no validation for other TokenCredentialType values that may be invalid or unsupported. If TokenCredentialType.valueOfType(tokenCredentialType) returns null or throws for an unrecognized type, the code may silently fall through to the MANAGED_IDENTITY path. Ensure that an unknown or null type is handled explicitly with an appropriate error. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [282-288] +if (type == null) { + throw new SettingsException("Unknown token_credential_type: " + tokenCredentialType); +} if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - if (Strings.hasText(key) || Strings.hasText(sasToken)) { - throw new SettingsException("Workload Identity cannot be used with key or sas_token"); - } - } + if (Strings.hasText(key) || Strings.hasText(sasToken)) { + throw new SettingsException("Workload Identity cannot be used with key or sas_token"); + } +} Suggestion importance[1-10]: 4 __ Why: This suggestion adds handling for unknown TokenCredentialType values, which improves robustness. However, the behavior of TokenCredentialType.valueOfType() for unknown types is not shown in the diff, so it's unclear if this null check is actually needed or if the method already throws for unknown types.	Low

Suggestions up to commit 507f851

Category	Suggestion	Impact
General	Use valid Base64 key in test to avoid false positives The key setting in Azure storage is expected to be a Base64-encoded value. Using "any_fake_key" as the key value may cause a decoding error before the SettingsException for Workload Identity is thrown, potentially making the test pass for the wrong reason or fail unexpectedly. Use a valid Base64-encoded string as the fake key value. plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java [238-241] public void testWorkloadIdentityWithKeyThrowsException() { MockSecureSettings secureSettings = new MockSecureSettings(); secureSettings.setString("azure.client.default.account", "teststorage"); - secureSettings.setString("azure.client.default.key", "any_fake_key"); + secureSettings.setString("azure.client.default.key", "YW55X2Zha2Vfa2V5"); // Base64 encoded value Suggestion importance[1-10]: 5 __ Why: This is a valid concern — if the key setting requires Base64 encoding and is decoded before the Workload Identity validation runs, the test could fail for the wrong reason. Using a valid Base64 string ensures the test accurately validates the intended behavior.	Low
	Add missing account validation for Workload Identity The validation for WORKLOAD_IDENTITY only checks if key or sasToken are present, but does not validate that the account field is set, which is required for constructing the blob endpoint URL. A missing account would result in a malformed URI like https://.blob.core.windows.net without a clear error message. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [284-288] if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - if (Strings.hasText(key) || Strings.hasText(sasToken)) { - throw new SettingsException("Workload Identity cannot be used with key or sas_token"); - } - } + if (Strings.hasText(key) || Strings.hasText(sasToken)) { + throw new SettingsException("Workload Identity cannot be used with key or sas_token"); + } + if (!Strings.hasText(account)) { + throw new SettingsException("Account must be set when using Workload Identity"); + } +} Suggestion importance[1-10]: 4 __ Why: Adding validation for the account field when using Workload Identity is a reasonable defensive check, but account is likely already validated elsewhere in the settings loading process, making this a minor improvement rather than a critical fix.	Low
Possible issue	Ensure lambda-captured variable is explicitly final The type variable is declared inside the if (Strings.hasText(tokenCredentialType)) block but is used inside the lambda clientBuilder, which may cause a compilation error since type must be effectively final to be captured by the lambda. While it is effectively final here, ensure it is explicitly declared as final to make the intent clear and avoid potential issues. plugins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureStorageSettings.java [300-303] -this.clientBuilder = (builder, executor, logger) -> { - if (type == TokenCredentialType.WORKLOAD_IDENTITY) { - return builder.credential(new WorkloadIdentityCredentialBuilder().executorService(executor).build()) - .endpoint(endpointBuilder.apply(logger).getPrimaryUri()); +final TokenCredentialType type = TokenCredentialType.valueOfType(tokenCredentialType); Suggestion importance[1-10]: 3 __ Why: The type variable is effectively final as-is, so this is a style suggestion rather than a bug fix. The improved_code only shows the declaration line and doesn't reflect the full context of the change, making it incomplete as a patch.	Low

[github-actions]

❌ Gradle check result for 5eebcad: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit ca4434f

[github-actions]

❌ Gradle check result for ca4434f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 21c33e1

[github-actions]

❌ Gradle check result for 21c33e1: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 19e865b

[github-actions]

❌ Gradle check result for 19e865b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit c9f8a9c

[github-actions]

❌ Gradle check result for c9f8a9c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit c260734

[github-actions]

❌ Gradle check result for c260734: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 1a23e22

[github-actions]

❌ Gradle check result for 1a23e22: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit b23522b

[github-actions]

❌ Gradle check result for b23522b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 1f82257

[github-actions]

❌ Gradle check result for 1f82257: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 507f851

[github-actions]

Persistent review updated to latest commit 82854f0

[github-actions]

Persistent review updated to latest commit 57fc4ad

[github-actions]

Persistent review updated to latest commit 0fd7379

[github-actions]

Persistent review updated to latest commit 37aa8d9

[github-actions]

❌ Gradle check result for 37aa8d9: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 84f4371

[github-actions]

✅ Gradle check result for 84f4371: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.18%. Comparing base (1bb2c64) to head (84f4371).

Files with missing lines	Patch %	Lines
...earch/repositories/azure/AzureStorageSettings.java	76.92%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #21185   +/-   ##
=========================================
  Coverage     73.18%   73.18%           
+ Complexity    72939    72926   -13     
=========================================
  Files          5888     5888           
  Lines        333169   333179   +10     
  Branches      48058    48060    +2     
=========================================
+ Hits         243820   243841   +21     
- Misses        69855    69861    +6     
+ Partials      19494    19477   -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.