Skip to content

Conversation

@Lomonosow
Copy link
Contributor

#13 Is work for me, but use mongo admin user. I`d tried to find another way but unsuccessfully)

@arm4b arm4b requested a review from warrenvw April 12, 2019 21:20
@warrenvw
Copy link
Contributor

@Lomonosow Thank you for your contribution. I'll be reviewing it shortly.

@Lomonosow
Copy link
Contributor Author

Lomonosow commented Apr 17, 2019

I found some mistake in configuration, mongo want key at least 6 symbols length. I have fix it in my commit.

@arm4b
Copy link
Member

arm4b commented Apr 25, 2019

@warrenvw Can you please drive this review, think about corner cases and check the PR?

From my point of view it would be nice to add a condition and set mongo username/password in st2.conf only if mongodb-ha.auth.enabled = true

Copy link
Contributor

@warrenvw warrenvw left a comment

Choose a reason for hiding this comment

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

Great work @Lomonosow. I manually tested this PR against our EKS cluster, and can only see a few things I'd do differently.

@warrenvw
Copy link
Contributor

warrenvw commented May 3, 2019

I'd appreciate if you could update the Changelog and bump the version in Charts.yaml. The Changelog should probably indicate that auth is now enabled by default.

Please note that I have not been able to successfully transition mongodb from auth disabled to auth enabled. Have you tried this? I'm attempting this particular scenario because auth is likely disabled on most installations... and now by default it will be enabled.

For example, in the st2actionrunner pod, I see:

OperationFailure: Authentication failed.

@Lomonosow
Copy link
Contributor Author

Lomonosow commented May 4, 2019

For example, in the st2actionrunner pod, I see:

OperationFailure: Authentication failed.

I have reproduce it when I`d change mongodb admin password and tried to upgrade helm release. Mongodb chart does not recreate user if directory for db already exists (in mongodb persistent storage enabled by default and if you delete release by helm, persistent storage will not be deleted).

@warrenvw warrenvw merged commit 609da84 into StackStorm:master May 10, 2019
@arm4b arm4b added enhancement New feature or request security labels May 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants