DataDog / chef-datadog

Chef cookbook for Datadog Agent & Integrations
https://www.datadoghq.com
Apache License 2.0
97 stars 262 forks source link

repository should not enforce gpgcheck when gpgkey is nil or empty String #349

Open haidangwa opened 8 years ago

haidangwa commented 8 years ago

node['datadog']['gpgkey'] can now be set to nil or empty String. This should automatically set the yum_repository resource to false.

  yum_repository 'datadog' do
    name 'datadog'
    description 'datadog'
    url node['datadog']['yumrepo']
    proxy node['datadog']['yumrepo_proxy']
    proxy_username node['datadog']['yumrepo_proxy_username']
    proxy_password node['datadog']['yumrepo_proxy_password']
    gpgkey node['datadog']['yumrepo_gpgkey']
    gpgcheck false if node['datadog']['yumrepo_gpgkey'].nil? || node['datadog']['yumrepo_gpgkey'].empty?
    action :add
  end

Without setting gpgcheck=0, this could cause a chef run to fail if the gpgkey is nil, for example, when we use an internal yum repo that doesn't have a gpgkey.

olivielpeau commented 8 years ago

Hi @haidangwa and thanks for reporting this!

I'm not sure what the correct approach is here: we could also add an attribute like node['datadog']['yumrepo_gpgcheck'] that would need to be explicitly set to false to disable gpg verification (this would follow the yum cookbook's default more closely, see https://github.com/chef-cookbooks/yum/pull/66).

As an aside, since the packages themselves are signed (i.e. not the repository metadata), if you're using the official yum packages in your internal yum repo you could use gpg verification on your internal yum repo by making the public key available on your internal network. Does that make sense?

haidangwa commented 8 years ago

@olivielpeau a new attribute to explicitly set it would be a good thing. As it was in the current code, there was no such setting and it seemed that by setting the gpgkey attribute to nil that it wouldn't attempt to check, but it did. I'd rather go the more secure route, anyway, so we did import the key into our repo and set the gpgkey URL to the internal repo. However, it wasn't apparent to me at first, because we "trust" our internal repos.

I'd be for logging a "WARN" that gpgcheck is disabled and is a less secure installation.

olivielpeau commented 8 years ago

Thanks for your response!

I'm going to put the issue in triage until we can assess if there are use cases where disabling the gpgcheck is needed. I'd prefer the cookbook to enforce the gpgcheck unless there are cases where disabling it is unavoidable.