-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Source MongoDb: added support via TLS/SSL #6364
Conversation
fixed replica connection; fixed incremental read;
…it by default for other types
/test connector=connectors/source-mongodb-v2
|
/test connector=connectors/source-mongodb-v2
|
case TIMESTAMP -> new BsonTimestamp(Long.parseLong(value)); | ||
case DATE_TIME -> new BsonDateTime(new DateTime(value).getValue()); | ||
case OBJECT_ID -> new ObjectId(value); | ||
default -> null; |
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.
instead of default null why don't we return string?
Also I don't see a mapping from string or object, how come?
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.
I've changed to String, thanks! Also added String and Symbol.
This mapping needed only for cursor field, since it couldn't be object or array, it isn't present here.
@@ -26,7 +26,29 @@ the Dockerfile. | |||
We use `JUnit` for Java tests. | |||
|
|||
### Test Configuration | |||
No specific configuration needed for testing, MongoDb Test Container is used. | |||
|
|||
In order to test the MongoDb source, you need to have a standalone instance, a replica set or Atlas cluster. |
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.
it doesn't work with a docker image anymore?
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.
It works, but I've also added test for Atlas Cluster with ssl, I've corrected doc.
@@ -98,13 +105,6 @@ | |||
"default": "admin", | |||
"examples": ["admin"], | |||
"order": 4 | |||
}, |
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.
you will need to make additionalProperties=true for this to be backwards compatible, and potentially handle it manually in the connector code
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 a lot, I've added.
### TLS/SSL on a Connection | ||
|
||
It is recommended to use encrypted connection. | ||
Connection with TLS/SSL security protocol for MongoDb Atlas Cluster and Replica Set instances is enabled by default. Encryption is based on the underlying TLS/SSL support in the JDK. |
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.
suggest removing since it's an implementation detail
Connection with TLS/SSL security protocol for MongoDb Atlas Cluster and Replica Set instances is enabled by default. Encryption is based on the underlying TLS/SSL support in the JDK. | |
Connection with TLS/SSL security protocol for MongoDb Atlas Cluster and Replica Set instances is enabled by default. |
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.
Done
/test connector=connectors/source-mongodb-v2
|
/publish connector=connectors/source-mongodb-v2
|
What
Added support of TLS encryption by default when connecting to the MongoDB source: MongoDb Atlas Cluster and Replica Set.
For Standalone MongoDb instance added possibility to select whether to encrypt, as it may require additional configuration from customer.
Provided small fix for incremental sync.
How
For MongoDb Atlas Cluster and Replica Set added
ssl=true
to connection param.For Standalone MongoDb instance it is configurable.
Recommended reading order
MongoDbSource.java
spec.json
Pre-merge Checklist
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>
command is passing./publish
command described here