Skip to content

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