-
Notifications
You must be signed in to change notification settings - Fork 521
(DOCSP-11424): Document SCRAM Auth #196
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
(DOCSP-11424): Document SCRAM Auth #196
Conversation
README.md
Outdated
| ## Documentation | ||
|
|
||
| ### Prerequisites | ||
| See the [`/docs`](/docs) directory to view documentation on how to: |
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.
| See the [`/docs`](/docs) directory to view documentation on how to: | |
| See the [`documentation`](/docs) to learn how to: |
concision
README.md
Outdated
| - Use of any of the available [Docker MongoDB images](https://hub.docker.com/_/mongo/) | ||
| - Clients inside the Kubernetes cluster can connect to the replica set (no external connectivity) | ||
| - TLS support for client-to-server and server-to-server communication | ||
| - Creating users with SCRAM-SHA authentication |
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.
your text looks good, but I think we should make these list items parallel. Doesn't have to be in this PR.
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.
Ah good point, I just copy/pasted from below. Updated.
docs/users.md
Outdated
| ``` | ||
| kubectl apply -f <mongodb-crd>.yaml --namespace <my-namespace> | ||
| ``` | ||
| 1. Once the MongoDB resource is running, securely store the user's password and then delete the user secret: |
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.
| 1. Once the MongoDB resource is running, securely store the user's password and then delete the user secret: | |
| 1. After the MongoDB resource is running, securely store the user's password and then delete the user secret: |
per style guide, use after here.
| @@ -0,0 +1,85 @@ | |||
| # Deploy and Configure a MongoDB Resource # | |||
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.
Consider adding a readme.md in /docs with a TOC to make it easier to find information instead of clicking the individual files.
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.
DIdn't realize I could have multiple readmes, good idea. Added.
docs/secure.md
Outdated
| ## Table of Contents | ||
|
|
||
| - [Secure MongoDB Resource Connections using TLS](#secure-mongodb-resource-connections-using-tls) | ||
| - [Prerequisites](#prerequisites-1) |
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 think the prerequisites and procedure links can change now that there aren't multiple headings with those titles on this page.
docs/secure.md
Outdated
| ## Table of Contents | ||
|
|
||
| - [Secure MongoDB Resource Connections using TLS](#secure-mongodb-resource-connections-using-tls) | ||
| - [Prerequisites](#prerequisites-1) |
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.
| - [Prerequisites](#prerequisites-1) | |
| - [Prerequisites](#prerequisites) |
docs/secure.md
Outdated
|
|
||
| - [Secure MongoDB Resource Connections using TLS](#secure-mongodb-resource-connections-using-tls) | ||
| - [Prerequisites](#prerequisites-1) | ||
| - [Procedure](#procedure-1) |
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.
| - [Procedure](#procedure-1) | |
| - [Procedure](#procedure) |
docs/users.md
Outdated
| password: <my-plain-text-password> # corresponds to spec.users.passwordSecretRef.key in the MongoDB CRD | ||
| ... | ||
| ``` | ||
| 1. Update `metadata.name` with the name of the secret and `stringData.password` with the user's 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.
Offer clearer guidance here that you can provide anything you want for <db-user-secret>. I think users might ask what the name of the secret is without realizing they can provide any value here.
docs/users.md
Outdated
| @@ -0,0 +1,86 @@ | |||
| # Create a Database User # | |||
|
|
|||
| You can create a MongoDB database user to authenticate to your MongoDB resource using SCRAM. First, [create a Kubernetes secret](#create-a-user-secret) for the new user's password. Then, [modify and apply the MongoDB CRD](#modify-the-mongodb-crd). | |||
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.
consider linking to the Manual page on SCRAM here.
| # Create a Database User # | ||
|
|
||
| You can create a MongoDB database user to authenticate to your MongoDB resource using SCRAM. First, [create a Kubernetes secret](#create-a-user-secret) for the new user's password. Then, [modify and apply the MongoDB CRD](#modify-the-mongodb-crd). | ||
|
|
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.
Question about behavior here: When you create one SCRAM user, you will no longer be able to perform any actions unauthenticated, correct? I think we can be a little clearer about this here.
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.
There are a handful of commands you can run without being authenticated, like the native commands (like listFiles() ) and a few database commands. I tried db.runCommand({connectionStatus : 1}) and isMaster, which both worked while unauthenticated. But I can't find a definitive list of unauthenticated commands, and I don't think the ones you can use actually do very much. Let me know if you want me to elaborate more in the topic. And maybe @chatton has an opinion :)
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 think it should be clear enough to link to the SCRAM docs like we do here, as everything written there will apply, I don't think we need to clarify any more about this, I don't feel too strongly about this though.
86e769e to
52efa06
Compare
jwilliams-mongo
left a comment
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.
LGTM % one nit
docs/README.md
Outdated
| @@ -0,0 +1,10 @@ | |||
| # MongoDB Community Kubernetes Operator Documentation # | |||
|
|
|||
| # Table of Contents | |||
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.
| # Table of Contents | |
| ## Table of Contents |
nit: this should be an H2.
chatton
left a comment
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.
Great work @melissamahoney-mongodb , I love the structure you added to the docs here.
I had just had one or two small comments!
docs/users.md
Outdated
| ``` | ||
| kubectl apply -f <mongodb-crd>.yaml --namespace <my-namespace> | ||
| ``` | ||
| 1. After the MongoDB resource is running, securely store the user's password and then delete the user secret: |
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.
as deleting the secret is not a required step, maybe we could say something more along the lines of "we recommend to delete the secret as it is no longer required by the operator",
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.
Updated. I moved this down under "Next Steps".
docs/users.md
Outdated
| ``` | ||
| mongo "mongodb://<mongodb-resource-metadata.name>-svc.<my-namespace>.svc.cluster.local:27017/?replicaSet=<replica-set-name>" --username <username> --password <password> --authenticationDatabase <authentication-database> | ||
| ``` | ||
| - To change a user's password, create and apply a new secret with a `metadata.name` that is the same as the name specified in `passwordSecretRef.name` of the MongoDB CRD. The Operator automatically re-generates the SCRAM credentials. |
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.
[nit] create a secret already indicates that the kubernetes secret resource is being created, but here you are referring to creating a new resource definition that contains the yaml for the new password.
I think we should phrase this as create and apply a new secret resource definition or just create a new secret as described above
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.
[tiny-nit] The line The Operator automatically re-generates the SCRAM credentials. sounds a little off to me, I think we can just say, The Operator will automatically generate new credentials
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.
Gotcha. Updated.
| # Create a Database User # | ||
|
|
||
| You can create a MongoDB database user to authenticate to your MongoDB resource using SCRAM. First, [create a Kubernetes secret](#create-a-user-secret) for the new user's password. Then, [modify and apply the MongoDB CRD](#modify-the-mongodb-crd). | ||
|
|
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 think it should be clear enough to link to the SCRAM docs like we do here, as everything written there will apply, I don't think we need to clarify any more about this, I don't feel too strongly about this though.
|
Thanks for the review @chatton ! Ready for another look. |
docs/users.md
Outdated
| kubectl apply -f <db-user-secret>.yaml --namespace <my-namespace> | ||
| ``` | ||
| ## Modify the MongoDB CRD |
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.
@melissamahoney-mongodb Ah! Sorry I think this may have been my fault, in this case CRD refers to a kubernetes CRD which is what we provide, the user will modify a MongoDB resource definition, but not the CRD itself. I think maybe most straight forward way to phrase this is Modify the MongoDB Resource
I think MongoDB resource is a good term to refer to individual instances of the Custom Resource which are described via the Custom Resource Definition (CRD)
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.
Ah, I think I wasn't quite clear on that distinction myself. Thanks for the explanation! Updated.
8265f92 to
10a9a57
Compare
chatton
left a comment
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.
LGTM thanks for the great work on this!
10a9a57 to
b4f5f4b
Compare
This PR:
Staging: