chef / cookbook-omnifetch

Fetch Chef Cookbooks from Various Sources to a Local Cache
Apache License 2.0
6 stars 9 forks source link

No longer able to cache cookbooks with `chef install` #35

Closed jasonwbarnett closed 2 years ago

jasonwbarnett commented 3 years ago

Description

33 introduced an issue where cached cookbooks cannot be used for chefspec testing. I rolled back to the previous version (i.e. v0.10.1) and the issue went away.

I think this may have been caused by this change or that change, but I really don't know.

Version:

v0.11.1

Environment:

OS: Red Hat 7

Scenario:

We have a multi-stage ci/cd pipeline. In one job we cache all cookbooks (CHEF_WORKSTATION_HOME) and that cache gets passed into future jobs that then use the cached CHEF_WORKSTATION_HOME so that future jobs don't depend on chef server.

Steps to Reproduce:

  1. change into cookbook directory that has rspec configured to use policyfiles (require "chefspec/policyfile") and depends on at least a single cookbook
  2. Set CHEF_WORKSTATION_HOME to a temporary directory
    export CHEF_WORKSTATION_HOME=/tmp/chef_workstation_home
  3. Run chef install
  4. Remove ~/.chef/config.rb and ~/.chef/knife.rb This simulates a system which has no access to chef server
  5. Run chef exec rspec

Expected Result:

You should be able to test without any errors.

Actual Result:

Bombs out because it can't resolve cookbook dependencies.

tas50 commented 3 years ago

@marcparadise Any insight into this issue?

marcparadise commented 3 years ago

I didn't see this one, I do wonder if it's a matter of chef install etc not respecting CHEF_WORKSTATION_HOME in all cases.

karmix commented 2 years ago

@jasonwbarnett is correct, #33 breaks the cookbook cache when using a chef server as a cookbook source. In particular, #33 makes it so that chefspec, test-kitchen, and any other tools that rely on chef-cli/policyfile_services to provide a collection of cookbook dependencies now crash unless they have access to the chef server. This access is often not available in certain environments, such as pipelines where you discard your client key with all your other credentials before you test untrusted code from a PR, or when you are trying to be productive while commuting and your laptop does not have network access.

33 was the result of a lengthy discussion in https://github.com/chef/chef-workstation/issues/1273 . Unfortunately, the impact on existing tools was not one of the risks identified. We might need to roll this back as it breaks pre-existing functionality to address what appears to be a rare edge case regarding cache consistency. (Was anybody ever able to reproduce the issue?)

Another solution might be to simply add the chef server and org names to the cache key, preventing similar cache conflicts that can occur when working with different cookbooks in different orgs that share the same name and version. This would also make it far less likely that a tool, berks or otherwise, would create a conflict in the cache directory.

Should we reopen the discussion in https://github.com/chef/chef-workstation/issues/1273, where we have a larger audience, or continue here?

careiley commented 2 years ago

@skkprogress - What are your thoughts here?

i5pranay93 commented 2 years ago

I tried to replicate as @jasonwbarnett steps, i could't replicate it.

After downloading chef server starter kit, i do cd chef-repo

mkdir cookbooks 
cd cookbooks
chef generate cookbook testc
chef generate cookbook testd
cd ..
knife cookbook upload testc
cd cookbooks/testd
#  default_source in Policyfile.rb is supermarket only, not changing to chef server w/ url 
# added cookbook testc to Policyfile.rb
export CHEF_WORKSTATION_HOME=/tmp/chef_workstation_home
 chef install

Removed config.rb and .pem file from /.chef, inside chef-repo starter kit

added some random spec

require 'chefspec'
# includes custom setup/teardown code specifically for testing this in memory
# not required when writing your own tests
#require_relative './policyfile_setup'
require 'chefspec/policyfile'

describe '1 plus 1' do
  it 'equals 2' do
    a = 1
    b = 1
    sum = a + b
    expect(sum).to eq(2)
    expect(sum).not_to eq(3)
  end
end

chef exec rspec runs without any issue.

i5pranay93 commented 2 years ago

Though I cannot see, cache being created after chef installs in ~/.chef-workstation/cache/cookbooks, couldn't find it in ~/.chef either.

jasonwbarnett commented 2 years ago

@i5pranay93 We don't use Chef Supermarket. Can you try testing with only chef infra server as a source?

P.S. if you look at the two changes referenced in the original issue description you'll see the methods changed are only related to chef infra server.

i5pranay93 commented 2 years ago

yes @jasonwbarnett keeping default source as chef server , also gives same result. here is my Policyfile.rb inside cookbbok testd

 Policyfile.rb - Describe how you want Chef Infra Client to build your system.
#
# For more information on the Policyfile feature, visit
# https://docs.chef.io/policyfile/

# A name that describes what the system you're building with Chef does.
name 'testd'

# Where to find external cookbooks:
default_source :chef_server, "https://api.chef.io/organizations/wschef"

# run_list: chef-client will run these recipes in the order specified.
run_list 'testd::default'

# Specify a custom source for a single cookbook:
cookbook 'testd', path: '.'
cookbook  'testc', path: '../testc'

/testd --> chef exec rspec

Finished in 3.44 seconds (files took 6.01 seconds to load)
1 example, 0 failures
i5pranay93 commented 2 years ago

chef version

hef Workstation version: 22.2.807
Chef Infra Client version: 17.9.52
Chef InSpec version: 4.52.9
Chef CLI version: 5.6.1
Chef Habitat version: 1.6.420
Test Kitchen version: 3.2.2
Cookstyle version: 7.32.0

Os - mac os 12.0.1

i5pranay93 commented 2 years ago

On windows Machine aswell,result are same, no chefspec failure.

