- Notifications
You must be signed in to change notification settings - Fork 23
[draft] Community TLS and cert manager #564
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?
[draft] Community TLS and cert manager #564
Conversation
…grpc # Conflicts: # controllers/searchcontroller/mongodbsearch_reconcile_helper.go
…r-community-search-snippets
…r-community-search-snippets
| @@ -0,0 +1,162 @@ | |||
| #!/usr/bin/env bash | |||
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.
Remove this file, it was broken down into 3 steps
| | kubectl apply --context "${K8S_CTX}" --namespace "${MDB_NS}" -f - | ||
| } | ||
| | ||
| create_secret "mdb-admin-user-password" "${MDB_ADMIN_USER_PASSWORD}" |
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.
while this is nice, let's remember that all the snippets should be optimized for copy&paste execution, not an automated one.
So we should aim for the simplest snippets possible and I think defining functions might be a bit too much. But I'm open to discussing it!
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 agree, thanks for the comment.
| MDB_SEARCH_TLS_SECRET_NAME | ||
| CERT_MANAGER_NAMESPACE | ||
| ) | ||
| missing=() |
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.
if we're going with additional scripts or functions, then we could define them in env_variables.sh or another script that is sourced in env_variables.sh.
The assumption is that sourcing env_variables.sh is a prerequisite step. But also let's remember that in the doc page we tell user "source the following env vars "
we don't sa anything about "source also this script containing helper functions".
| @@ -0,0 +1,28 @@ | |||
| required=( | |||
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.
having this in each snippet will not look great on the rendered docs page
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 have replaced these with a step that checks everything at once.
| TLS_FLAG="--tls" | ||
| CA_FLAG="--tlsCAFile" | ||
| else | ||
| TLS_FLAG="--ssl" |
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.
why do we need that?
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 we standardize on --tls? I believe --ssl is deprecated
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.
also, we should probably check if the connection MDB_CONNECTION_STRING contains tls=true and only in that case add tls and ca params.
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.
also consider injecting those path directly into conn str:
with tlsCAFile=/path/to/ca.pem
This way we could keep search-query-usage guide TLS-agnostic?
| #export MDB_SEARCH_TLS_SECRET_NAME="${MDB_RESOURCE_NAME}-search-tls" | ||
| | ||
| # default connection string if MongoDB database is deployed using the operator | ||
| #export MDB_CONNECTION_STRING="mongodb://mdb-user:${MDB_USER_PASSWORD}@${MDB_RESOURCE_NAME}-0.${MDB_RESOURCE_NAME}-svc.${MDB_NS}.svc.cluster.local:27017/?replicaSet=${MDB_RESOURCE_NAME}" |
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.
if we're going with tls params in connection string (I'd recommend that), then let's put an example here for tls as well
The assumption is that whatever connection string is defined in previous snippet modules (community, enterprise, external) should work in this module keeping it generic
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.
how the cert manager installation here is different from the snippet module that we have for Reference Architectures?
https://github.com/mongodb/mongodb-kubernetes/tree/master/public/architectures/setup-multi-cluster/ra-05-setup-cert-manager/code_snippets
Apart from the difference in the K8S_CLUSTER_0_CONTEXT_NAME, we could probably just refer users to the steps in that module or just copy the module next to search for now. Unless we're doing something completely different, I'd prefer to not create a completely different cert-manager scripts that what we already have.
It's even better to copy the ref arch's snippets and keep the same snippet structure than write them again.
WE have the cert-manager snippet module documented here: https://www.mongodb.com/docs/kubernetes/current/reference-architectures/multi-cluster/ca-certs/
If not for the difference in the env var we could even import it in here similarly how it's done in the GKE's snippets
| --from-literal=password="${MDB_USER_PASSWORD}" \ | ||
| --dry-run=client -o yaml | kubectl apply --context "${K8S_CTX}" --namespace "${MDB_NS}" -f - | ||
| | ||
| echo "User secrets created." |
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.
Added these logs so that it is easier for LLMs to debug issues
MCK 1.6.0 Release NotesNew Features
Bug Fixes
Other Changes
|
Summary
This pull request introduces comprehensive changes to the MongoDB Search community deployment scripts and documentation, with a primary focus on enabling and validating TLS (Transport Layer Security) throughout the deployment process. The updates ensure secure communication between components, improve environment validation, and streamline certificate management and resource creation. Additionally, the test suite and code snippets are updated to reflect these changes and enhance reliability.
TLS enablement and certificate management:
cert-manager, bootstrap self-signed CA issuers, issue TLS certificates for MongoDB and Search, and expose CA bundles via ConfigMaps, ensuring all resources use TLS for secure communication. [1] [2] [3]MongoDBCommunityandMongoDBSearchresources. [1] [2] [3] [4] [5]Environment validation and deployment reliability:
kubectl applywith--dry-run, making secret creation idempotent and safer.Test suite and deployment flow updates:
Code snippet and usage improvements:
Testing and configuration:
GetMongodConfigParametersto verify correct transport and port selection based on wire protocol annotations.Proof of Work
Checklist
skip-changeloglabel if not needed