Change $repos_ensure to default to false. #440
Conversation
|
@hunner any opinion on this? It sounds like the initial default value was not really a good choice, but this chance is likely now backward compatible... |
jguiditta
force-pushed
the
change_repos_ensure
branch
2 times, most recently
from
53c029d to
fc75c29
Compare
|
I want to do a 6.0.0 soon, so it can go in that. |
jguiditta
force-pushed
the
change_repos_ensure
branch
from
fc75c29 to
421c1f5
Compare
| let(:params) {{ :manage_repos => true, :repos_ensure => true }} | ||
| let(:facts) {{ :osfamily => 'RedHat', :operatingsystemmajrelease => '7' }} | ||
| it 'does import repo public key when manage_repos is true' do | ||
| it 'does import repo public key when manage_repos and repos_ensure are true' do |
There was a problem hiding this comment.
Please note this change. If you disagree with it, then it may be needed to look again at the logic in init.pp, as that checks for manage_repos != false as the condition to include repo class. Since this parameter is itself deprecated, it seems to me this logic should change, though it is not a requirement for this patch.
|
Looks like this needs #451 based on output of the one version of tests that failed - https://travis-ci.org/puppetlabs/puppetlabs-rabbitmq/jobs/116965330 |
The problem with it being true by default is that the majority of production environments, and any in an isolated nextwork, will not want to get the gpg key, especially if they have not explicitly configured the module to get the rpm from the rabbitmq repos. Having the gpg key pulled in by default while installing the rpm from distro repos seems inconsistent at best, and at worst, causes an additional fail point for deployments that are unable to contact this location. Leaving it true by default forces the majority of production users to have to override this setting.
jguiditta
force-pushed
the
change_repos_ensure
branch
from
421c1f5 to
ea7e565
Compare
|
@hunner do we plan to release 6.0.0 soon? |
|
@hunner @EmilienM, sorry to hijack, but would love some feedback on https://github.com/puppetlabs/puppetlabs-rabbitmq/pull/493, which also makes a lot of changes in this area (though still uses the same GPG key when the repo is ensured only). There's been some discussion about it in the Puppet Community Slack; would love to get some more ideas and for people more familiar with the codebase to review the changes. This also gets rid of $version, which wasn't honored / ensured for Apt / Yum anyway, and pulls out some confusing parameters that were already deprecated, like $manage_repos. If the regular repos for Debian / Ubuntu have a recent-ish package, I'd be fine with switching to using those by default (vs. the RabbitMQ repo). I've tried to incorporate that change to be default in my other PR, and adjusted some tests accordingly. If anyone has more ideas about whether SLES has an available package without any special configuration, that would also be helpful. |
|
👍 Always support isolated environments first. Nobody wants random repositories from the outside world magically showing up in their trust chain. Likewise, don't grab random tarballs from the Internet and install them without explicit permission. |
|
Ok, defaults to false now in https://github.com/puppetlabs/puppetlabs-rabbitmq/pull/493 |
|
closing because #493 |
6 participants
The problem with it being true by default is that the majority of production
environments, and any in an isolated nextwork, will not want to get the gpg key,
especially if they have not explicitly configured the module to get the rpm from
the rabbitmq repos. Having the gpg key pulled in by default while installing
the rpm from distro repos seems inconsistent at best, and at worst, causes an
additional fail point for deployments that are unable to contact this location.
Leaving it true by default forces the majority of production users to have to
override this setting.