From 227c33216e7677706880df6e2437094b94148c27 Mon Sep 17 00:00:00 2001 From: Sumit Bhattacharya Date: Wed, 11 Mar 2026 23:38:22 +0000 Subject: [PATCH 1/3] feat: Add per-secret validate_at_bootstrap flag for credential validation - Add validate_at_bootstrap boolean field to AwsSecretManagerConfiguration - Default value is true (safe-by-default - validates credentials at bootstrap) - When set to false, secret retrieval is deferred until first access - Implement lazy-loading logic in AwsSecretsSupplier for deferred secrets - Add comprehensive unit and integration tests - Maintain backward compatibility (default behavior unchanged) This allows users to disable credential validation per-secret when credentials are not available at bootstrap time, while maintaining fail-fast behavior by default for production safety. Resolves issue where DataPrepper fails to start with invalid credentials by providing per-secret control over bootstrap validation. Signed-off-by: Sumit Bhattacharya --- .../aws/AwsSecretManagerConfiguration.java | 7 + .../plugins/aws/AwsSecretsSupplier.java | 30 ++++- ...rConfigurationValidateAtBootstrapTest.java | 74 +++++++++++ .../plugins/aws/AwsSecretPluginIT.java | 2 + .../aws/AwsSecretsSupplierLazyLoadTest.java | 125 ++++++++++++++++++ .../plugins/aws/AwsSecretsSupplierTest.java | 1 + ...data-prepper-config-secret-validation.yaml | 21 +++ 7 files changed, 258 insertions(+), 2 deletions(-) create mode 100644 data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java create mode 100644 data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java create mode 100644 examples/config/data-prepper-config-secret-validation.yaml diff --git a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java index 84b2cf680c..cf4001c311 100644 --- a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java +++ b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java @@ -52,6 +52,9 @@ public class AwsSecretManagerConfiguration { @JsonProperty("disable_refresh") private boolean disableRefresh = false; + @JsonProperty("validate_at_bootstrap") + private boolean validateAtBootstrap = true; // Default: true (safe-by-default) + public String getAwsSecretId() { return awsSecretId; } @@ -68,6 +71,10 @@ public boolean isDisableRefresh() { return disableRefresh; } + public boolean isValidateAtBootstrap() { + return validateAtBootstrap; + } + public SecretsManagerClient createSecretManagerClient(final AwsCredentialsSupplier awsCredentialsSupplier) { final AwsCredentialsProvider awsCredentialsProvider = awsCredentialsSupplier.getProvider(AwsCredentialsOptions.builder() .withRegion(this.awsRegion) diff --git a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java index ac8cddf5db..3cccde5d41 100644 --- a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java +++ b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java @@ -31,6 +31,8 @@ public class AwsSecretsSupplier implements SecretsSupplier { static final TypeReference> MAP_TYPE_REFERENCE = new TypeReference<>() { }; private static final Logger LOG = LoggerFactory.getLogger(AwsSecretsSupplier.class); + private static final Object NOT_LOADED_SENTINEL = new Object(); // Sentinel to indicate secret not loaded yet + private final SecretValueDecoder secretValueDecoder; private final ObjectMapper objectMapper; private final Map awsSecretManagerConfigurationMap; @@ -58,6 +60,14 @@ private ConcurrentMap toSecretMap( final AwsSecretManagerConfiguration awsSecretManagerConfiguration = awsSecretManagerConfigurationMap.get(secretConfigurationId); final SecretsManagerClient secretsManagerClient = entry.getValue(); + + // Check if validation at bootstrap is disabled for this secret + if (!awsSecretManagerConfiguration.isValidateAtBootstrap()) { + LOG.info("Skipping secret retrieval at bootstrap for secret: {} (validate_at_bootstrap=false)", + awsSecretManagerConfiguration.getAwsSecretId()); + return NOT_LOADED_SENTINEL; // Mark as not loaded, will be loaded on first access + } + return retrieveSecretsFromSecretManager(awsSecretManagerConfiguration, secretsManagerClient); })); } @@ -77,7 +87,15 @@ public Object retrieveValue(String secretId, String key) { if (!secretIdToValue.containsKey(secretId)) { throw new IllegalArgumentException(String.format("Unable to find secretId: %s", secretId)); } - final Object keyValuePairs = secretIdToValue.get(secretId); + + // Check if secret was skipped at bootstrap and needs to be loaded now + Object keyValuePairs = secretIdToValue.get(secretId); + if (keyValuePairs == NOT_LOADED_SENTINEL) { + LOG.info("Secret {} was not loaded at bootstrap, loading now on first access...", secretId); + refresh(secretId); + keyValuePairs = secretIdToValue.get(secretId); + } + if (!(keyValuePairs instanceof Map)) { throw new IllegalArgumentException(String.format("The value under secretId: %s is not a valid json.", secretId)); @@ -95,8 +113,16 @@ public Object retrieveValue(String secretId) { if (!secretIdToValue.containsKey(secretId)) { throw new IllegalArgumentException(String.format("Unable to find secretId: %s", secretId)); } + + // Check if secret was skipped at bootstrap and needs to be loaded now + Object secretValue = secretIdToValue.get(secretId); + if (secretValue == NOT_LOADED_SENTINEL) { + LOG.info("Secret {} was not loaded at bootstrap, loading now on first access...", secretId); + refresh(secretId); + secretValue = secretIdToValue.get(secretId); + } + try { - final Object secretValue = secretIdToValue.get(secretId); return secretValue instanceof Map ? objectMapper.writeValueAsString(secretValue) : secretValue; } catch (JsonProcessingException e) { diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java new file mode 100644 index 0000000000..4c4d140b32 --- /dev/null +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java @@ -0,0 +1,74 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.dataprepper.plugins.aws; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; + +class AwsSecretManagerConfigurationValidateAtBootstrapTest { + + private final ObjectMapper objectMapper = new ObjectMapper(new YAMLFactory()) + .registerModule(new JavaTimeModule()); + + @Test + void testDefaultValidateAtBootstrap() { + final AwsSecretManagerConfiguration config = new AwsSecretManagerConfiguration(); + + // Default should be true (safe-by-default) + assertThat(config.isValidateAtBootstrap(), equalTo(true)); + } + + @Test + void testValidateAtBootstrapFromYaml_Enabled() throws Exception { + final String yaml = + "secret_id: my-secret\n" + + "region: us-east-1\n" + + "refresh_interval: PT1H\n" + + "validate_at_bootstrap: true\n"; + + final AwsSecretManagerConfiguration config = + objectMapper.readValue(yaml, AwsSecretManagerConfiguration.class); + + assertThat(config.isValidateAtBootstrap(), equalTo(true)); + } + + @Test + void testValidateAtBootstrapFromYaml_Disabled() throws Exception { + final String yaml = + "secret_id: my-secret\n" + + "region: us-east-1\n" + + "refresh_interval: PT1H\n" + + "validate_at_bootstrap: false\n"; + + final AwsSecretManagerConfiguration config = + objectMapper.readValue(yaml, AwsSecretManagerConfiguration.class); + + assertThat(config.isValidateAtBootstrap(), equalTo(false)); + } + + @Test + void testValidateAtBootstrapFromYaml_NotSpecified_UsesDefault() throws Exception { + final String yaml = + "secret_id: my-secret\n" + + "region: us-east-1\n" + + "refresh_interval: PT1H\n"; + + final AwsSecretManagerConfiguration config = + objectMapper.readValue(yaml, AwsSecretManagerConfiguration.class); + + // Should default to true when not specified + assertThat(config.isValidateAtBootstrap(), equalTo(true)); + } +} diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java index 5439111c08..92cfcb049f 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java @@ -99,6 +99,7 @@ void testInitializationWithNonNullConfig() { when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( Map.of(TEST_SECRET_CONFIG_ID, awsSecretManagerConfiguration)); when(awsSecretManagerConfiguration.getRefreshInterval()).thenReturn(testInterval); + when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default behavior when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); @@ -133,6 +134,7 @@ void testInitializationWithDisableRefresh() { when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( Map.of(TEST_SECRET_CONFIG_ID, awsSecretManagerConfiguration)); when(awsSecretManagerConfiguration.isDisableRefresh()).thenReturn(true); + when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default behavior when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java new file mode 100644 index 0000000000..c5dcc8c75b --- /dev/null +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java @@ -0,0 +1,125 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.dataprepper.plugins.aws; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.dataprepper.aws.api.AwsCredentialsSupplier; +import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient; +import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueRequest; +import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueResponse; + +import java.util.Map; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for lazy-loading behavior when validate_at_bootstrap is false. + */ +@ExtendWith(MockitoExtension.class) +class AwsSecretsSupplierLazyLoadTest { + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final String TEST_SECRET_ID = "test-secret"; + private static final String TEST_KEY = "test-key"; + private static final String TEST_VALUE = "test-value"; + + @Mock + private SecretValueDecoder secretValueDecoder; + + @Mock + private AwsSecretPluginConfig awsSecretPluginConfig; + + @Mock + private AwsSecretManagerConfiguration awsSecretManagerConfiguration; + + @Mock + private SecretsManagerClient secretsManagerClient; + + @Mock + private GetSecretValueRequest getSecretValueRequest; + + @Mock + private GetSecretValueResponse getSecretValueResponse; + + @Mock + private AwsCredentialsSupplier awsCredentialsSupplier; + + @Test + void testSecretWithValidateAtBootstrapFalse_LoadsOnFirstAccess() throws JsonProcessingException { + // Given: Secret configured with validate_at_bootstrap=false + when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( + Map.of(TEST_SECRET_ID, awsSecretManagerConfiguration) + ); + when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(false); // Skip at bootstrap + when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); + when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); + when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(OBJECT_MAPPER.writeValueAsString( + Map.of(TEST_KEY, TEST_VALUE) + )); + when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); + + // When: AwsSecretsSupplier is constructed + final AwsSecretsSupplier supplier = new AwsSecretsSupplier( + secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER, awsCredentialsSupplier + ); + + // Then: Secret is NOT retrieved at construction time + verify(secretsManagerClient, never()).getSecretValue(eq(getSecretValueRequest)); + + // When: Secret is accessed for the first time + final Object value = supplier.retrieveValue(TEST_SECRET_ID, TEST_KEY); + + // Then: Secret is loaded on-demand + verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); + assertThat(value, equalTo(TEST_VALUE)); + } + + @Test + void testSecretWithValidateAtBootstrapTrue_LoadsAtConstruction() throws JsonProcessingException { + // Given: Secret configured with validate_at_bootstrap=true (default) + when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( + Map.of(TEST_SECRET_ID, awsSecretManagerConfiguration) + ); + when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Load at bootstrap + when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); + when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); + when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(OBJECT_MAPPER.writeValueAsString( + Map.of(TEST_KEY, TEST_VALUE) + )); + when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); + + // When: AwsSecretsSupplier is constructed + final AwsSecretsSupplier supplier = new AwsSecretsSupplier( + secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER, awsCredentialsSupplier + ); + + // Then: Secret IS retrieved at construction time + verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); + + // When: Secret is accessed + final Object value = supplier.retrieveValue(TEST_SECRET_ID, TEST_KEY); + + // Then: No additional retrieval (already loaded) + verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); + assertThat(value, equalTo(TEST_VALUE)); + } +} diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java index 6ba9dd2917..2abf68a055 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java @@ -86,6 +86,7 @@ class AwsSecretsSupplierTest { @BeforeEach void setUp() throws JsonProcessingException { when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); + when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default: validate at bootstrap when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( Map.of(TEST_AWS_SECRET_CONFIGURATION_NAME, awsSecretManagerConfiguration) ); diff --git a/examples/config/data-prepper-config-secret-validation.yaml b/examples/config/data-prepper-config-secret-validation.yaml new file mode 100644 index 0000000000..2ecd23b58b --- /dev/null +++ b/examples/config/data-prepper-config-secret-validation.yaml @@ -0,0 +1,21 @@ +# Example DataPrepper configuration with per-secret credential validation control + +extensions: + aws: + secrets: + # Example 1: Secret with validation enabled (default) + production_secret: + secret_id: prod-db-credentials + region: us-east-1 + sts_role_arn: arn:aws:iam::123456789012:role/DataPrepperRole + refresh_interval: PT1H + validate_at_bootstrap: true # Default - validates credentials at startup + + # Example 2: Secret with validation disabled + # Use this when credentials may not be available at bootstrap + delayed_secret: + secret_id: delayed-credentials + region: us-west-2 + sts_role_arn: arn:aws:iam::123456789012:role/DataPrepperRole + refresh_interval: PT1H + validate_at_bootstrap: false # Skip validation at startup From 7360f5906cc5bdd6054717f4cd591965da05031f Mon Sep 17 00:00:00 2001 From: Sumit Bhattacharya Date: Thu, 12 Mar 2026 16:19:42 +0000 Subject: [PATCH 2/3] refactor: Address PR feedback - Rename validate_at_bootstrap to skip_validation_on_start - Change default from true to false (validate by default per DataPrepper convention) - Remove ellipses from log messages - Extract duplicate lazy-loading code into loadSecretIfNeeded() helper method - Update tests to use random values as member variables initialized in @BeforeEach - Update all test mocks to use new method name - Remove example configuration file - Remove accidentally committed build artifacts Signed-off-by: Sumit Bhattacharya --- .../aws/AwsSecretManagerConfiguration.java | 8 +-- .../plugins/aws/AwsSecretsSupplier.java | 40 ++++++++------ ...rConfigurationValidateAtBootstrapTest.java | 32 +++-------- .../plugins/aws/AwsSecretPluginIT.java | 4 +- .../aws/AwsSecretsSupplierLazyLoadTest.java | 55 +++++++++++-------- .../plugins/aws/AwsSecretsSupplierTest.java | 2 +- ...data-prepper-config-secret-validation.yaml | 21 ------- 7 files changed, 71 insertions(+), 91 deletions(-) delete mode 100644 examples/config/data-prepper-config-secret-validation.yaml diff --git a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java index cf4001c311..42839f13ca 100644 --- a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java +++ b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfiguration.java @@ -52,8 +52,8 @@ public class AwsSecretManagerConfiguration { @JsonProperty("disable_refresh") private boolean disableRefresh = false; - @JsonProperty("validate_at_bootstrap") - private boolean validateAtBootstrap = true; // Default: true (safe-by-default) + @JsonProperty("skip_validation_on_start") + private boolean skipValidationOnStart = false; // Default: false (validate by default) public String getAwsSecretId() { return awsSecretId; @@ -71,8 +71,8 @@ public boolean isDisableRefresh() { return disableRefresh; } - public boolean isValidateAtBootstrap() { - return validateAtBootstrap; + public boolean isSkipValidationOnStart() { + return skipValidationOnStart; } public SecretsManagerClient createSecretManagerClient(final AwsCredentialsSupplier awsCredentialsSupplier) { diff --git a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java index 3cccde5d41..a50484e839 100644 --- a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java +++ b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java @@ -61,9 +61,9 @@ private ConcurrentMap toSecretMap( awsSecretManagerConfigurationMap.get(secretConfigurationId); final SecretsManagerClient secretsManagerClient = entry.getValue(); - // Check if validation at bootstrap is disabled for this secret - if (!awsSecretManagerConfiguration.isValidateAtBootstrap()) { - LOG.info("Skipping secret retrieval at bootstrap for secret: {} (validate_at_bootstrap=false)", + // Check if validation on start is skipped for this secret + if (awsSecretManagerConfiguration.isSkipValidationOnStart()) { + LOG.info("Skipping secret retrieval on start for secret: {} (skip_validation_on_start=true)", awsSecretManagerConfiguration.getAwsSecretId()); return NOT_LOADED_SENTINEL; // Mark as not loaded, will be loaded on first access } @@ -88,13 +88,8 @@ public Object retrieveValue(String secretId, String key) { throw new IllegalArgumentException(String.format("Unable to find secretId: %s", secretId)); } - // Check if secret was skipped at bootstrap and needs to be loaded now - Object keyValuePairs = secretIdToValue.get(secretId); - if (keyValuePairs == NOT_LOADED_SENTINEL) { - LOG.info("Secret {} was not loaded at bootstrap, loading now on first access...", secretId); - refresh(secretId); - keyValuePairs = secretIdToValue.get(secretId); - } + // Load secret if it was skipped on start + final Object keyValuePairs = loadSecretIfNeeded(secretId); if (!(keyValuePairs instanceof Map)) { throw new IllegalArgumentException(String.format("The value under secretId: %s is not a valid json.", @@ -114,13 +109,8 @@ public Object retrieveValue(String secretId) { throw new IllegalArgumentException(String.format("Unable to find secretId: %s", secretId)); } - // Check if secret was skipped at bootstrap and needs to be loaded now - Object secretValue = secretIdToValue.get(secretId); - if (secretValue == NOT_LOADED_SENTINEL) { - LOG.info("Secret {} was not loaded at bootstrap, loading now on first access...", secretId); - refresh(secretId); - secretValue = secretIdToValue.get(secretId); - } + // Load secret if it was skipped on start + final Object secretValue = loadSecretIfNeeded(secretId); try { return secretValue instanceof Map ? objectMapper.writeValueAsString(secretValue) : @@ -131,6 +121,22 @@ public Object retrieveValue(String secretId) { } } + /** + * Loads a secret if it was skipped on start (lazy-loading). + * + * @param secretId The secret configuration ID + * @return The loaded secret value + */ + private Object loadSecretIfNeeded(String secretId) { + Object value = secretIdToValue.get(secretId); + if (value == NOT_LOADED_SENTINEL) { + LOG.info("Secret {} was not loaded on start, loading now on first access.", secretId); + refresh(secretId); + value = secretIdToValue.get(secretId); + } + return value; + } + @Override public void refresh(String secretConfigId) { diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java index 4c4d140b32..d0988adbe8 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretManagerConfigurationValidateAtBootstrapTest.java @@ -23,52 +23,38 @@ class AwsSecretManagerConfigurationValidateAtBootstrapTest { .registerModule(new JavaTimeModule()); @Test - void testDefaultValidateAtBootstrap() { + void testDefaultSkipValidationOnStart() { final AwsSecretManagerConfiguration config = new AwsSecretManagerConfiguration(); - // Default should be true (safe-by-default) - assertThat(config.isValidateAtBootstrap(), equalTo(true)); + // Default should be false (validate by default) + assertThat(config.isSkipValidationOnStart(), equalTo(false)); } @Test - void testValidateAtBootstrapFromYaml_Enabled() throws Exception { + void testSkipValidationOnStartFromYaml_Enabled() throws Exception { final String yaml = "secret_id: my-secret\n" + "region: us-east-1\n" + "refresh_interval: PT1H\n" + - "validate_at_bootstrap: true\n"; + "skip_validation_on_start: true\n"; final AwsSecretManagerConfiguration config = objectMapper.readValue(yaml, AwsSecretManagerConfiguration.class); - assertThat(config.isValidateAtBootstrap(), equalTo(true)); + assertThat(config.isSkipValidationOnStart(), equalTo(true)); } @Test - void testValidateAtBootstrapFromYaml_Disabled() throws Exception { + void testSkipValidationOnStartFromYaml_Disabled() throws Exception { final String yaml = "secret_id: my-secret\n" + "region: us-east-1\n" + "refresh_interval: PT1H\n" + - "validate_at_bootstrap: false\n"; + "skip_validation_on_start: false\n"; final AwsSecretManagerConfiguration config = objectMapper.readValue(yaml, AwsSecretManagerConfiguration.class); - assertThat(config.isValidateAtBootstrap(), equalTo(false)); - } - - @Test - void testValidateAtBootstrapFromYaml_NotSpecified_UsesDefault() throws Exception { - final String yaml = - "secret_id: my-secret\n" + - "region: us-east-1\n" + - "refresh_interval: PT1H\n"; - - final AwsSecretManagerConfiguration config = - objectMapper.readValue(yaml, AwsSecretManagerConfiguration.class); - - // Should default to true when not specified - assertThat(config.isValidateAtBootstrap(), equalTo(true)); + assertThat(config.isSkipValidationOnStart(), equalTo(false)); } } diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java index 92cfcb049f..91665681a3 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPluginIT.java @@ -99,7 +99,7 @@ void testInitializationWithNonNullConfig() { when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( Map.of(TEST_SECRET_CONFIG_ID, awsSecretManagerConfiguration)); when(awsSecretManagerConfiguration.getRefreshInterval()).thenReturn(testInterval); - when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default behavior + when(awsSecretManagerConfiguration.isSkipValidationOnStart()).thenReturn(false); // Default behavior when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); @@ -134,7 +134,7 @@ void testInitializationWithDisableRefresh() { when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( Map.of(TEST_SECRET_CONFIG_ID, awsSecretManagerConfiguration)); when(awsSecretManagerConfiguration.isDisableRefresh()).thenReturn(true); - when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default behavior + when(awsSecretManagerConfiguration.isSkipValidationOnStart()).thenReturn(false); // Default behavior when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java index c5dcc8c75b..5cc90199d5 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java @@ -22,6 +22,7 @@ import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueResponse; import java.util.Map; +import java.util.UUID; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; @@ -32,15 +33,15 @@ import static org.mockito.Mockito.when; /** - * Tests for lazy-loading behavior when validate_at_bootstrap is false. + * Tests for lazy-loading behavior when skip_validation_on_start is true. */ @ExtendWith(MockitoExtension.class) class AwsSecretsSupplierLazyLoadTest { - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - private static final String TEST_SECRET_ID = "test-secret"; - private static final String TEST_KEY = "test-key"; - private static final String TEST_VALUE = "test-value"; + private ObjectMapper objectMapper; + private String testSecretId; + private String testKey; + private String testValue; @Mock private SecretValueDecoder secretValueDecoder; @@ -63,63 +64,71 @@ class AwsSecretsSupplierLazyLoadTest { @Mock private AwsCredentialsSupplier awsCredentialsSupplier; + @BeforeEach + void setUp() { + objectMapper = new ObjectMapper(); + testSecretId = UUID.randomUUID().toString(); + testKey = UUID.randomUUID().toString(); + testValue = UUID.randomUUID().toString(); + } + @Test - void testSecretWithValidateAtBootstrapFalse_LoadsOnFirstAccess() throws JsonProcessingException { - // Given: Secret configured with validate_at_bootstrap=false + void testSecretWithSkipValidationOnStartTrue_LoadsOnFirstAccess() throws JsonProcessingException { + // Given: Secret configured with skip_validation_on_start=true when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( - Map.of(TEST_SECRET_ID, awsSecretManagerConfiguration) + Map.of(testSecretId, awsSecretManagerConfiguration) ); - when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(false); // Skip at bootstrap + when(awsSecretManagerConfiguration.isSkipValidationOnStart()).thenReturn(true); // Skip on start when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); - when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(OBJECT_MAPPER.writeValueAsString( - Map.of(TEST_KEY, TEST_VALUE) + when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(objectMapper.writeValueAsString( + Map.of(testKey, testValue) )); when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); // When: AwsSecretsSupplier is constructed final AwsSecretsSupplier supplier = new AwsSecretsSupplier( - secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER, awsCredentialsSupplier + secretValueDecoder, awsSecretPluginConfig, objectMapper, awsCredentialsSupplier ); // Then: Secret is NOT retrieved at construction time verify(secretsManagerClient, never()).getSecretValue(eq(getSecretValueRequest)); // When: Secret is accessed for the first time - final Object value = supplier.retrieveValue(TEST_SECRET_ID, TEST_KEY); + final Object value = supplier.retrieveValue(testSecretId, testKey); // Then: Secret is loaded on-demand verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); - assertThat(value, equalTo(TEST_VALUE)); + assertThat(value, equalTo(testValue)); } @Test - void testSecretWithValidateAtBootstrapTrue_LoadsAtConstruction() throws JsonProcessingException { - // Given: Secret configured with validate_at_bootstrap=true (default) + void testSecretWithSkipValidationOnStartFalse_LoadsAtConstruction() throws JsonProcessingException { + // Given: Secret configured with skip_validation_on_start=false (default) when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( - Map.of(TEST_SECRET_ID, awsSecretManagerConfiguration) + Map.of(testSecretId, awsSecretManagerConfiguration) ); - when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Load at bootstrap + when(awsSecretManagerConfiguration.isSkipValidationOnStart()).thenReturn(false); // Load on start when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); - when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(OBJECT_MAPPER.writeValueAsString( - Map.of(TEST_KEY, TEST_VALUE) + when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(objectMapper.writeValueAsString( + Map.of(testKey, testValue) )); when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); // When: AwsSecretsSupplier is constructed final AwsSecretsSupplier supplier = new AwsSecretsSupplier( - secretValueDecoder, awsSecretPluginConfig, OBJECT_MAPPER, awsCredentialsSupplier + secretValueDecoder, awsSecretPluginConfig, objectMapper, awsCredentialsSupplier ); // Then: Secret IS retrieved at construction time verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); // When: Secret is accessed - final Object value = supplier.retrieveValue(TEST_SECRET_ID, TEST_KEY); + final Object value = supplier.retrieveValue(testSecretId, testKey); // Then: No additional retrieval (already loaded) verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); - assertThat(value, equalTo(TEST_VALUE)); + assertThat(value, equalTo(testValue)); } } diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java index 2abf68a055..37c4178ed7 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java @@ -86,7 +86,7 @@ class AwsSecretsSupplierTest { @BeforeEach void setUp() throws JsonProcessingException { when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); - when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default: validate at bootstrap + when(awsSecretManagerConfiguration.isSkipValidationOnStart()).thenReturn(false); // Default: validate on start when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( Map.of(TEST_AWS_SECRET_CONFIGURATION_NAME, awsSecretManagerConfiguration) ); diff --git a/examples/config/data-prepper-config-secret-validation.yaml b/examples/config/data-prepper-config-secret-validation.yaml deleted file mode 100644 index 2ecd23b58b..0000000000 --- a/examples/config/data-prepper-config-secret-validation.yaml +++ /dev/null @@ -1,21 +0,0 @@ -# Example DataPrepper configuration with per-secret credential validation control - -extensions: - aws: - secrets: - # Example 1: Secret with validation enabled (default) - production_secret: - secret_id: prod-db-credentials - region: us-east-1 - sts_role_arn: arn:aws:iam::123456789012:role/DataPrepperRole - refresh_interval: PT1H - validate_at_bootstrap: true # Default - validates credentials at startup - - # Example 2: Secret with validation disabled - # Use this when credentials may not be available at bootstrap - delayed_secret: - secret_id: delayed-credentials - region: us-west-2 - sts_role_arn: arn:aws:iam::123456789012:role/DataPrepperRole - refresh_interval: PT1H - validate_at_bootstrap: false # Skip validation at startup From ae87113bd1d7d9311535cec5a9203f7977daab21 Mon Sep 17 00:00:00 2001 From: Sumit Bhattacharya Date: Mon, 23 Mar 2026 15:05:14 +0000 Subject: [PATCH 3/3] fix: Make lazy-loading of secrets atomic and handle updateValue with unloaded secrets Use ConcurrentMap.compute() in loadSecretIfNeeded() to ensure the sentinel check and secret retrieval are performed atomically, avoiding race conditions with concurrent access. Add loadSecretIfNeeded() call at the beginning of updateValue() to ensure secrets are loaded before update logic runs, preventing the NOT_LOADED_SENTINEL from being treated as a plain value store. Signed-off-by: Sumit Bhattacharya --- .../plugins/aws/AwsSecretsSupplier.java | 21 ++++++---- .../aws/AwsSecretsSupplierLazyLoadTest.java | 42 +++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java index a50484e839..8fc84477d1 100644 --- a/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java +++ b/data-prepper-plugins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java @@ -123,18 +123,21 @@ public Object retrieveValue(String secretId) { /** * Loads a secret if it was skipped on start (lazy-loading). - * + * Uses {@link ConcurrentMap#compute} to ensure atomicity of the sentinel check and refresh. + * * @param secretId The secret configuration ID * @return The loaded secret value */ private Object loadSecretIfNeeded(String secretId) { - Object value = secretIdToValue.get(secretId); - if (value == NOT_LOADED_SENTINEL) { - LOG.info("Secret {} was not loaded on start, loading now on first access.", secretId); - refresh(secretId); - value = secretIdToValue.get(secretId); - } - return value; + return secretIdToValue.compute(secretId, (key, currentValue) -> { + if (currentValue == NOT_LOADED_SENTINEL) { + LOG.info("Secret {} was not loaded on start, loading now on first access.", key); + final AwsSecretManagerConfiguration config = awsSecretManagerConfigurationMap.get(key); + final SecretsManagerClient client = secretsManagerClientMap.get(key); + return retrieveSecretsFromSecretManager(config, client); + } + return currentValue; + }); } @@ -184,6 +187,8 @@ public String updateValue(String secretId, Object newValue) { @Override public String updateValue(String secretId, String keyToUpdate, Object newValue) { + // Ensure the secret is loaded before attempting to update + loadSecretIfNeeded(secretId); Object currentSecretStore = secretIdToValue.get(secretId); if (currentSecretStore instanceof Map) { if (keyToUpdate == null) { diff --git a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java index 5cc90199d5..216bc2574d 100644 --- a/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java +++ b/data-prepper-plugins/aws-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierLazyLoadTest.java @@ -20,12 +20,15 @@ import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient; import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueRequest; import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueResponse; +import software.amazon.awssdk.services.secretsmanager.model.PutSecretValueRequest; +import software.amazon.awssdk.services.secretsmanager.model.PutSecretValueResponse; import java.util.Map; import java.util.UUID; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -61,6 +64,12 @@ class AwsSecretsSupplierLazyLoadTest { @Mock private GetSecretValueResponse getSecretValueResponse; + @Mock + private PutSecretValueRequest putSecretValueRequest; + + @Mock + private PutSecretValueResponse putSecretValueResponse; + @Mock private AwsCredentialsSupplier awsCredentialsSupplier; @@ -131,4 +140,37 @@ void testSecretWithSkipValidationOnStartFalse_LoadsAtConstruction() throws JsonP verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); assertThat(value, equalTo(testValue)); } + + @Test + void testUpdateValue_withSkipValidationOnStart_loadsSecretBeforeUpdate() throws JsonProcessingException { + // Given: Secret configured with skip_validation_on_start=true + when(awsSecretPluginConfig.getAwsSecretManagerConfigurationMap()).thenReturn( + Map.of(testSecretId, awsSecretManagerConfiguration) + ); + when(awsSecretManagerConfiguration.isSkipValidationOnStart()).thenReturn(true); + when(awsSecretManagerConfiguration.createSecretManagerClient(awsCredentialsSupplier)).thenReturn(secretsManagerClient); + when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest); + when(secretValueDecoder.decode(eq(getSecretValueResponse))).thenReturn(objectMapper.writeValueAsString( + Map.of(testKey, testValue) + )); + when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse); + when(awsSecretManagerConfiguration.putSecretValueRequest(any())).thenReturn(putSecretValueRequest); + when(secretsManagerClient.putSecretValue(eq(putSecretValueRequest))).thenReturn(putSecretValueResponse); + final String newVersionId = UUID.randomUUID().toString(); + when(putSecretValueResponse.versionId()).thenReturn(newVersionId); + + final AwsSecretsSupplier supplier = new AwsSecretsSupplier( + secretValueDecoder, awsSecretPluginConfig, objectMapper, awsCredentialsSupplier + ); + + // Then: Secret is NOT retrieved at construction time + verify(secretsManagerClient, never()).getSecretValue(eq(getSecretValueRequest)); + + // When: updateValue is called before any retrieveValue + final String versionId = supplier.updateValue(testSecretId, testKey, "newValue"); + + // Then: Secret was loaded on-demand and update succeeded + verify(secretsManagerClient, times(1)).getSecretValue(eq(getSecretValueRequest)); + assertThat(versionId, equalTo(newVersionId)); + } }