Skip to content

Common server builder and auth module for push based plugins#5423

Merged
dlvenable merged 30 commits intoopensearch-project:common-server-builder-and-auth-modulefrom
Galactus22625:common-server-builder-and-auth-module
Feb 26, 2025
Merged

Common server builder and auth module for push based plugins#5423
dlvenable merged 30 commits intoopensearch-project:common-server-builder-and-auth-modulefrom
Galactus22625:common-server-builder-and-auth-module

Conversation

@Galactus22625
Copy link
Copy Markdown
Member

@Galactus22625 Galactus22625 commented Feb 7, 2025

Description

Creating a common module for push based plugins to create server

Testing

Tested on local machine:
Original Unit tests from http source and otel logs, trace, and metrics sources pass
Tested http source pipeline works
Tested otel logs pipeline works
tested otel trace pipeline works
tested otel metrics pipeline works

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • 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.

Galactus22625 and others added 18 commits January 16, 2025 14:58
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
…rver builder into common module checkpoint)

Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Galactus22625 and others added 3 commits February 24, 2025 15:20
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
…o more build error lets go hooray ya

Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
@dlvenable dlvenable changed the base branch from main to common-server-builder-and-auth-module February 26, 2025 18:21
@dlvenable
Copy link
Copy Markdown
Member

I updated the target branch to common-server-builder-and-auth-module. We could merge it there and take a look. Thoughts @dinujoh , @Galactus22625 ?

Copy link
Copy Markdown
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Galactus22625 , These are good changes overall. This will be a good improvement to the project. I left a few comments.

*/

package org.opensearch.dataprepper.plugins.source.loghttp;
package org.opensearch.dataprepper.plugins.server;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this class moved entirely as-is? This code is very specific to the http source. It is not connected directly to how to create a server or authentication.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I should pass this in as an argument instead probably

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed


public class ConvertConfiguration {

public static ServerConfiguration convertConfiguration(final OTelLogsSourceConfig oTelLogsSourceConfig) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ServerConfiguration were an interface instead of a class with concrete methods, we could either have an adapter or possibly even inherit from that class.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea. simple for otel/grpc sources. for http I think the config extends a different interface, but can probably migrate it over.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or as an abstract class maybe

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i changed serverconfiguration over to an abstract class for grpc and used the httpconfiguration for http

@Galactus22625
Copy link
Copy Markdown
Member Author

I updated the target branch to common-server-builder-and-auth-module. We could merge it there and take a look. Thoughts @dinujoh , @Galactus22625 ?

I think this is a good idea

Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
@Galactus22625
Copy link
Copy Markdown
Member Author

Test were passing at this commit 9293e4f, but have started failing after trying to migrate serverConfiguration to abstract class. May need to revert changes or just fix

Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
@dlvenable dlvenable merged commit 68b7be2 into opensearch-project:common-server-builder-and-auth-module Feb 26, 2025
43 of 46 checks passed
@Galactus22625 Galactus22625 deleted the common-server-builder-and-auth-module branch February 27, 2025 03:03
Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 24, 2025
…rch-project#5423)

Common server builder and auth module for push based plugins

Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Siqi Ding <dingdd@amazon.com>
Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 25, 2025
…rch-project#5423)

Common server builder and auth module for push based plugins

Signed-off-by: Maxwell Brown <55033421+Galactus22625@users.noreply.github.com>
Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 25, 2025
…rch-project#5423)

Common server builder and auth module for push based plugins

Signed-off-by: Maxwell Brown <55033421+Galactus22625@users.noreply.github.com>
Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 25, 2025
…rch-project#5423)

Common server builder and auth module for push based plugins

Signed-off-by: Maxwell Brown <55033421+Galactus22625@users.noreply.github.com>
Davidding4718 pushed a commit to Davidding4718/data-prepper that referenced this pull request Apr 25, 2025
…rch-project#5423)

Common server builder and auth module for push based plugins

Signed-off-by: Maxwell Brown <55033421+Galactus22625@users.noreply.github.com>
dlvenable pushed a commit that referenced this pull request Apr 28, 2025
Common server builder and auth module for push based plugins (#5423)

Add Custom Auth Provider with support for gRPC, plus tests and exception handling

Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
Signed-off-by: Siqi Ding <109874435+Davidding4718@users.noreply.github.com>
Co-authored-by: Maxwell Brown <55033421+Galactus22625@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants