-
Notifications
You must be signed in to change notification settings - Fork 2k
[Feature][elasticsearch-connector] Add API key authentication support #9610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @CosmosNi
...-e2e/src/test/java/org/apache/seatunnel/e2e/connector/elasticsearch/ElasticsearchAuthIT.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds API key authentication support to the Elasticsearch connector in SeaTunnel, addressing issue #9609 by implementing a new authentication system alongside the existing basic authentication.
- Introduces a pluggable authentication provider system that supports both basic and API key authentication
- Adds new configuration options for API key authentication including
auth_type
,auth.api_key_id
,auth.api_key
, andauth.api_key_encoded
- Refactors the existing authentication implementation to use the new provider pattern while maintaining backward compatibility
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
ElasticsearchBaseOptions.java | Adds new configuration options for authentication type and API key parameters |
EsRestClient.java | Refactors client creation to use the new authentication provider system |
AuthenticationProvider.java | Defines the interface for authentication providers |
AbstractAuthenticationProvider.java | Provides base implementation handling TLS configuration |
BasicAuthProvider.java | Implements basic authentication using the new provider pattern |
ApiKeyAuthProvider.java | Implements API key authentication with support for both separate and encoded formats |
AuthenticationProviderFactory.java | Factory class for creating appropriate authentication providers |
ElasticsearchConnectorErrorCode.java | Adds new error codes for authentication failures |
ElasticsearchSourceFactory.java | Updates source factory to include new authentication options |
ElasticsearchSinkFactory.java | Updates sink factory to include new authentication options |
ElasticsearchIT.java | Updates test to use new configuration format |
ElasticsearchSchemaChangeIT.java | Updates test to use new configuration format |
ElasticsearchAuthIT.java | Adds comprehensive test suite for authentication functionality |
Elasticsearch.md (source) | Updates documentation with new authentication options and examples |
Elasticsearch.md (sink) | Updates documentation with new authentication options and examples |
Comments suppressed due to low confidence (4)
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-elasticsearch-e2e/src/test/java/org/apache/seatunnel/e2e/connector/elasticsearch/ElasticsearchSchemaChangeIT.java:56
- The removed import 'java.util.Optional' is still being used in the code. This suggests the import should not have been removed, or there are still references to Optional that need to be updated.
import java.util.concurrent.CompletableFuture;
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-elasticsearch-e2e/src/test/java/org/apache/seatunnel/e2e/connector/elasticsearch/ElasticsearchIT.java:79
- The removed import 'java.util.Optional' is still being used in the code. This suggests the import should not have been removed, or there are still references to Optional that need to be updated.
import java.util.Set;
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-elasticsearch-e2e/src/test/java/org/apache/seatunnel/e2e/connector/elasticsearch/ElasticsearchAuthIT.java:84
- [nitpick] Consider using more descriptive names like 'testApiKeyId' or 'createdApiKeyId' to clarify that these are test-specific API keys created dynamically.
private String validApiKeyId;
...-e2e/src/test/java/org/apache/seatunnel/e2e/connector/elasticsearch/ElasticsearchAuthIT.java
Show resolved
Hide resolved
.../org/apache/seatunnel/connectors/seatunnel/elasticsearch/client/auth/ApiKeyAuthProvider.java
Show resolved
Hide resolved
// Authentication configuration options | ||
public static final Option<String> AUTH_TYPE = | ||
Options.key("auth_type") | ||
.stringType() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use enum type for auth_type
.
.stringType() | |
.enumType() |
AUTH_TYPE, | ||
API_KEY_ID, | ||
API_KEY, | ||
API_KEY_ENCODED, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AUTH_TYPE, | |
API_KEY_ID, | |
API_KEY, | |
API_KEY_ENCODED, | |
.conditional(AUTH_TYPE, "api_key", | |
API_KEY_ID, | |
API_KEY, | |
API_KEY_ENCODED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if ci passes. Thanks @CosmosNi
Add API key authentication support
close #9609
Purpose of this pull request
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide