Enable HTTP Sink with aws sigv4 auth mode only by kkondaka · Pull Request #6747 · opensearch-project/data-prepper

kkondaka · 2026-04-09T21:33:36Z

Description

Enable HTTP Sink with aws sigv4 auth mode only.
Modified HTTP sink code to use only aws auth mode only.
Removed other auth options

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.

Signed-off-by: Kondaka <krishkdk@amazon.com>

github-actions · 2026-04-09T21:34:13Z

✅ License Header Check Passed

All newly added files have proper license headers. Great work! 🎉

dlvenable

Nice improvement @kkondaka . It will be good to get this sink ready to use! I have a number of suggestions. Overall we should be sure this is a generic sink and not so tightly coupled with AWS/SigV4.

One alternative is to create an aws_http sink. But I think some code clean up will make that unnecessary.

dlvenable · 2026-04-10T14:59:04Z

    private PluginModel codec;

-    @JsonProperty("http_method")
-    private HTTPMethodOptions httpMethod = HTTPMethodOptions.POST;


This seems worth keeping.

I thought about it but since we are not using now, decided to keep it out. We can always add things that are needed back when they are needed.

dlvenable · 2026-04-10T15:03:17Z

+    }
+
+    public HttpEndPointResponse send(final byte[] payload) {
+        HttpEndPointResponse response = null;


Let's also rename this class to HttpEndpointResponse since this is a single word - "endpoint."

dlvenable · 2026-04-10T15:16:25Z

-
-import com.fasterxml.jackson.annotation.JsonProperty;
-
-public class BasicAuthCredentials {


I think removing a lot of the authentication options is good. But we should at least support basic auth. Otherwise nobody can use this other than with AWS. If we start here, we can look for community contributions for expanded support.

We should also support unauthenticated requests.

Requests are unauthenticated when "aws:" is not used. There is already integration tests that shows that path works.

dlvenable · 2026-04-10T15:17:57Z

+
+    SdkHttpFullRequest signRequest(final SdkHttpFullRequest unsignedRequest) {
+        if (credentialsProvider == null || credentialsProvider.resolveCredentials() == null) {
+            return null;


If we fail to load credentials this will give an NPE, right? We should have a better way to fail.

dlvenable · 2026-04-10T15:19:16Z

+
+    private SdkHttpFullRequest createSdkHttpRequest(final String url, @Nonnull final byte[] payload) {
+        final SdkHttpFullRequest.Builder builder = SdkHttpFullRequest.builder()
+                .method(SdkHttpMethod.POST)


This should be configurable. It is an easy config to offer.

Also, this and the URI should be provided by the HttpSinkSender and then used by the suggested decorator. That is, don't mix the concerns of determining the method/URI with the concern of signing.

Agreed. But do not need it initially. I will do it in a follow-up PR

I think we can add additional configurations later. But we shouldn't leave the code in a state where two things that don't need to be connected are. Specifically having the AWS SDK code in HttpSinkSender. Let's just decouple that and we should be good to go.

dlvenable · 2026-04-10T15:20:32Z

+
+public class HttpSinkSender {
+    private static final Logger LOG = LoggerFactory.getLogger(HttpSinkSender.class);
+    private static final String HTTP_METHOD = "post";


I think this is unused.

dlvenable · 2026-04-10T15:50:32Z

+        }
+
+
+        if (config.getCustomHeaderOptions() != null) {


I think the Codecs should ideally offer a MIME type interface.

Optional<String> getMimeType();

Then the codec can provide a default MIME type to use in the Content-Type header.

This would work well for the json codec.

However it might not work for all. I think ND-JSON doesn't have a MIME type. Maybe application/jsonl or application/x-ndjson. We should still override it with custom headers.

Let's look at this in a follow-up PR

Yea, that's good.

srikanthpadakanti · 2026-04-10T18:36:37Z

+        this.estimatedSize = calculateSize(event, codec, codecContext);
+    }
+
+    private long calculateSize(final Event event, final OutputCodec codec, final OutputCodecContext codecContext) throws IOException {


The event is serialized once in calculateSize() for size estimation, then again in flush() for actual sending. Consider caching the serialized bytes from the size calculation and reusing them during flush, or using a cheaper estimation like event.toJsonString().length()

Yes, this is a known issue. We cannot use event.toJsonString().length() because the codec is not always json.

One way to improve this is to store the OutputStream from calculateSize() and reuse it in flush(), not sure if that would work. But something to look into in future.

Signed-off-by: Kondaka <krishkdk@amazon.com>

dlvenable

Two things before merging:

Let's require the service_name because this is a generic http sink that should be able to send to any AWS SigV4 service. It isn't just for API Gateway.
Let's remove the AWS code from HttpSinkSender. This way if the community wants to extend the authentication options there is no need to extract this AWS code which isn't tested in an integration environment. This will give more confidence.

Signed-off-by: Kondaka <krishkdk@amazon.com>

dlvenable · 2026-04-14T15:28:32Z

    @JsonProperty("connection_timeout")
    private Duration connectionTimeout = DEFAULT_CONNECTION_TIMEOUT;

+    @JsonProperty("aws_sigv4_service_name")


Do we have an aws: block? If so, I think we can just make this service_name under that block. We can follow on with this change or discuss further before the PR.

There is an aws block. Would be better to move this under that

graytaylor0 · 2026-04-14T16:29:45Z

+    }
+
+    @Override
+    public boolean isReady() {


Doesn't have to be in this PR but would be good to consider implementing graceful shutdown where we flush everything in the sink before exiting.

Enable HTTP Sink with aws sigv4 auth mode only

c9d9c8f

Signed-off-by: Kondaka <krishkdk@amazon.com>

kkondaka requested review from KarstenSchnitter, dinujoh, dlvenable, engechas, graytaylor0, oeyh, san81, sb2k16 and srikanthjg as code owners April 9, 2026 21:33

dlvenable requested changes Apr 10, 2026

View reviewed changes

dlvenable mentioned this pull request Apr 10, 2026

Add Prometheus scrape/pull source to prometheus plugin [Adding in the same Prometheus Remote write source] #6743

Merged

1 task

srikanthpadakanti reviewed Apr 10, 2026

View reviewed changes

Addressed review comments

86a20c0

Signed-off-by: Kondaka <krishkdk@amazon.com>

dlvenable reviewed Apr 13, 2026

View reviewed changes

Addressed review comments

2c1814e

Signed-off-by: Kondaka <krishkdk@amazon.com>

dlvenable approved these changes Apr 14, 2026

View reviewed changes

graytaylor0 reviewed Apr 14, 2026

View reviewed changes

graytaylor0 approved these changes Apr 14, 2026

View reviewed changes

kkondaka merged commit 2ef318d into opensearch-project:main Apr 14, 2026
109 of 121 checks passed

srikanthpadakanti mentioned this pull request Apr 20, 2026

Move common HTTP auth mechanisms (Basic Auth, Bearer Token) to a shared module #6767

Closed


		import com.fasterxml.jackson.annotation.JsonProperty;

		public class BasicAuthCredentials {

Conversation

kkondaka commented Apr 9, 2026

Description

Issues Resolved

Check List

Uh oh!

github-actions Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ License Header Check Passed

Uh oh!

dlvenable left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

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 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Apr 9, 2026 •

edited

Loading