david-streamlio opened a new issue, #78:
URL: https://github.com/apache/pulsar-connectors/issues/78
## Description
The DynamoDB source's `awsEndpoint` / `dynamoEndpoint` /
`cloudwatchEndpoint` settings are unusable. Any configuration that sets an
endpoint fails at client construction with:
```
java.lang.IllegalStateException: Only one of Region or EndpointConfiguration
may be set.
```
## Root cause
All three client builders in `DynamoDBSourceConfig` set **both** an endpoint
configuration and a region:
```java
if (!this.getAwsEndpoint().isEmpty()) {
builder.setEndpointConfiguration(
new
AwsClientBuilder.EndpointConfiguration(this.getAwsEndpoint(),
this.getAwsRegion()));
}
if (!this.getAwsRegion().isEmpty()) { // <-- not an else
builder.setRegion(this.getAwsRegion());
}
```
AWS SDK v1's `AwsClientBuilder` rejects that combination — the
`EndpointConfiguration` already carries the signing region.
And `awsRegion` is **mandatory**: `DynamoDBSource.open()` enforces it.
```java
// Even if the endpoint is set, it seems to require a region to go with it
checkArgument(isNotBlank(dynamodbSourceConfig.getAwsRegion()), "The
aws-region must be set");
```
So whenever an endpoint is supplied, both branches run and `build()` throws.
The endpoint path can never have worked. It went unnoticed because production
usage against real AWS leaves the endpoint empty, and the module had no
integration test (#50).
The comment quoted above suggests a previous encounter with this, resolved
by making the region mandatory rather than by making the two mutually exclusive.
## Second, related defect
`buildDynamoDBClient` and `buildCloudwatchClient` **gate on `awsEndpoint`**
but construct the `EndpointConfiguration` from `dynamoEndpoint` /
`cloudwatchEndpoint`:
```java
if (!this.getAwsEndpoint().isEmpty()) { // gate:
awsEndpoint
builder.setEndpointConfiguration(
new
AwsClientBuilder.EndpointConfiguration(this.getDynamoEndpoint(), ...)); //
value: dynamoEndpoint
}
```
Setting `awsEndpoint` alone therefore builds an endpoint configuration from
an **empty string**. Each builder should gate on the endpoint field it actually
uses.
## Fix
Make region and endpoint mutually exclusive (`else if`), and gate each
builder on its own endpoint field. Proposed in #77, which also adds the
LocalStack integration test that exposes this — the test cannot run against
LocalStack without the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
