feat: Add configurable AWS credential validation at bootstrap by BhattacharyaSumit · Pull Request #6629 · opensearch-project/data-prepper

BhattacharyaSumit · 2026-03-11T17:03:13Z

Description

Adds a skip_validation_on_start boolean field to AWS Secrets Manager configuration, allowing per-secret control over credential validation at DataPrepper bootstrap.

Motivation

DataPrepper currently fails to start if AWS credentials for secrets are invalid at bootstrap, with no way to defer validation. This blocks deployment scenarios where credentials may not be available at startup but will be provided later during runtime.

Changes

Added skip_validation_on_start field to AwsSecretManagerConfiguration (default: false)
Implemented lazy-loading in AwsSecretsSupplier for secrets with validation disabled
Secrets with skip_validation_on_start=true are loaded on first access instead of at bootstrap
Added comprehensive unit and integration tests
Created example configuration

Configuration Example

extensions:
  aws:
    secrets:
      my_secret:
        secret_id: my-secret-id
        region: us-east-1
        skip_validation_on_start: true  # Skip validation at bootstrap

Issues Resolved

Resolves #[Issue number to be closed when this PR is merged]

Check List

[ X] New functionality includes testing.
New functionality has a documentation issue. Please link to it in this PR.
- New functionality has javadoc added
[ X] Commits are signed with a real name per the DCO

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.

mzurita-amz · 2026-03-11T17:14:53Z

Maybe a better configuration experience could be

extensions:
  aws:
    secrets:
       <secret-id>
         ...
         assume_at_bootstrap: ENABLED|DISABLED

…tion - 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 <sumit4739@gmail.com>

dlvenable

Thanks for this change @BhattacharyaSumit !

dlvenable · 2026-03-12T15:15:18Z

+class AwsSecretsSupplierLazyLoadTest {
+
+    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+    private static final String TEST_SECRET_ID = "test-secret";


Use random values and initialize them in @BeforeEach. Make them member variables instead of static.

dlvenable · 2026-03-12T15:19:06Z

    private boolean disableRefresh = false;

+    @JsonProperty("validate_at_bootstrap")
+    private boolean validateAtBootstrap = true;  // Default: true (safe-by-default)


We generally make our boolean configurations false by default. This is a convention offered to end-users to help with expectations.

Also we don't use the term bootstrap much.

I'd tend to think these are better names:

skip_initial_validation

skip_validation

skip_validation_on_start

dlvenable · 2026-03-12T15:19:33Z

+
+                    // 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)", 


Update this value as well.

dlvenable · 2026-03-12T15:20:02Z

+        // 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);


Do not use ellipses like this. Just end the sentence with a period.

dlvenable · 2026-03-12T15:20:13Z

+        // 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);


Similar comment about ellipses.

dlvenable · 2026-03-12T15:21:03Z

        }
+
+        // Check if secret was skipped at bootstrap and needs to be loaded now
+        Object secretValue = secretIdToValue.get(secretId);


This is duplicate code. Consolidate it with the code above.

dlvenable · 2026-03-12T15:21:24Z

+        final AwsSecretManagerConfiguration config = new AwsSecretManagerConfiguration();
+
+        // Default should be true (safe-by-default)
+        assertThat(config.isValidateAtBootstrap(), equalTo(true));


You won't need this with the updated default of false.

dlvenable · 2026-03-12T15:21:50Z

    @BeforeEach
    void setUp() throws JsonProcessingException {
        when(awsSecretManagerConfiguration.createGetSecretValueRequest()).thenReturn(getSecretValueRequest);
+        when(awsSecretManagerConfiguration.isValidateAtBootstrap()).thenReturn(true); // Default: validate at bootstrap


You also won't need this with the updated default.

dlvenable · 2026-03-12T15:25:50Z

Also, one header violation.

Missing license header: examples/config/data-prepper-config-secret-validation.yaml

- 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 <sumit4739@gmail.com>

dlvenable

Thank you @BhattacharyaSumit !

oeyh · 2026-03-21T21:07:35Z

+    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;


secretIdToValue is ConcurrentMap, but the check and refresh sequence here is not atomic. We should consider using compute() method or maybe put the sentinel check inside refresh method's compute() call.

oeyh · 2026-03-21T23:20:00Z

        } else {
            //This store is not a key value pair store. It is just a value store.
            //If we are here, either KeyToUpdate passed is null or we simply ignore it and just put value in the store
            secretIdToValue.put(secretId, newValue);


We may need to call loadSecretIfNeeded at the beginning of this updateValue method as well, otherwise there's chance that this else block will be executed if secret is not loaded yet (because NOT_LOADED_SENTINEL is not a Map), which will lead to overwriting the sentinel with newValue.

…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 <sumit4739@gmail.com>

oeyh

Thanks @BhattacharyaSumit !

BhattacharyaSumit requested review from KarstenSchnitter, dinujoh, dlvenable, engechas, graytaylor0, kkondaka, oeyh, san81, sb2k16 and srikanthjg as code owners March 11, 2026 17:03

BhattacharyaSumit force-pushed the feature/configurable-credential-validation branch from ee9d46a to 227c332 Compare March 11, 2026 23:40

dlvenable requested changes Mar 12, 2026

View reviewed changes

BhattacharyaSumit force-pushed the feature/configurable-credential-validation branch from b4c9b05 to 803e4fd Compare March 12, 2026 16:40

BhattacharyaSumit force-pushed the feature/configurable-credential-validation branch from 803e4fd to 7360f59 Compare March 12, 2026 16:47

BhattacharyaSumit requested a review from dlvenable March 20, 2026 11:00

dlvenable previously approved these changes Mar 20, 2026

View reviewed changes

oeyh reviewed Mar 21, 2026

View reviewed changes

BhattacharyaSumit dismissed dlvenable’s stale review via ae87113 March 23, 2026 15:05

oeyh approved these changes Mar 23, 2026

View reviewed changes

dlvenable approved these changes Mar 23, 2026

View reviewed changes

dlvenable merged commit e2bb830 into opensearch-project:main Mar 23, 2026
71 of 76 checks passed

Conversation

BhattacharyaSumit commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation

Changes

Configuration Example

Issues Resolved

Check List

Uh oh!

mzurita-amz commented Mar 11, 2026

Uh oh!

dlvenable left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlvenable commented Mar 12, 2026

Uh oh!

dlvenable left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oeyh Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oeyh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BhattacharyaSumit commented Mar 11, 2026 •

edited

Loading

oeyh Mar 21, 2026 •

edited

Loading