- 
                Notifications
    You must be signed in to change notification settings 
- Fork 22
CLOUDP-353160 [Search] Implement gRPC and mTLS #527
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
base: master
Are you sure you want to change the base?
Conversation
| MCK 1.6.0 Release NotesNew Features
 Bug Fixes
 Other Changes
 | 
5cc67a1    to
    3ec055e      
    Compare
  
    |  | ||
| serviceBuilder.AddPort(&corev1.ServicePort{ | ||
| Name: "mongot", | ||
| Name: "mongot-wireproto", | 
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.
could we add service port only if we force wireproto?
|  | ||
|  | ||
| @mark.e2e_search_community_tls | ||
| def test_validate_tls_connections(mdbc: MongoDBCommunity, mdbs: MongoDBSearch, namespace: str, issuer_ca_filepath: str): | 
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 don't attempt to validate connections here 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.
I wasn't able to find a simple way to call the gRPC service in mongot from Python that didn't involve a lot of boilerplate, so I opted to remove this altogether.
| DataPath: searchcontroller.MongotDataPath, | ||
| }, | ||
| Server: mongot.ConfigServer{ | ||
| Wireproto: &mongot.ConfigWireproto{ | 
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.
Should we still keep a wireproto test as long as it is possible to enabled it?
| } | ||
|  | ||
| r.watch.AddWatchedResourceIfNotAdded(searchSource.KeyfileSecretName(), mdbSearch.Namespace, watch.Secret, mdbSearch.NamespacedName()) | ||
| if mdbSearch.IsWireprotoForced() { | 
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 all these if statements, would it make sense to abstract the wireproto stuff away in a struct and then it's also easier to just remove it.
| "searchIndexManagementHostAndPort": mongotHostAndPort(search), | ||
| "skipAuthenticationToSearchIndexManagementServer": false, | ||
| "searchTLSMode": string(searchTLSMode), | ||
| "useGrpcForSearch": !search.IsWireprotoForced(), | 
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.
What is the meaning of this, don't we always enable this 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.
The server doesn't enable useGrpcForSearch by default, no. We enable it only when the force-wireproto annotation is not set, but in essenceuseGrpcForSearch will be turned on by default, unless the annotation requests otherwise.
…grpc # Conflicts: # controllers/searchcontroller/mongodbsearch_reconcile_helper.go
ef87f24    to
    fc149a0      
    Compare
  
    fc149a0    to
    cb83dbc      
    Compare
  
    
Summary
This updates the
mongod->mongotcommunication to use the gRPC protocol as the Wire Protocol server is deprecated inmongot.Proof of Work
Checklist
skip-changeloglabel if not needed