jasonwbarnett commented 2 years ago

@i5pranay93 I think the reason why this isn't failing is because you have a local source for the cookbook.

cookbook  'testc', path: '../testc'

Try removing that line so that chef-cli attempts to resolve the dependency using chef infra server, aka

default_source :chef_server, "https://api.chef.io/organizations/wschef"

See this diff: https://gist.github.com/jasonwbarnett/a4486008bacfecb77e03ba194d47c1aa/revisions

karmix commented 2 years ago

@i5pranay93:

The cookbook directives in your policy file are giving chef special instructions for locating your cookbook. When you do this, chef will not search the default_source for those cookbooks. In your case, you are installing the cookbooks using the path installer, not the chef_server installer. The path installer does not use the cookbook cache, which is why you don't see anything in your cache directory.

Remove the cookbook entry for testc from your policy file as @jasonwbarnett described and also make sure you list testc as a dependency in your metadata.rb.

Once you have done that, rebuild your lock file and you should see files in your cache directory.

Validate that rspec works, then disconnect from the network, rename your client key, or do anything else that would prevent you from authenticating with the chef server, and run rspec again. It should crash the second time.

i5pranay93 commented 2 years ago

@karmix @jasonwbarnett I was able to replicate spec failure it, with the given error stac

Failure/Error: raise PolicyfileInstallError.new("Failed to install cookbooks from lockfile", error)

ChefCLI::PolicyfileInstallError:
  Failed to install cookbooks from lockfile
# /Users/prsingh/Documents/chef-code-resources/non-forked/chef-cli/lib/chef-cli/policyfile_services/install.rb:164:in `rescue in install_from_lock'
# /Users/prsingh/Documents/chef-code-resources/non-forked/chef-cli/lib/chef-cli/policyfile_services/install.rb:159:in `install_from_lock'
# /Users/prsingh/Documents/chef-code-resources/non-forked/chef-cli/lib/chef-cli/policyfile_services/install.rb:63:in `run'
# /Users/prsingh/.chefdk/gem/ruby/3.0.0/gems/chefspec-9.3.3/lib/chefspec/policyfile.rb:39:in `setup!'
# /Users/prsingh/.chefdk/gem/ruby/3.0.0/gems/chefspec-9.3.3/lib/chefspec/policyfile.rb:67:in `block (2 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# CookbookOmnifetch::MissingConfiguration:
#   `default_chef_server_http_client` is not configured
#   /Users/prsingh/Documents/chef-code-resources/cookbook-omnifetch/lib/cookbook-omnifetch/integration.rb:24:in `block in configurable'

but it failed without disconnecting from network (chep-repo/.chef/config.rb, remained same).

To confirm if new changes were functional after https://github.com/chef/cookbook-omnifetch/pull/41 change I cloned @karmix changes(https://github.com/chef/cookbook-omnifetch/pull/41)

in my chef-cli(https://github.com/chef/chef-cli) repo i added new cookbook-omnifetch repo in gemfile, also added chefspec gem

 gem "cookbook-omnifetch", path: "../../cookbook-omnifetch"
 gem "chefspec"

and repeated repro this time using bundle exec (e.g bundle exec chef install) bundle exec chef exec spec fails with the same error.

cache path has cache 
└── cache
    └── cookbooks
        ├── testa-0.1.0
        │   ├── CHANGELOG.md
        │   ├── LICENSE
        │   ├── README.md
        │   ├── chefignore
        │   ├── compliance
        │   │   └── README.md
        │   ├── metadata.json
        │   ├── metadata.rb
        │   └── recipes
        │       └── default.rb
        └── testc-0.1.0
            ├── CHANGELOG.md
            ├── LICENSE
            ├── README.md
            ├── chefignore
            ├── compliance
            │   └── README.md
            ├── metadata.json
            ├── metadata.rb
            └── recipes
                └── default.rb
i5pranay93 commented 2 years ago

adding this to _spec.rb worked, apparantely

CookbookOmnifetch.configure do |config|
  chef_server_url = 'https://your-chef-server/organizations/your-org'
  config.default_chef_server_http_client = ChefCLI::ChefServerAPIMulti.new(chef_server_url, {})
end
i5pranay93 commented 2 years ago

confirmed, @karmix changes(https://github.com/chef/cookbook-omnifetch/pull/41) seems to be working so far.

karmix commented 2 years ago

@i5pranay93: Correct, the lack of default_chef_server_http_client in the config is a separate issue tracked in chefspec/chefspec#934. I've been meaning to address that for a while, but haven't found the time. What you have done should work fine in this case. We have an even uglier hack so we don't have to hard code the chef server in spec_helper.rb:

# Load the chef-cli config so rspec has the config it needs to work with policy files.
require "mixlib/cli"
require 'chef-cli/configurable'
class FakeCommand
  include Mixlib::CLI
  include ChefCLI::Configurable
end
FakeCommand.new.chef_config
jasonwbarnett commented 2 years ago

@i5pranay93: Correct, the lack of default_chef_server_http_client in the config is a separate issue tracked in chefspec/chefspec#934. I've been meaning to address that for a while, but haven't found the time. What you have done should work fine in this case. We have an even uglier hack so we don't have to hard code the chef server in spec_helper.rb:

# Load the chef-cli config so rspec has the config it needs to work with policy files.
require "mixlib/cli"
require 'chef-cli/configurable'
class FakeCommand
  include Mixlib::CLI
  include ChefCLI::Configurable
end
FakeCommand.new.chef_config

We ended up writing a gem and requiring it across all of our spec_helper.rb in our cookbooks to fill in the gap.

require 'our/custom/gem/chefspec_helper'