[I] [Bug] DynamoDB source cannot use a custom endpoint: region and endpoint are both set, which AWS SDK v1 forbids [pulsar-connectors]

["david-streamlio (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@pulsar.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] DynamoDB source cannot use a custom endpoint: region and endpoint are both set, which AWS SDK v1 forbids [pulsar-connectors]

["david-streamlio (via GitHub)" <gi...@apache.org>]

david-streamlio closed issue #78: [Bug] DynamoDB source cannot use a custom endpoint: region and endpoint are both set, which AWS SDK v1 forbids
URL: https://github.com/apache/pulsar-connectors/issues/78


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org