chef-cookbooks / chef-server

Cookbook to install standalone Chef Server
http://supermarket.chef.io/cookbooks/chef-server
Apache License 2.0
176 stars 154 forks source link

add support for "source" attributes to chef-ingredient #152

Open qubitrenegade opened 6 years ago

qubitrenegade commented 6 years ago

Cookbook version

5.5.1

Chef-client version

any (we support 12 and 13 internally)

Platform Details

CentOS (any)

Scenario:

We need to be able to specify the location that the Chef RPMs come from. Supplying a "repo" cookbook is not sufficient as we are not installing from repos.

Steps to Reproduce:

I was able to solve my issue by using edit_resource!, however, this feels like it's prone to failure... and could make debugging hard:

{
  'chef-server' => 'https://some.internal.address/chef-server.rpm',
  'chef-manage' => 'https://some.internal.address/chef-manage.rpm'
}.each do |pkg, src|
  edit_resource!(:chef_ingredient, pkg) do
    package_source source
  end
end

Expected Result:

What I would like to see is a method of passing the source RPM to the appropriate blocks. For instance, I was thinking instead of passing a hash of package => version you could pass a package => config_hash such that:

default['chef-server']['addons'] = {'chef-manage' => '2.5.0', reporting: nil}

Becomes:

default['chef-server']['addons'] = {
  'chef-manage': {
    version: '2.5.0',
    source: 'https://foo.bar/chef-manage.rpm'
  },
  push-jobs: {
    version: '1.2.3',
  },
  reporting: nil
}

Then this could be consumed by updating recipes/addons.rb to

node['chef-server']['addons'].each do |addon, cfg|
 chef_ingredient addon do
   accept_license node['chef-server']['accept_license'] unless node['chef-server']['accept_license'].nil?
   notifies :reconfigure, "chef_ingredient[#{addon}]"
   version cfg['ver'] unless cfg['ver'].nil?
   package_source cfg['src'] unless cfg['src'].nil?
 end
end

I think this approach provides the most obvious method of configuration, however, it comes at a cost of backwards incompatibility...

The other solution I was thinking was we could add some logic to recipes/addons.rb

node['chef-server']['addons'].each do |addon, cfg|
 chef_ingredient addon do
   accept_license node['chef-server']['accept_license'] unless node['chef-server']['accept_license'].nil?
   notifies :reconfigure, "chef_ingredient[#{addon}]"
   if cfg.is_a?(Hash)
     version cfg['ver'] unless cfg['ver'].nil?
     package_source cfg['src'] unless cfg['src'].nil?
    elsif cfg =~ /^(http|https|file):\/\//
      package_source cfg
    elsif cfg =~ /^\d+\.\d+\.\d+/
      version cfg
    end
 end
end

This would be less clean, but would be backwards compatible... I also question if there are other sources, i.e. if you could feed package_source just /path/to/chef-manage.rpm instead of file://path/to/chef-manage.rpm (which would make my regex invalid...)

Actual Result:

I'm more than happy to write the patch and open a pull reuqest, but wanted some feedback from the Chef folks as to which approach would be the more appropriate/most likely to get pulled.

Thanks! -Q

tas50 commented 5 years ago

I'd certainly be open to a PR that would add just this if you or someone else reading this is interested in this feature.

qubitrenegade commented 5 years ago

@tas50 and thoughts/comments on the proposed approaches?

Thanks! -Q

On Tue, Jan 15, 2019, 21:19 Tim Smith notifications@github.com wrote:

I'd certainly be open to a PR that would add just this if you or someone else reading this is interested in this feature.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/chef-cookbooks/chef-server/issues/152#issuecomment-454646498, or mute the thread https://github.com/notifications/unsubscribe-auth/AOQTiVG180_H-1FzN6s2R0qrlELtAuW4ks5vDqhAgaJpZM4RSVYi .