Skip to content
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

Add ability to enable mongo auth and add users #162

Closed
wants to merge 2 commits into from

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Sep 5, 2017

Adds auth to mongo when the mongodb_auth_enable flag is true.
Any additional users should be passed in via mongodb_users.
There are several other default variables as well (like mongodb_host,
mongodb_port which are used when logging in) that can be overridden
in the play or inventory that uses this role.
That is this PR completes this TODO from #75 for mongo security:

  • Configure/Set username/password

This does not attempt to generate any passwords. It only adds users if
explicitly requested, but allows external tasks/roles to import the
mongodb_auth.yml tasks to add users as required after mongo is installed.
That is, this PR skips this TODO from #75:

If not explicitly set, passwords should be generated randomly and placed in st2.conf

This uses lineinfile to edit mongod.conf with regexs to catch as many
edge cases in yaml formatting (spaces, quotes) as possible.
Also, this uses a bit of python to validate that the yaml file was
modified in such a way that it is still valid yaml, and the entries
intrduced in the file are present as expected.

This uses the mongo shell to see if authorization is required before
adding the admin user. This should handle cases such as the localhost
exception in a new install or upgrades where auth is not enabled or
upgrades where auth is enabled.

This installs pip using system packages if pip is not present. pip
is needed to install pymongo which is used to manage mongo users through
the mongodb_user ansible module.

As explained in comments, we only update mongo user passwords on_create
because that is the only way to maintain idempotency.

This should be idempotent.

TODO:

  • Run tests with and without mongodb auth enabled.

To configure host (in a separate PR) we need to modify net.bindIp in mongod.conf (see https://docs.mongodb.com/manual/reference/configuration-options/#net-options). I believe mongo only listens on 127.0.0.1 by default, but we might add some checks to be more explicit.
That is, this PR skips these TODOs from #75:

Ensure we can configure host
Services should run on 127.0.0.1 by default

@arm4b arm4b added the feature label Sep 5, 2017
@arm4b arm4b self-assigned this Sep 5, 2017
@cognifloyd cognifloyd mentioned this pull request Sep 6, 2017
4 tasks
@cognifloyd cognifloyd force-pushed the mongo_auth branch 2 times, most recently from 3cd7339 to 7477ed9 Compare September 6, 2017 17:01
@cognifloyd cognifloyd changed the base branch from upgrade-node-and-mongo to master September 6, 2017 17:02
@cognifloyd cognifloyd force-pushed the mongo_auth branch 2 times, most recently from dcd362e to a907ee0 Compare September 6, 2017 22:01
@arm4b arm4b added the WIP label Sep 7, 2017
@cognifloyd
Copy link
Member Author

cognifloyd commented Nov 3, 2017

obsolete comment

@cognifloyd
Copy link
Member Author

cognifloyd commented Nov 9, 2017

obsolete comment

@cognifloyd
Copy link
Member Author

I added a todo list based on #75

@cognifloyd
Copy link
Member Author

cognifloyd commented Nov 9, 2017

obsolete comment

1 similar comment
@cognifloyd
Copy link
Member Author

cognifloyd commented Nov 9, 2017

obsolete comment

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall direction and at a high-level looks good 👍

Left some comments where we need to think about the corner cases.

roles/mongodb/tasks/mongodb_auth.yml Outdated Show resolved Hide resolved
roles/mongodb/tasks/mongodb_auth.yml Outdated Show resolved Hide resolved
roles/mongodb/tasks/mongodb_auth.yml Outdated Show resolved Hide resolved
roles/mongodb/defaults/main.yml Outdated Show resolved Hide resolved
roles/mongodb/tasks/mongodb_auth.yml Outdated Show resolved Hide resolved
roles/mongodb/tasks/mongodb_auth.yml Outdated Show resolved Hide resolved
@arm4b
Copy link
Member

arm4b commented Nov 9, 2017

Also let's not rush with the random passwords for st2mongo user. Not sure yet where we should generate it: is it user's responsibility and we show how in stackstorm.yml play itself or it's a role responsibility (and it's not clear which role exactly).

Let's clarify this PR and hopefully we can add more password generation functionality as another PR.

@cognifloyd
Copy link
Member Author

cognifloyd commented Nov 10, 2017 via email

@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 13, 2018

obsolete comment

@cognifloyd cognifloyd force-pushed the mongo_auth branch 2 times, most recently from ba77aac to 3a15717 Compare September 13, 2018 16:51
@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 13, 2018

obsolete comment

@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 15, 2018

obsolete comment

1 similar comment
@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 15, 2018

obsolete comment

@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 15, 2018

obsolete comment

@cognifloyd cognifloyd force-pushed the mongo_auth branch 10 times, most recently from 798c6b6 to 7705014 Compare September 20, 2018 20:00
@cognifloyd
Copy link
Member Author

DONE! No longer WIP. Travis is now testing two suites: one without auth enabled, and one with auth enabled. I think this is ready for another review, and hopefully, a merge @armab.

@cognifloyd
Copy link
Member Author

Rebased and squashed various commits to make it easier to review each piece/commit of this PR.

@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 21, 2018

obsolete comment

@cognifloyd cognifloyd force-pushed the mongo_auth branch 2 times, most recently from 449810c to e817bb0 Compare September 22, 2018 14:42
@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 22, 2018

Dropped changes to st2 role. Now adding users is ONLY activated if mongodb_enable_auth is true (defaults to false which matches current behavior).

@cognifloyd cognifloyd force-pushed the mongo_auth branch 3 times, most recently from 686e714 to 8b56c9a Compare September 22, 2018 20:06
Adds auth to mongo when the mongodb_auth_enable flag is true.
Any additional users should be passed in via mongodb_users.
There are several other default variables as well (like mongodb_host,
mongodb_port) that can be overridden in the play or inventory that uses
this role.

This does not attempt to generate any passwords. It only adds users if
explicitly requested, but allows external tasks/roles to import the
mongodb_auth.yml tasks to add users as required after mongo is installed.

This uses lineinfile to edit mongod.conf with regexs to catch as many
edge cases in yaml formatting (spaces, quotes) as possible.
Also, this uses a bit of python to validate that the yaml file was
modified in such a way that it is still valid yaml, and the entries
intrduced in the file are present as expected.

This uses the mongo shell to see if authorization is required before
adding the admin user. This should handle cases such as the localhost
exception in a new install or upgrades where auth is not enabled or
upgrades where auth is enabled.

This installs pip using system packages if `pip` is not present. `pip`
is needed to install pymongo which is used to manage mongo users through
the mongodb_user ansible module.

As explained in comments, we only update mongo user passwords on_create
because that is the only way to maintain idempotency.

This should be idempotent.

Part of StackStorm#75.
@cognifloyd
Copy link
Member Author

cognifloyd commented Sep 22, 2018

When you review, I recommend this order (which puts files with the simplest changes first):


  • The only file in the second commit (add secure test suite):
    • .kitchen.yml suite.provisioner.extra_vars gets merged with global level provisioner.extra_vars

@cognifloyd
Copy link
Member Author

cognifloyd commented Oct 1, 2018

I will try to break this down into ity-bity PRs. Starting with a way to install pip #204.
I'll close this for now since there's no resources to review it.

Next steps:

@cognifloyd cognifloyd closed this Oct 1, 2018
@arm4b arm4b mentioned this pull request Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants