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 support for net.ssl.allowConnectionsWithoutCertificates setting in mongod.conf #577

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

yennchen
Copy link

Pull Request (PR) description

Add support for net.ssl.allowConnectionsWithoutCertificates setting in mongod.conf

This Pull Request (PR) fixes the following issues

@raphink
Copy link
Member

raphink commented Mar 18, 2020

Could you provide tests with this PR please?

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

This is a large PR that does multiple things. Is it really needed to change SSL to TLS everywhere? It makes this much harder to review. It's also a breaking change which we generally avoid.

The reason I ask for a small PR is that I don't have time to review such large PRs due to other work. Chances are that it will linger for a long time. Small PRs on the other hand a easier to review and increase the chance of merging. https://theforeman.org/2019/03/merge-time-review-complexity.html is for a different project, the the same things still apply.

@@ -18,7 +18,7 @@ These should not affect the functionality of the module.
- Wrong APT-key [\#546](https://github.com/voxpupuli/puppet-mongodb/issues/546)
- Mongo 4.0.x: unable to create user [\#525](https://github.com/voxpupuli/puppet-mongodb/issues/525)
- user creation idempotency issues [\#412](https://github.com/voxpupuli/puppet-mongodb/issues/412)
- fix\(is\_master-fact\): use --ssl if --sslPEMKeyFile or --sslCAFile is s… [\#573](https://github.com/voxpupuli/puppet-mongodb/pull/573) ([buchstabensalat](https://github.com/buchstabensalat))
- fix\(is\_master-fact\): use --tls if --tlsCertificateKeyFile or --tlsCAFile is s… [\#573](https://github.com/voxpupuli/puppet-mongodb/pull/573) ([buchstabensalat](https://github.com/buchstabensalat))
Copy link
Member

Choose a reason for hiding this comment

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

This is odd. Normally we don't modify changelog entries

@@ -19,7 +19,7 @@
Variant[Boolean, String] $package_ensure = $mongodb::params::package_ensure,
String $package_name = $mongodb::params::server_package_name,
Variant[Boolean, Stdlib::Absolutepath] $logpath = $mongodb::params::logpath,
Array[Stdlib::Compat::Ip_address] $bind_ip = $mongodb::params::bind_ip,
Array[Stdlib::Host] $bind_ip = $mongodb::params::bind_ip,
Copy link
Member

Choose a reason for hiding this comment

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

Is it allowed to bind on a FQDN?

@@ -0,0 +1,168 @@
#mongodb.conf - generated from Puppet
Copy link
Member

Choose a reason for hiding this comment

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

since this is a new template, can you please convert it to epp? We require that for all new templates.

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet labels May 21, 2020
@vox-pupuli-tasks
Copy link

Dear @yennchen, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request merge-conflicts needs-tests needs-work not ready to merge just yet tests-fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants