-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 fetch authorized collections only #9238
Source MongoDB fetch authorized collections only #9238
Conversation
vmaltsev seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
/test connector=connectors/source-mongodb-v2
|
Document document = database.getDatabase().runCommand(new Document("listCollections", 1) | ||
.append("authorizedCollections", true).append("nameOnly", true)); | ||
|
||
return document.toBsonDocument() | ||
.get("cursor").asDocument() | ||
.getArray("firstBatch") | ||
.stream() | ||
.filter(bsonValue -> bsonValue.asDocument().getString("type").getValue().equals("collection")) | ||
.map(bsonValue -> bsonValue.asDocument().getString("name").getValue()) | ||
.collect(Collectors.toSet()); |
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.
Looks great)
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.
Can you format the code before merging? I think the indentation of the getAuthorizedCollections
method is off.
.get("cursor").asDocument() | ||
.getArray("firstBatch") | ||
.stream() | ||
.filter(bsonValue -> bsonValue.asDocument().getString("type").getValue().equals("collection")) |
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.
Nitpick .The listCollections
command can take in a filter
parameter to filter on any field in the returned document directly.
Reference: https://docs.mongodb.com/manual/reference/command/listCollections/#definition
I think if you add the filter parameter, you won't need to do the filter here:
Document document = database.getDatabase().runCommand(new Document("listCollections", 1)
.append("authorizedCollections", true)
.append("nameOnly", true))
.append("filter", "{ 'type': 'collection' }")
I am not 100% sure this is the correct syntax though.
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.
Nitpick .The
listCollections
command can take in afilter
parameter to filter on any field in the returned document directly.Reference: https://docs.mongodb.com/manual/reference/command/listCollections/#definition
I think if you add the filter parameter, you won't need to do the filter here:
Document document = database.getDatabase().runCommand(new Document("listCollections", 1) .append("authorizedCollections", true) .append("nameOnly", true)) .append("filter", "{ 'type': 'collection' }")I am not 100% sure this is the correct syntax though.
added filter into mongo shell command
formatted |
…ed-collections # Conflicts: # airbyte-integrations/connectors/destination-snowflake/src/main/java/io/airbyte/integrations/destination/snowflake/SnowflakeInternalStagingConsumerFactory.java
/publish connector=connectors/source-mongodb-v2 |
/publish connector=connectors/source-mongodb-v2
|
What
Airbyte UI should return only those collections for which the user has privileges.
How
Used authorizedCollections:true flag in listCollections Mongo command
Recommended reading order
x.java
y.python
🚨 User Impact 🚨
There should not visible user impact. UI now will show only authorized collections
Pre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
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 hereUpdating a connector
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 hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changes