From b96bd8410987d161cdf458b917398be322301073 Mon Sep 17 00:00:00 2001 From: Thom May Date: Wed, 14 Dec 2016 17:08:30 +0000 Subject: [PATCH] Segmentless cookbook data (RFC 67) Implement RFC 67, which causes us to bump our max API version to 2. * Accept segmentless and segmented downloads and uploads of cookbooks * Add pedant tests Signed-off-by: Thom May --- oc-chef-pedant/lib/pedant/request.rb | 2 +- .../lib/pedant/rspec/cookbook_util.rb | 268 ++- .../api/cookbook_artifacts/create_spec.rb | 780 ++++----- .../api/cookbook_artifacts/delete_spec.rb | 246 +-- .../spec/api/cookbook_artifacts/read_spec.rb | 334 ++-- .../api/cookbook_artifacts/update_spec.rb | 157 +- .../spec/api/cookbooks/create_spec.rb | 610 +++---- .../spec/api/cookbooks/delete_spec.rb | 245 +-- .../spec/api/cookbooks/named_filters_spec.rb | 300 ++-- .../spec/api/cookbooks/read_spec.rb | 518 +++--- .../spec/api/cookbooks/update_spec.rb | 1516 +++++++++-------- .../api/cookbooks/version_conversion_spec.rb | 116 ++ .../spec/api/cookbooks/version_spec.rb | 174 +- .../src/chef_cookbook_version.erl | 212 ++- .../test/chef_cookbook_version_tests.erl | 30 +- .../src/oc_chef_cookbook_artifact_version.erl | 16 +- .../oc_chef_wm_cookbook_artifact_SUITE.erl | 2 +- .../src/chef_wm_cookbook_version.erl | 10 +- .../apps/oc_chef_wm/src/chef_wm_depsolver.erl | 4 +- src/oc_erchef/include/server_api_version.hrl | 5 +- 20 files changed, 3126 insertions(+), 2419 deletions(-) create mode 100644 oc-chef-pedant/spec/api/cookbooks/version_conversion_spec.rb diff --git a/oc-chef-pedant/lib/pedant/request.rb b/oc-chef-pedant/lib/pedant/request.rb index 577a60cf618..ff2839c49d9 100644 --- a/oc-chef-pedant/lib/pedant/request.rb +++ b/oc-chef-pedant/lib/pedant/request.rb @@ -191,7 +191,7 @@ def reset_server_api_version $server_api_version = Pedant::Config.server_api_version end def use_max_server_api_version - $server_api_version = 1 # TODO Pedant::Config.max_server_api_version + $server_api_version = 2 # TODO Pedant::Config.max_server_api_version end end diff --git a/oc-chef-pedant/lib/pedant/rspec/cookbook_util.rb b/oc-chef-pedant/lib/pedant/rspec/cookbook_util.rb index 60efc254641..ef40e0238b2 100644 --- a/oc-chef-pedant/lib/pedant/rspec/cookbook_util.rb +++ b/oc-chef-pedant/lib/pedant/rspec/cookbook_util.rb @@ -176,12 +176,11 @@ def generate_dummy_checksum() end end - def new_cookbook_artifact(name, identifier, opts = {}) + def new_cookbook_artifact_v0(name, identifier, opts = {}) result = { "name" => "#{name}", "identifier" => identifier, "version" => opts[:version] || default_version, # version doesn't matter for cookbook_artifacts - "json_class" => "Chef::CookbookVersion", "chef_type" => "cookbook_version", "frozen?" => false, "recipes" => opts[:recipes] || [], @@ -212,6 +211,50 @@ def new_cookbook_artifact(name, identifier, opts = {}) result end + def new_cookbook_artifact_v2(name, identifier, opts = {}) + if opts.key?(:recipes) + recipes = opts[:recipes].map do |r| + r["name"] = "recipes/#{r["name"]}" + r + end + end + + all_files = recipes || [] + + result = { + "name" => "#{name}", + "identifier" => identifier, + "version" => opts[:version] || default_version, # version doesn't matter for cookbook_artifacts + "chef_type" => "cookbook_version", + "frozen?" => false, + "all_files" => all_files, + "metadata" => { + "version" => opts[:version] || default_version, + "name" => name, # not actually used + "maintainer" => opts[:maintainer] || default_maintainer, + "maintainer_email" => opts[:maintainer_email] || default_maintainer_email, + "description" => opts[:description] || default_description, + "long_description" => opts[:long_description] || default_long_description, + "license" => opts[:license] || default_license, + "dependencies" => opts[:dependencies] || {}, + "attributes" => opts[:attributes] || {}, + # this recipies list is not the same as the top level list + # this is a list of recipes and their descriptions + "recipes" => opts[:meta_recipes] || {} + } + } + result["metadata"]["providing"] = opts[:providing] if opts[:providing] + result + end + + def new_cookbook_artifact(name, identifier, opts = {}) + if platform.server_api_version >= 2 + new_cookbook_artifact_v2(name, identifier, opts) + else + new_cookbook_artifact_v0(name, identifier, opts) + end + end + def delete_cookbook_artifact(requestor, name, identifier) res = delete(api_url("/#{cookbook_url_base}/#{name}/#{identifier}"), requestor) @@ -237,7 +280,7 @@ def make_cookbook_artifact_with_recipes(cookbook_name, identifier, recipe_list) opts = { recipes: recipes, meta_recipes: recipes.each_with_object({}) { |r,h| h["#{cookbook_name}::#{r["name"][0..-4]}"] = "" }, - providing: recipes.each_with_object({}) { |r,h| h["#{cookbook_name}::#{r["name"][0..-4]}"] = ">= 0.0.0" } + providing: recipes.each_with_object({}) { |r,h| h["#{cookbook_name}::#{r["name"][0..-4]}"] = ">= 0.0.0" }, } make_cookbook_artifact(admin_user, cookbook_name, identifier, opts) end @@ -248,11 +291,11 @@ def make_cookbook_artifact_with_recipes(cookbook_name, identifier, recipe_list) # and passed in as function arugments or automatically computed if a # block is provided. def verify_checksum_cleanup(segment_type, existing_checksums=nil, updated_checksums=nil, &block) - existing_checksums ||= checksums_for_segment_type(segment_type) + existing_checksums ||= checksums_for_type(segment_type) yield if block_given? - updated_checksums ||= checksums_for_segment_type(segment_type) + updated_checksums ||= checksums_for_type(segment_type) deletions = existing_checksums.keys - updated_checksums.keys @@ -298,7 +341,7 @@ def delete_cookbook(requestor, name, version) def make_cookbook(requestor, name, version, opts={}) payload = new_cookbook(name, version, opts) - upload_cookbook(requestor, name, version, payload) + ensure_2xx(upload_cookbook(requestor, name, version, payload)) end def upload_cookbook(requestor, name, version, payload) @@ -306,8 +349,8 @@ def upload_cookbook(requestor, name, version, payload) requestor, :payload => payload) end - def new_cookbook(name, version, opts = {}) - { + def new_cookbook_v0(name, version, opts = {}) + cb = { "name" => "#{name}-#{version}", "cookbook_name" => name, "version" => version, # not actually used @@ -330,44 +373,75 @@ def new_cookbook(name, version, opts = {}) "recipes" => opts[:meta_recipes] || {} } } + if opts.key?(:payload) + opts[:payload].each do |part, files| + cb[part.to_s] ||= [] + cb[part.to_s] += files + end + end + cb end - shared(:default_description) { "A fabulous new cookbook" } - shared(:default_long_description) { "" } - shared(:default_maintainer) { "Your Name" } - shared(:default_maintainer_email) { "youremail@example.com" } - shared(:default_license) { "Apache v2.0" } + def new_cookbook_v2(name, version, opts = {}) + all_files = [] + if opts.key?(:recipes) + all_files = opts[:recipes].each_with_object([]) do |r, acc| + af = r.dup + af["name"] = "recipes/#{r['name']}" + acc << af + end + end + + if opts.key?(:payload) + opts[:payload].each do |part, files| + all_files += files.each_with_object([]) do |f, acc| + file = f.dup + file["name"] = "#{part}/#{file["name"]}" + acc << file + end + end + end - def full_cookbook(name, version, opts = {}) { "name" => "#{name}-#{version}", "cookbook_name" => name, + "version" => version, # not actually used + "json_class" => "Chef::CookbookVersion", + "chef_type" => "cookbook_version", + "frozen?" => false, + "all_files" => all_files.flatten, + "metadata" => { "version" => version, - "json_class" => "Chef::CookbookVersion", - "chef_type" => "cookbook_version", - "recipes" => opts[:recipes] || [], - "metadata" => { - "name" => name, - "description" => opts[:description] || default_description, - "long_description" => opts[:long_description] || default_long_description, + "name" => name, # not actually used "maintainer" => opts[:maintainer] || default_maintainer, "maintainer_email" => opts[:maintainer_email] || default_maintainer_email, + "description" => opts[:description] || default_description, + "long_description" => opts[:long_description] || default_long_description, "license" => opts[:license] || default_license, - "platforms" => {}, - "dependencies" => {}, - "providing" => {}, - "attributes" => {}, - "recipes" => opts[:meta_recipes] || {}, - "version" => version - }, - "frozen?" => opts[:frozen] || false + "dependencies" => opts[:dependencies] || {}, + "attributes" => opts[:attributes] || {}, + # this recipies list is not the same as the top level list + # this is a list of recipes and their descriptions + "recipes" => opts[:meta_recipes] || {} + } } end - # We don't return all the metadata when fetching a cookbook via - # the API because it's not used by the client and wastes - # bandwidth - def retrieved_cookbook(name, version, opts = {}) + def new_cookbook(name, version, opts = {}) + if platform.server_api_version >= 2 + new_cookbook_v2(name, version, opts) + else + new_cookbook_v0(name, version, opts) + end + end + + shared(:default_description) { "A fabulous new cookbook" } + shared(:default_long_description) { "" } + shared(:default_maintainer) { "Your Name" } + shared(:default_maintainer_email) { "youremail@example.com" } + shared(:default_license) { "Apache v2.0" } + + def retrieved_cookbook_v0(name, version, opts = {}) cookbook = { "name" => "#{name}-#{version}", "version" => version, @@ -395,6 +469,53 @@ def retrieved_cookbook(name, version, opts = {}) cookbook end + def retrieved_cookbook_v2(name, version, opts = {}) + all_files = nil + if (recipes = opts.key?(:recipes)) && recipes.is_a?(Array) + all_files = recipes.map do |r| + r["name"] = "recipes/#{r["name"]}" + r + end + end + + cookbook = { + "name" => "#{name}-#{version}", + "version" => version, + "cookbook_name" => name, + "json_class" => "Chef::CookbookVersion", + "chef_type" => "cookbook_version", + "frozen?" => opts[:frozen] || false, + "all_files" => all_files || opts[:recipes] || [] + } + + metadata = { + "attributes" => {}, + "dependencies" => {}, + "description" => opts[:description] || default_description, + "license" => opts[:license] || default_license, + "long_description" => opts[:long_description] || default_long_description, + "maintainer" => opts[:maintainer] || default_maintainer, + "maintainer_email" => opts[:maintainer_email] || default_maintainer_email, + "name" => name, + "recipes" => opts[:meta_recipes] || {}, + "version" => version + } + + cookbook["metadata"] = metadata + cookbook + end + + # We don't return all the metadata when fetching a cookbook via + # the API because it's not used by the client and wastes + # bandwidth + def retrieved_cookbook(name, version, opts = {}) + if platform.server_api_version >= 2 + retrieved_cookbook_v2(name, version, opts) + else + retrieved_cookbook_v0(name, version, opts) + end + end + # Create a dummy recipe for a cookbook recipe manifest. The # checksum is assumed to already exist in the organization. def dummy_recipe(name, checksum) @@ -496,9 +617,17 @@ def get_latest_cookbooks(cookbook_spec, num_versions=1) # "https://...", # } # + + def checksums_for_type(type, cb_version = cookbook_version) + if platform.server_api_version >= 2 + checksums_for_all_files(type, cb_version) + else + checksums_for_segment_type(type, cb_version) + end + end + def checksums_for_segment_type(segment_type, cb_version=cookbook_version) - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cb_version}"), - admin_user) do |response| + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cb_version}"), admin_user) do |response| segment_contents = parse(response)[segment_type.to_s] || [] segment_contents.inject({}) do |return_hash, segment_member| return_hash[segment_member['checksum']] = segment_member['url'] @@ -507,6 +636,33 @@ def checksums_for_segment_type(segment_type, cb_version=cookbook_version) end end + def checksums_for_all_files(type, cb_version = cookbook_version) + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cb_version}"), admin_user) do |response| + extract_segment(parse(response), type).each_with_object({}) { |file, acc| acc[file["checksum"]] = file["url"] } + end + end + + def extract_segment(cbv, segment) + if platform.server_api_version >= 2 + files = cbv["all_files"] || [] + files.select do |f| + seg, name = f["name"].split("/") + seg = "root_files" if name.nil? + seg == segment.to_s + end + else + cbv[segment.to_s] + end + end + + def select_segment(segment) + if platform.server_api_version >= 2 + "all_files" + else + segment + end + end + module ClassMethods # This is used for testing creates with changes to the default @@ -526,7 +682,7 @@ def should_create(key, value, ignores_value = false, actual_value = nil, metadat # key: key to change # value: value to use def should_not_change(key, value, actual_value) - should_change(key, value, true, actual_value) + should_change(key, value, true, actual_value, false, {}) end # This is used for testing updates with changes to the default @@ -548,7 +704,7 @@ def should_change(key, value, ignores_value = false, actual_value = nil, payload[key] = value end put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| + admin_user, :payload => payload) do |response| if (ignores_value) payload[key] = actual_value end @@ -560,14 +716,13 @@ def should_change(key, value, ignores_value = false, actual_value = nil, end # Verified change (or creation) happened - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) - end + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| + response. + should look_like({ + :status => 200, + :body => payload + }) + end end end @@ -622,8 +777,7 @@ def should_fail_to_change(key, value, error, message, server_error = false, # Verified change (or creation) did not happen if (create) - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| response. should look_like({ :status => 404 @@ -686,19 +840,21 @@ def should_change_metadata(key, value, new_value = nil, _expected_status = 200, end put_payload["metadata"] = put_metadata - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, :payload => put_payload) do |response| + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, + :payload => put_payload) do |response| # The PUT response returns the payload exactly as it was sent response.should look_like({:status => _expected_status, :body_exact => put_payload}) end - # Verified change (or creation) happened - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| - get_response = cookbook.dup - if (new_value) - get_metadata = get_response["metadata"] - get_metadata[key] = new_value - get_response["metadata"] = get_metadata - end + # Verified change (or creation) happened + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, + ) do |response| + get_response = cookbook.dup + if (new_value) + get_metadata = get_response["metadata"] + get_metadata[key] = new_value + get_response["metadata"] = get_metadata + end response.should look_like({:status => 200, :body => get_response}) end diff --git a/oc-chef-pedant/spec/api/cookbook_artifacts/create_spec.rb b/oc-chef-pedant/spec/api/cookbook_artifacts/create_spec.rb index 6b5987054d2..687610cc162 100644 --- a/oc-chef-pedant/spec/api/cookbook_artifacts/create_spec.rb +++ b/oc-chef-pedant/spec/api/cookbook_artifacts/create_spec.rb @@ -19,516 +19,518 @@ describe "Cookbook Artifacts API endpoint", :cookbook_artifacts, :cookbook_artifacts_create do - let(:cookbook_url_base) { "cookbook_artifacts" } - - let(:default_cookbook_id) { "1111111111111111111111111111111111111111" } - - include Pedant::RSpec::CookbookUtil - - let(:default_version) { "1.0.0" } - - context "PUT /cookbook_artifacts// [create]" do - - include Pedant::RSpec::Validations::Create - - let(:request_method){:PUT} - let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} - shared(:requestor){admin_user} - - let(:cookbook_artifact_to_create){ new_cookbook_artifact(cookbook_name, cookbook_identifier)} - - context 'with a basic cookbook', :smoke do - after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } - - let(:cookbook_name) { "pedant_basic" } - let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } - - let(:expected_get_response_data) do - { - "name"=>"pedant_basic", - "identifier"=>"1111111111111111111111111111111111111111", - "version"=>"1.0.0", - "attributes" => [], - "chef_type"=>"cookbook_version", - "definitions" => [], - "files" => [], - "frozen?"=>false, - "providers" => [], - "recipes"=>[], - "resources" => [], - "root_files" => [], - "templates" => [], - "libraries" => [], - "metadata"=> { - "version"=>"1.0.0", - "name"=>"pedant_basic", - "maintainer"=>"Your Name", - "maintainer_email"=>"youremail@example.com", - "description"=>"A fabulous new cookbook", - "long_description"=>"", - "license"=>"Apache v2.0", - "dependencies"=>{}, - "attributes"=>{}, - "recipes"=>{} - } - } - end - - it "creates a basic cookbook_artifact" do - # create it: - create_response = put(request_url, requestor, payload: cookbook_artifact_to_create) - expect(create_response.code.to_s).to eq("201") - - # list: - list_response = get(api_url("/#{cookbook_url_base}/#{cookbook_name}"), requestor) + shared_examples "creates cookbook artifacts" do |api_version| + let(:api_version) { api_version } - list_data = parse(list_response) - expect(list_data).to have_key("pedant_basic") - expect(list_data["pedant_basic"]["versions"].size).to eq(1) - expect(list_data["pedant_basic"]["versions"].first["identifier"]).to eq(cookbook_identifier) + before do + platform.server_api_version = api_version + end - # fetch it: - get_response = get(request_url, requestor) + after do + platform.reset_server_api_version + end - expect(get_response.code.to_s).to eq("200") - expect(parse(get_response)).to eq(expected_get_response_data) + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" end end - # Start using the new validation macros - context "when validating" do + let(:cookbook_url_base) { "cookbook_artifacts" } - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } + let(:default_cookbook_id) { "1111111111111111111111111111111111111111" } - let(:resource_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} - let(:persisted_resource_response){ get(resource_url, requestor) } + include Pedant::RSpec::CookbookUtil - after(:each){ delete_cookbook_artifact(requestor, cookbook_name, cookbook_identifier)} + let(:default_version) { "1.0.0" } - context "the cookbook version" do + context "PUT /cookbook_artifacts// [create]" do - let(:cookbook_artifact_to_create) do - new_cookbook_artifact(cookbook_name, cookbook_identifier, version: cookbook_version) - end + include Pedant::RSpec::Validations::Create - let(:request_payload) { cookbook_artifact_to_create } - let(:default_resource_attributes) { cookbook_artifact_to_create } + let(:request_method){:PUT} + let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} + shared(:requestor){admin_user} - context "with versions at exactly 4 bytes" do - int4_exact = "2147483647" - let(:cookbook_version) { "1.2.#{int4_exact}" } - it { should look_like http_201_response } - end + let(:cookbook_artifact_to_create){ new_cookbook_artifact(cookbook_name, cookbook_identifier)} - context "with versions larger than 4 bytes" do - int4_overflow = "2147483669" # max = 2147483647 (add 42) - let(:cookbook_version) { "1.2.#{int4_overflow}" } - it { should look_like http_201_response } + context 'with a basic cookbook', :smoke do + after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } + + let(:cookbook_name) { "pedant_basic" } + let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } + + let(:expected_get_response_data) do + new_cookbook_artifact(cookbook_name, cookbook_identifier, version: "1.0.0") end - context "with versions with 'prerelease' fields" do + it "creates a basic cookbook_artifact" do + # create it: + create_response = put(request_url, requestor, payload: cookbook_artifact_to_create) + expect(create_response.code.to_s).to eq("201") + + # list: + list_response = get(api_url("/#{cookbook_url_base}/#{cookbook_name}"), requestor) - let(:cookbook_version) { "1.2.3.beta.5" } + list_data = parse(list_response) + expect(list_data).to have_key("pedant_basic") + expect(list_data["pedant_basic"]["versions"].size).to eq(1) + expect(list_data["pedant_basic"]["versions"].first["identifier"]).to eq(cookbook_identifier) - it { should look_like http_201_response } + # fetch it: + get_response = get(request_url, requestor) + + expect(get_response.code.to_s).to eq("200") + expect(parse(get_response)).to eq(expected_get_response_data) end end - end + # Start using the new validation macros + context "when validating" do - context "creating broken cookbook_artifacts to test validation and defaults", :validation do - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_version) { "1.2.3" } + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } - let(:cookbook_identifier) { default_cookbook_id } - let(:base_payload) do - new_cookbook_artifact(cookbook_name, cookbook_identifier, version: cookbook_version).tap do |ca| - ca.delete("json_class") - end - end + let(:resource_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} + let(:persisted_resource_response){ get(resource_url, requestor) } - # delete b/c some tests actually do succeed w/ create or to clean up - # after failed negative test. - after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } + after(:each){ delete_cookbook_artifact(requestor, cookbook_name, cookbook_identifier)} - def update_payload(key, value) - base_payload.dup.tap do |payload| - if (value == :delete) - payload.delete(key) - else - payload[key] = value + context "the cookbook version" do + + let(:cookbook_artifact_to_create) do + new_cookbook_artifact(cookbook_name, cookbook_identifier, version: cookbook_version) end - end - end - shared_examples_for "valid_cookbook_artifact" do + let(:request_payload) { cookbook_artifact_to_create } + let(:default_resource_attributes) { cookbook_artifact_to_create } - it "create returns 201" do - #create it - put(request_url, requestor, :payload => payload) do |response| - expect(response.code).to eq(201) + context "with versions at exactly 4 bytes" do + int4_exact = "2147483647" + let(:cookbook_version) { "1.2.#{int4_exact}" } + it { should look_like http_201_response } end - # verify it looks correct - get(request_url, requestor) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) + context "with versions larger than 4 bytes" do + int4_overflow = "2147483669" # max = 2147483647 (add 42) + let(:cookbook_version) { "1.2.#{int4_overflow}" } + it { should look_like http_201_response } end - end - end - shared_examples_for "invalid_cookbook_artifact" do - it "create returns 400" do - # create should respond with error - put(request_url, requestor, :payload => payload) do |response| - expect(response).to look_like({ - :status => 400, - :body_exact => {"error" => [error_message]} - }) - end + context "with versions with 'prerelease' fields" do - # double check it did't get created - get(request_url, requestor) do |response| - expect(response).to look_like({ - :status => 404 - }) + let(:cookbook_version) { "1.2.3.beta.5" } + + it { should look_like http_201_response } end end - end - context "basic tests" do - after(:each) do - delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) - end + end - context "with empty metadata", skip: "FIXME - server needs to add validation" do - let(:payload) { update_payload("metadata", {}) } - let(:error_message) { "Field 'metadata.version' missing" } + context "creating broken cookbook_artifacts to test validation and defaults", :validation do + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_version) { "1.2.3" } - include_examples("invalid_cookbook_artifact") + let(:cookbook_identifier) { default_cookbook_id } + let(:base_payload) do + new_cookbook_artifact(cookbook_name, cookbook_identifier, version: cookbook_version).tap do |ca| + ca.delete("json_class") + end end - end # context basic tests - - segment_names = %w{resources providers recipes definitions libraries attributes files templates root_files} - segment_names.each do |segment| - context "with segment '#{segment}' set to a String", skip: "FIXME - server returns 500" do + # delete b/c some tests actually do succeed w/ create or to clean up + # after failed negative test. + after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } - let(:payload) { update_payload(segment, "foo") } - let(:error_message) { "Field '#{segment}' invalid" } - - include_examples("invalid_cookbook_artifact") + def update_payload(key, value) + base_payload.dup.tap do |payload| + if (value == :delete) + payload.delete(key) + else + payload[key] = value + end + end end - context "with segment '#{segment}' set to an Array with one empty JSON object", skip: "FIXME - server returns 500" do + shared_examples_for "valid_cookbook_artifact" do - let(:payload) { update_payload(segment, [ {} ]) } - let(:error_message) { "Invalid element in array value of '#{segment}'." } - - include_examples("invalid_cookbook_artifact") + it "create returns 201" do + #create it + put(request_url, requestor, :payload => payload) do |response| + expect(response.code).to eq(201) + end + # verify it looks correct + get(request_url, requestor) do |response| + response. + should look_like({ + :status => 200, + :body => payload + }) + end + end end - end # context checking segments - context "checking metadata sections" do + shared_examples_for "invalid_cookbook_artifact" do + it "create returns 400" do + # create should respond with error + put(request_url, requestor, :payload => payload) do |response| + expect(response).to look_like({ + :status => 400, + :body_exact => {"error" => [error_message]} + }) + end - def update_payload_metadata(key, value) - base_payload.dup.tap do |payload| - payload["metadata"] = payload["metadata"].dup.tap do |md| - if (value == :delete) - md.delete(key) - else - md[key] = value - end + # double check it did't get created + get(request_url, requestor) do |response| + expect(response).to look_like({ + :status => 404 + }) end end end - let(:malformed_constraint) { "s395dss@#" } + context "basic tests" do + after(:each) do + delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) + end + + context "with empty metadata", skip: "FIXME - server needs to add validation" do + let(:payload) { update_payload("metadata", {}) } + let(:error_message) { "Field 'metadata.version' missing" } - %w{platforms dependencies}.each do |section| + include_examples("invalid_cookbook_artifact") + end + end # context basic tests - context "with metadata section '#{section}' set to a string", skip: "FIXME - missing server validation" do + segment_names = %w{resources providers recipes definitions libraries attributes files templates root_files} + segment_names.each do |segment| - let(:payload) { update_payload_metadata(section, "foo") } - let(:error_message) { "Field 'metadata.#{section}' invalid" } + context "with segment '#{segment}' set to a String", skip: "FIXME - server returns 500" do - include_examples("invalid_cookbook_artifact") + let(:payload) { update_payload(segment, "foo") } + let(:error_message) { "Field '#{segment}' invalid" } + include_examples("invalid_cookbook_artifact") end - context "with a malformed constraint for metadata section #{section}", skip: "FIXME - missing server validation" do + context "with segment '#{segment}' set to an Array with one empty JSON object", skip: "FIXME - server returns 500" do - let(:payload) { update_payload_metadata(section, {"foo" => malformed_constraint}) } - let(:error_message) { "Invalid value #{malformed_constraint} for metadata.#{section}" } + let(:payload) { update_payload(segment, [ {} ]) } + let(:error_message) { "Invalid element in array value of '#{segment}'." } include_examples("invalid_cookbook_artifact") + + end + end # context checking segments + + context "checking metadata sections" do + + def update_payload_metadata(key, value) + base_payload.dup.tap do |payload| + payload["metadata"] = payload["metadata"].dup.tap do |md| + if (value == :delete) + md.delete(key) + else + md[key] = value + end + end + end end - end + let(:malformed_constraint) { "s395dss@#" } + + %w{platforms dependencies}.each do |section| + context "with metadata section '#{section}' set to a string", skip: "FIXME - missing server validation" do - ["> 1.0", "< 2.1.2", "3.3", "<= 4.6", "~> 5.6.2", ">= 6.0"].each do |dep| + let(:payload) { update_payload_metadata(section, "foo") } + let(:error_message) { "Field 'metadata.#{section}' invalid" } + + include_examples("invalid_cookbook_artifact") - context "with valid metadata dependency '#{dep}'" do - let(:payload) do - update_payload_metadata("dependencies", {"chef-client" => "> 2.0.0", "apt" => dep}) end - include_examples("valid_cookbook_artifact") + context "with a malformed constraint for metadata section #{section}", skip: "FIXME - missing server validation" do + + let(:payload) { update_payload_metadata(section, {"foo" => malformed_constraint}) } + let(:error_message) { "Invalid value #{malformed_constraint} for metadata.#{section}" } + + include_examples("invalid_cookbook_artifact") + end end - end - ["> 1", "< 2", "3", "<= 4", "~> 5", ">= 6", "= 7", ">= 1.2.3.4", "<= 5.6.7.8.9.0"].each do |dep| - context "with invalid metadata dependency '#{dep}'", skip: "FIXME - missing server validation" do - let(:payload) do - update_payload_metadata("dependencies", {"chef-client" => "> 2.0.0", "apt" => dep}) - end + ["> 1.0", "< 2.1.2", "3.3", "<= 4.6", "~> 5.6.2", ">= 6.0"].each do |dep| - let(:error_message) { "Invalid value '#{dep}' for metadata.dependencies" } + context "with valid metadata dependency '#{dep}'" do + let(:payload) do + update_payload_metadata("dependencies", {"chef-client" => "> 2.0.0", "apt" => dep}) + end - include_examples("invalid_cookbook_artifact") + include_examples("valid_cookbook_artifact") + + end end - end - [ - 'cats::sleep', - 'here(:kitty, :time_to_eat)', - 'service[snuggle]', - '', - 1, - true, - ['cats', 'sleep', 'here'], + ["> 1", "< 2", "3", "<= 4", "~> 5", ">= 6", "= 7", ">= 1.2.3.4", "<= 5.6.7.8.9.0"].each do |dep| - { 'cats::sleep' => '0.0.1', - 'here(:kitty, :time_to_eat)' => '0.0.1', - 'service[snuggle]' => '0.0.1' } - ].each do |valid_provides| + context "with invalid metadata dependency '#{dep}'", skip: "FIXME - missing server validation" do + let(:payload) do + update_payload_metadata("dependencies", {"chef-client" => "> 2.0.0", "apt" => dep}) + end - # http://docs.chef.io/config_rb_metadata.html#provides - context "with metadata.providing set to valid value #{valid_provides}" do + let(:error_message) { "Invalid value '#{dep}' for metadata.dependencies" } - let(:payload) { update_payload_metadata("providing", valid_provides) } + include_examples("invalid_cookbook_artifact") + end + end - include_examples("valid_cookbook_artifact") + [ + 'cats::sleep', + 'here(:kitty, :time_to_eat)', + 'service[snuggle]', + '', + 1, + true, + ['cats', 'sleep', 'here'], - end - end - end # context checking metadata sections + { 'cats::sleep' => '0.0.1', + 'here(:kitty, :time_to_eat)' => '0.0.1', + 'service[snuggle]' => '0.0.1' } + ].each do |valid_provides| - context 'with invalid cookbook artifact identifier in url' do + # http://docs.chef.io/config_rb_metadata.html#provides + context "with metadata.providing set to valid value #{valid_provides}" do - let(:cookbook_identifier) { "foo@bar" } - let(:payload){ new_cookbook_artifact(cookbook_name, cookbook_identifier)} + let(:payload) { update_payload_metadata("providing", valid_provides) } - it "should respond with an error" do - put(request_url, requestor, :payload => payload) do |response| - expect(response.code).to eq(400) - expect(parse(response)).to eq({"error" => ["Field 'identifier' invalid"]}) + include_examples("valid_cookbook_artifact") + + end end - end # it invalid version in URL is a 400 - end # with invalid version in url + end # context checking metadata sections - context "when the cookbook name is invalid" do + context 'with invalid cookbook artifact identifier in url' do - let(:cookbook_name) { "first@second" } - let(:payload){ new_cookbook_artifact(cookbook_name, default_cookbook_id)} + let(:cookbook_identifier) { "foo@bar" } + let(:payload){ new_cookbook_artifact(cookbook_name, cookbook_identifier)} - it "responds with a 400" do - put(request_url, requestor, :payload => payload) do |response| - expect(response).to look_like({ - :status => 400, - :body => { - "error" => ["Field 'name' invalid"] - } - }) + it "should respond with an error" do + put(request_url, requestor, :payload => payload) do |response| + expect(response.code).to eq(400) + expect(parse(response)).to eq({"error" => ["Field 'identifier' invalid"]}) + end + end # it invalid version in URL is a 400 + end # with invalid version in url + + context "when the cookbook name is invalid" do + + let(:cookbook_name) { "first@second" } + let(:payload){ new_cookbook_artifact(cookbook_name, default_cookbook_id)} + + it "responds with a 400" do + put(request_url, requestor, :payload => payload) do |response| + expect(response).to look_like({ + :status => 400, + :body => { + "error" => ["Field 'name' invalid"] + } + }) + end end end - end - context "when the identifier in the URL doesn't match the payload" do - let(:payload) do - new_cookbook_artifact(cookbook_name, "ffffffffffffffffffffffffffffffffffffffff") - end + context "when the identifier in the URL doesn't match the payload" do + let(:payload) do + new_cookbook_artifact(cookbook_name, "ffffffffffffffffffffffffffffffffffffffff") + end - let(:request_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{default_cookbook_id}") } + let(:request_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{default_cookbook_id}") } + + it "responds with a 400" do + put(request_url, requestor, :payload => payload) do |response| + error = "Field 'identifier' invalid : 1111111111111111111111111111111111111111 does not match ffffffffffffffffffffffffffffffffffffffff" + response.should look_like({ + :status => 400, + :body => { + "error" => [error] + } + }) + end + end # it mismatched metadata.cookbook_version is a 400 + end - it "responds with a 400" do - put(request_url, requestor, :payload => payload) do |response| - error = "Field 'identifier' invalid : 1111111111111111111111111111111111111111 does not match ffffffffffffffffffffffffffffffffffffffff" - response.should look_like({ - :status => 400, - :body => { - "error" => [error] - } - }) + context "when the cookbook name in the URL doesn't match the payload" do + let(:payload) { new_cookbook_artifact("foobar", cookbook_version) } + let(:request_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}") } + + it "mismatched cookbook_name is a 400" do + put(request_url, requestor, :payload => payload) do |response| + error = "Field 'name' invalid : cookbook_name does not match foobar" + expect(response).to look_like({ + :status => 400, + :body => { + "error" => [error] + } + }) + end end - end # it mismatched metadata.cookbook_version is a 400 - end + end - context "when the cookbook name in the URL doesn't match the payload" do - let(:payload) { new_cookbook_artifact("foobar", cookbook_version) } - let(:request_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}") } + context "when uploading a cookbook artifact with a missing checksum" do + after(:each) do + delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) + end - it "mismatched cookbook_name is a 400" do - put(request_url, requestor, :payload => payload) do |response| - error = "Field 'name' invalid : cookbook_name does not match foobar" - expect(response).to look_like({ - :status => 400, - :body => { - "error" => [error] - } - }) + let(:payload) do + new_cookbook_artifact(cookbook_name, default_cookbook_id).tap do |p| + p["recipes"] = [ + { + "name" => "default.rb", + "path" => "recipes/default.rb", + "checksum" => "8288b67da0793b5abec709d6226e6b73", + "specificity" => "default" + } + ] + end end - end - end - context "when uploading a cookbook artifact with a missing checksum" do - after(:each) do + it "specifying file not in sandbox is a 400" do + put(request_url, requestor, :payload => payload) do |response| + error = "Manifest has a checksum that hasn't been uploaded." + expect(response).to look_like({ + :status => 400, + :body => { + "error" => [error] + } + }) + end + end # it specifying file not in sandbox is a 400 + end # context sandbox checks + end # context creating broken cookbook_artifacts to test validation and defaults + + context "creating good cookbook_artifacts to test defaults" do + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_version) { "1.2.3" } + + let(:cookbook_identifier) { default_cookbook_id } + + let(:description) { "my cookbook" } + let(:long_description) { "this is a great cookbook" } + let(:maintainer) { "This is my name" } + let(:maintainer_email) { "cookbook_author@example.com" } + let(:license) { "MPL" } + + let (:opts) { + { + :version => cookbook_version, + :description => description, + :long_description => long_description, + :maintainer => maintainer, + :maintainer_email => maintainer_email, + :license => license + } + } + + after :each do delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) end + respects_maximum_payload_size + let(:payload) do - new_cookbook_artifact(cookbook_name, default_cookbook_id).tap do |p| - p["recipes"] = [ - { - "name" => "default.rb", - "path" => "recipes/default.rb", - "checksum" => "8288b67da0793b5abec709d6226e6b73", - "specificity" => "default" - } - ] + new_cookbook_artifact(cookbook_name, cookbook_identifier, opts) + end + + let(:expected_get_response_data) do + payload.dup.tap do |artifact| + artifact.delete("json_class") end end - it "specifying file not in sandbox is a 400" do + it "allows override of defaults" do put(request_url, requestor, :payload => payload) do |response| - error = "Manifest has a checksum that hasn't been uploaded." + expect(response.code).to eq(201) + end + get(request_url, requestor) do |response| expect(response).to look_like({ - :status => 400, - :body => { - "error" => [error] - } + :status => 200, + :body => expected_get_response_data }) end - end # it specifying file not in sandbox is a 400 - end # context sandbox checks - end # context creating broken cookbook_artifacts to test validation and defaults - - context "creating good cookbook_artifacts to test defaults" do - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_version) { "1.2.3" } - - let(:cookbook_identifier) { default_cookbook_id } - - let(:description) { "my cookbook" } - let(:long_description) { "this is a great cookbook" } - let(:maintainer) { "This is my name" } - let(:maintainer_email) { "cookbook_author@example.com" } - let(:license) { "MPL" } - - let (:opts) { - { - :version => cookbook_version, - :description => description, - :long_description => long_description, - :maintainer => maintainer, - :maintainer_email => maintainer_email, - :license => license - } - } - - after :each do - delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) - end + end # it allows override of defaults + end # context creating good gookbooks to test defaults + end # context PUT /cookbook_artifacts// [create] - respects_maximum_payload_size + context "PUT multiple cookbook_artifacts" do + let(:cookbook_name) { "multiple_versions" } + let(:cookbook_id_1) { "1111111111111111111111111111111111111111" } + let(:cookbook_id_2) { "2222222222222222222222222222222222222222" } - let(:payload) do - new_cookbook_artifact(cookbook_name, cookbook_identifier, opts) + let(:cookbook_1_payload) do + new_cookbook_artifact(cookbook_name, cookbook_id_1, {}) end - let(:expected_get_response_data) do - payload.dup.tap do |artifact| - artifact.delete("json_class") - end + let(:cookbook_1_get_response) do + cookbook_1_payload.dup.tap { |c| c.delete("json_class") } end - it "allows override of defaults" do - put(request_url, requestor, :payload => payload) do |response| - expect(response.code).to eq(201) - end - get(request_url, requestor) do |response| - expect(response).to look_like({ - :status => 200, - :body => expected_get_response_data - }) - end - end # it allows override of defaults - end # context creating good gookbooks to test defaults - end # context PUT /cookbook_artifacts// [create] - - context "PUT multiple cookbook_artifacts" do - let(:cookbook_name) { "multiple_versions" } - let(:cookbook_id_1) { "1111111111111111111111111111111111111111" } - let(:cookbook_id_2) { "2222222222222222222222222222222222222222" } - - let(:cookbook_1_payload) do - new_cookbook_artifact(cookbook_name, cookbook_id_1, {}) - end - - let(:cookbook_1_get_response) do - cookbook_1_payload.dup.tap { |c| c.delete("json_class") } - end - - let(:cookbook_2_payload) do - new_cookbook_artifact(cookbook_name, cookbook_id_2, {}) - end - - let(:cookbook_2_get_response) do - cookbook_2_payload.dup.tap { |c| c.delete("json_class") } - end - - after :each do - [cookbook_id_1, cookbook_id_2].each do |v| - delete(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{v}"), admin_user) + let(:cookbook_2_payload) do + new_cookbook_artifact(cookbook_name, cookbook_id_2, {}) end - end - it "allows us to create 2 revisions of the same cookbook" do - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_1}"), - admin_user, - :payload => cookbook_1_payload) do |response| - expect(response.code).to eq(201) + let(:cookbook_2_get_response) do + cookbook_2_payload.dup.tap { |c| c.delete("json_class") } end - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_2}"), - admin_user, - :payload => cookbook_2_payload) do |response| - expect(response.code).to eq(201) + after :each do + [cookbook_id_1, cookbook_id_2].each do |v| + delete(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{v}"), admin_user) + end end - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_1}"), - admin_user) do |response| - response.should look_like({ - :status => 200, - :body => cookbook_1_get_response - }) - end + it "allows us to create 2 revisions of the same cookbook" do + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_1}"), + admin_user, + :payload => cookbook_1_payload) do |response| + expect(response.code).to eq(201) + end + + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_2}"), + admin_user, + :payload => cookbook_2_payload) do |response| + expect(response.code).to eq(201) + end + + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_1}"), + admin_user) do |response| + response.should look_like({ + :status => 200, + :body => cookbook_1_get_response + }) + end + + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_2}"), + admin_user) do |response| + response.should look_like({ + :status => 200, + :body => cookbook_2_get_response + }) + end + end # it allows us to create 2 versions of the same cookbook + end # context PUT multiple cookbook_artifacts + end + + describe "API v0" do + it_behaves_like "creates cookbook artifacts", 0 + end + + describe "API v2" do + it_behaves_like "creates cookbook artifacts", 2 + end - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_id_2}"), - admin_user) do |response| - response.should look_like({ - :status => 200, - :body => cookbook_2_get_response - }) - end - end # it allows us to create 2 versions of the same cookbook - end # context PUT multiple cookbook_artifacts end # describe cookbook_artifacts API endpoint diff --git a/oc-chef-pedant/spec/api/cookbook_artifacts/delete_spec.rb b/oc-chef-pedant/spec/api/cookbook_artifacts/delete_spec.rb index 4ec58d11ee2..bc3624e137e 100644 --- a/oc-chef-pedant/spec/api/cookbook_artifacts/delete_spec.rb +++ b/oc-chef-pedant/spec/api/cookbook_artifacts/delete_spec.rb @@ -17,33 +17,42 @@ describe "Cookbook Artifacts API endpoint", :cookbook_artifacts, :cookbook_artifacts_delete do - let(:cookbook_url_base) { "cookbook_artifacts" } - - include Pedant::RSpec::CookbookUtil - - context "DELETE /cookbook_artifacts//" do - let(:request_method) { :DELETE } - let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} - let(:requestor) { admin_user } + shared_examples "deletes cookbook artifacts" do |api_version| + let(:api_version) { api_version } + + before do + platform.server_api_version = api_version + end + + after do + platform.reset_server_api_version + end + + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" + end + end - let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } - let(:default_version) { "1.2.3" } + let(:cookbook_url_base) { "cookbook_artifacts" } - context "for non-existent cookbooks" do - let(:expected_response) { cookbook_version_not_found_exact_response } + include Pedant::RSpec::CookbookUtil - let(:cookbook_name) { "non_existent" } - let(:cookbook_version) { "1.2.3" } + context "DELETE /cookbook_artifacts//" do + let(:request_method) { :DELETE } + let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} + let(:requestor) { admin_user } - it "returns 404" do - response = delete(request_url, requestor) - expect(response.code).to eq(404) - expect(parse(response)).to eq({"error"=>["not_found"]}) - end + let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } + let(:default_version) { "1.2.3" } - context 'with bad identifier' do + context "for non-existent cookbooks" do + let(:expected_response) { cookbook_version_not_found_exact_response } - let(:cookbook_identifier) { "foo@bar" } + let(:cookbook_name) { "non_existent" } + let(:cookbook_version) { "1.2.3" } it "returns 404" do response = delete(request_url, requestor) @@ -51,123 +60,144 @@ expect(parse(response)).to eq({"error"=>["not_found"]}) end - end # with bad version - end # context for non-existent cookbooks - - context "for existing cookbooks" do - - let(:cookbook_name) { "cookbook-to-be-deleted" } - - context "when deleting non-existent version of an existing cookbook" do - let(:non_existing_identifier) { "ffffffffffffffffffffffffffffffffffffffff" } - let(:non_existing_version_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{non_existing_identifier}") } + context 'with bad identifier' do - before(:each) { make_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } - after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_identifier) } + let(:cookbook_identifier) { "foo@bar" } - it "should respond with 404 (\"Not Found\") and not delete existing versions" do - delete(non_existing_version_url, requestor) do |response| + it "returns 404" do + response = delete(request_url, requestor) expect(response.code).to eq(404) expect(parse(response)).to eq({"error"=>["not_found"]}) end - expect(get(request_url, requestor).code).to eq(200) - end - end # it doesn't delete the wrong version of an existing cookbook + end # with bad version + end # context for non-existent cookbooks + + context "for existing cookbooks" do + + let(:cookbook_name) { "cookbook-to-be-deleted" } + + context "when deleting non-existent version of an existing cookbook" do + let(:non_existing_identifier) { "ffffffffffffffffffffffffffffffffffffffff" } + let(:non_existing_version_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{non_existing_identifier}") } + + before(:each) { make_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } + after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_identifier) } + + it "should respond with 404 (\"Not Found\") and not delete existing versions" do + delete(non_existing_version_url, requestor) do |response| + expect(response.code).to eq(404) + expect(parse(response)).to eq({"error"=>["not_found"]}) + end - context "when deleting existent version of an existing cookbook", :smoke do + expect(get(request_url, requestor).code).to eq(200) + end + end # it doesn't delete the wrong version of an existing cookbook + + context "when deleting existent version of an existing cookbook", :smoke do - let(:recipe_name) { "test_recipe" } - let(:recipe_content) { "hello-#{unique_suffix}" } - let(:recipe_spec) do + let(:recipe_name) { "test_recipe" } + let(:recipe_content) { "hello-#{unique_suffix}" } + let(:recipe_spec) do { :name => recipe_name, :content => recipe_content } - end + end - before(:each) do - make_cookbook_artifact_with_recipes(cookbook_name, cookbook_identifier, [recipe_spec]) - end + before(:each) do + make_cookbook_artifact_with_recipes(cookbook_name, cookbook_identifier, [recipe_spec]) + end - after(:each) { delete_cookbook_artifact(requestor, cookbook_name, cookbook_identifier) } + after(:each) { delete_cookbook_artifact(requestor, cookbook_name, cookbook_identifier) } - it "should cleanup unused checksum data in s3/bookshelf" do - artifact_json = get(request_url, requestor) - expect(artifact_json.code).to eq(200) - artifact_before_delete = parse(artifact_json) - existing_recipes = artifact_before_delete["recipes"] + it "should cleanup unused checksum data in s3/bookshelf" do + artifact_json = get(request_url, requestor) + expect(artifact_json.code).to eq(200) + artifact_before_delete = parse(artifact_json) + existing_recipes = extract_segment(artifact_before_delete, "recipes") - expect(existing_recipes.size).to eq(1) - remote_recipe_spec = existing_recipes.first - expect(remote_recipe_spec).to be_a_kind_of(Hash) + expect(existing_recipes.size).to eq(1) + remote_recipe_spec = existing_recipes.first + expect(remote_recipe_spec).to be_a_kind_of(Hash) - expect(remote_recipe_spec["name"]).to eq("test_recipe.rb") - expect(remote_recipe_spec["path"]).to eq("recipes/test_recipe.rb") - expect(remote_recipe_spec["checksum"]).to be_a_kind_of(String) - expect(remote_recipe_spec["specificity"]).to eq("default") - expect(remote_recipe_spec["url"]).to be_a_kind_of(String) + rn = platform.server_api_version >= 2 ? "recipes/#{recipe_name}.rb" : "#{recipe_name}.rb" + expect(remote_recipe_spec["name"]).to eq(rn) + expect(remote_recipe_spec["path"]).to eq("recipes/test_recipe.rb") + expect(remote_recipe_spec["checksum"]).to be_a_kind_of(String) + expect(remote_recipe_spec["specificity"]).to eq("default") + expect(remote_recipe_spec["url"]).to be_a_kind_of(String) - delete_response = delete(request_url, requestor) - expect(delete_response.code).to eq(200) + delete_response = delete(request_url, requestor) + expect(delete_response.code).to eq(200) - verify_checksum_url(remote_recipe_spec["url"], 404) - end + verify_checksum_url(remote_recipe_spec["url"], 404) + end - end # context when deleting existent version... - end # context for existing cookbooks + end # context when deleting existent version... + end # context for existing cookbooks - context "with permissions for" do - let(:cookbook_name) {"delete-cookbook"} - let(:cookbook_identifier) { "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" } - let(:not_found_msg) { ["Cannot find a cookbook named delete-cookbook with version 0.0.1"] } + context "with permissions for" do + let(:cookbook_name) {"delete-cookbook"} + let(:cookbook_identifier) { "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" } + let(:not_found_msg) { ["Cannot find a cookbook named delete-cookbook with version 0.0.1"] } - let(:original_cookbook) { new_cookbook_artifact(cookbook_name, cookbook_identifier) } - let(:fetched_cookbook) { original_cookbook.dup.tap { |c| c.delete("json_class") } } + let(:original_cookbook) { new_cookbook_artifact(cookbook_name, cookbook_identifier) } + let(:fetched_cookbook) { original_cookbook.dup.tap { |c| c.delete("json_class") } } - before(:each) { make_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } - after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } + before(:each) { make_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } + after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } - context 'as admin user' do + context 'as admin user' do - it "should respond with 200 (\"OK\") and be deleted" do - delete_response = delete(request_url, admin_user) - expect(delete_response.code).to eq(200) - expect(parse(delete_response)).to eq(fetched_cookbook) + it "should respond with 200 (\"OK\") and be deleted" do + delete_response = delete(request_url, admin_user) + expect(delete_response.code).to eq(200) + expect(parse(delete_response)).to eq(fetched_cookbook) - expect(get(request_url, admin_user).code).to eq(404) - end # it admin user returns 200 - end # as admin user + expect(get(request_url, admin_user).code).to eq(404) + end # it admin user returns 200 + end # as admin user - context 'as normal user', :authorization do - let(:expected_response) { delete_cookbook_success_response } + context 'as normal user', :authorization do + let(:expected_response) { delete_cookbook_success_response } - let(:requestor) { normal_user } - it "should respond with 200 (\"OK\") and be deleted" do - expect(delete(request_url, requestor).code).to eq(200) - end # it admin user returns 200 - end # with normal user + let(:requestor) { normal_user } + it "should respond with 200 (\"OK\") and be deleted" do + expect(delete(request_url, requestor).code).to eq(200) + end # it admin user returns 200 + end # with normal user - context 'as a user outside of the organization', :authorization do - let(:expected_response) { unauthorized_access_credential_response } - let(:requestor) { outside_user } + context 'as a user outside of the organization', :authorization do + let(:expected_response) { unauthorized_access_credential_response } + let(:requestor) { outside_user } - it "should respond with 403 (\"Forbidden\") and does not delete cookbook" do - response.should look_like expected_response - should_not_be_deleted - end - end # it outside user returns 403 + it "should respond with 403 (\"Forbidden\") and does not delete cookbook" do + response.should look_like expected_response + should_not_be_deleted + end + end # it outside user returns 403 + + context 'with invalid user', :authorization do + let(:expected_response) { invalid_credential_exact_response } + let(:requestor) { invalid_user } + + it "should respond with 401 (\"Unauthorized\") and does not delete cookbook" do + response.should look_like expected_response + should_not_be_deleted + end # responds with 401 + end # with invalid user + + end # context with permissions for + end # context DELETE /cookbook_artifacts// + end - context 'with invalid user', :authorization do - let(:expected_response) { invalid_credential_exact_response } - let(:requestor) { invalid_user } + describe "API v0" do + it_behaves_like "deletes cookbook artifacts", 0 + end - it "should respond with 401 (\"Unauthorized\") and does not delete cookbook" do - response.should look_like expected_response - should_not_be_deleted - end # responds with 401 - end # with invalid user + describe "API v2" do + it_behaves_like "deletes cookbook artifacts", 2 + end - end # context with permissions for - end # context DELETE /cookbook_artifacts// -end +end # describe cookbook_artifacts API endpoint diff --git a/oc-chef-pedant/spec/api/cookbook_artifacts/read_spec.rb b/oc-chef-pedant/spec/api/cookbook_artifacts/read_spec.rb index 850f329f584..3d3414e286f 100644 --- a/oc-chef-pedant/spec/api/cookbook_artifacts/read_spec.rb +++ b/oc-chef-pedant/spec/api/cookbook_artifacts/read_spec.rb @@ -22,208 +22,240 @@ describe "Cookbook Artifacts API endpoint", :cookbook_artifacts, :cookbook_artifacts_read do - let(:cookbook_url_base) { "cookbook_artifacts" } + shared_examples "reads cookbook artifacts" do |api_version| + let(:api_version) { api_version } - include Pedant::RSpec::CookbookUtil + before do + platform.server_api_version = api_version + end - def cookbook_artifact_version_url(name, identifier) - api_url("/#{cookbook_url_base}/#{name}/#{identifier}") - end + after do + platform.reset_server_api_version + end + + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" + end + end - context "GET /cookbook_artifacts" do - let(:request_url){api_url("/#{cookbook_url_base}/")} - let(:requestor) { admin_user } + let(:cookbook_url_base) { "cookbook_artifacts" } + + include Pedant::RSpec::CookbookUtil + + def cookbook_artifact_version_url(name, identifier) + api_url("/#{cookbook_url_base}/#{name}/#{identifier}") + end + + context "GET /cookbook_artifacts" do + let(:request_url){api_url("/#{cookbook_url_base}/")} + let(:requestor) { admin_user } - context "with no cookbook artifacts on the server", :smoke do - it "responds with 200" do - get(request_url, requestor) do |response| - expect(response.code).to eq(200) - expect(parse(response)).to eq({}) + context "with no cookbook artifacts on the server", :smoke do + it "responds with 200" do + get(request_url, requestor) do |response| + expect(response.code).to eq(200) + expect(parse(response)).to eq({}) + end end end - end - context "with existing cookbook_artifacts and multiple versions" do + context "with existing cookbook_artifacts and multiple versions" do - let(:request_url) { api_url("/#{cookbook_url_base}") } + let(:request_url) { api_url("/#{cookbook_url_base}") } - let(:fetched_cookbook_artifacts) { cookbook_collection } + let(:fetched_cookbook_artifacts) { cookbook_collection } - let(:default_version) { "1.0.0" } + let(:default_version) { "1.0.0" } - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_name2) { "cookbook_name2" } - let(:identifier_1) { "1111111111111111111111111111111111111111" } - let(:identifier_2) { "2222222222222222222222222222222222222222" } - let(:identifier_3) { "3333333333333333333333333333333333333333" } + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_name2) { "cookbook_name2" } + let(:identifier_1) { "1111111111111111111111111111111111111111" } + let(:identifier_2) { "2222222222222222222222222222222222222222" } + let(:identifier_3) { "3333333333333333333333333333333333333333" } - let(:cba_1_url) { request_url + "/#{cookbook_name}/#{identifier_1}" } - let(:cba_2_url) { request_url + "/#{cookbook_name}/#{identifier_2}" } - let(:cba_3_url) { request_url + "/#{cookbook_name2}/#{identifier_3}" } + let(:cba_1_url) { request_url + "/#{cookbook_name}/#{identifier_1}" } + let(:cba_2_url) { request_url + "/#{cookbook_name}/#{identifier_2}" } + let(:cba_3_url) { request_url + "/#{cookbook_name2}/#{identifier_3}" } - let(:cba_1) { new_cookbook_artifact(cookbook_name, identifier_1) } - let(:cba_2) { new_cookbook_artifact(cookbook_name, identifier_2) } - let(:cba_3) { new_cookbook_artifact(cookbook_name2, identifier_3) } + let(:cba_1) { new_cookbook_artifact(cookbook_name, identifier_1) } + let(:cba_2) { new_cookbook_artifact(cookbook_name, identifier_2) } + let(:cba_3) { new_cookbook_artifact(cookbook_name2, identifier_3) } - before(:each) do - r1 = put(cba_1_url, admin_user, payload: cba_1) - expect(r1.code).to eq(201) + before(:each) do + r1 = put(cba_1_url, admin_user, payload: cba_1) + expect(r1.code).to eq(201) - r2 = put(cba_2_url, admin_user, payload: cba_2) - expect(r2.code).to eq(201) + r2 = put(cba_2_url, admin_user, payload: cba_2) + expect(r2.code).to eq(201) - r3 = put(cba_3_url, admin_user, payload: cba_3) - expect(r3.code).to eq(201) - end + r3 = put(cba_3_url, admin_user, payload: cba_3) + expect(r3.code).to eq(201) + end - after(:each) do - r1 = delete(cba_1_url, admin_user) - expect(r1.code).to eq(200) + after(:each) do + r1 = delete(cba_1_url, admin_user) + expect(r1.code).to eq(200) - r2 = delete(cba_2_url, admin_user) - expect(r2.code).to eq(200) + r2 = delete(cba_2_url, admin_user) + expect(r2.code).to eq(200) - r3 = delete(cba_3_url, admin_user) - expect(r3.code).to eq(200) - end + r3 = delete(cba_3_url, admin_user) + expect(r3.code).to eq(200) + end - let(:expected_cookbook_artifact_collection) do - { - cookbook_name => { - "url" => cookbook_url(cookbook_name), - "versions" => [ - { "identifier" => identifier_1, - "url" => cookbook_artifact_version_url(cookbook_name, identifier_1) }, + let(:expected_cookbook_artifact_collection) do + { + cookbook_name => { + "url" => cookbook_url(cookbook_name), + "versions" => [ + { "identifier" => identifier_1, + "url" => cookbook_artifact_version_url(cookbook_name, identifier_1) }, { "identifier" => identifier_2, "url" => cookbook_artifact_version_url(cookbook_name, identifier_2) } - ]}, - cookbook_name2 => { - "url" => cookbook_url(cookbook_name2), - "versions" => [ - { "identifier" => identifier_3, - "url" => cookbook_artifact_version_url(cookbook_name2, identifier_3) }]} - } - end + ]}, + cookbook_name2 => { + "url" => cookbook_url(cookbook_name2), + "versions" => [ + { "identifier" => identifier_3, + "url" => cookbook_artifact_version_url(cookbook_name2, identifier_3) }]} + } + end - it 'should respond with a cookbook collection containing all versions of each cookbook' do - list_response = get(request_url, requestor) - expect(list_response.code).to eq(200) - expect(parse(list_response)).to strictly_match(expected_cookbook_artifact_collection) - end + it 'should respond with a cookbook collection containing all versions of each cookbook' do + list_response = get(request_url, requestor) + expect(list_response.code).to eq(200) + expect(parse(list_response)).to strictly_match(expected_cookbook_artifact_collection) + end - end + end - end # context GET /cookbook_artifacts + end # context GET /cookbook_artifacts - context "GET /cookbook_artifacts//" do + context "GET /cookbook_artifacts//" do - let(:cookbook_name) { "the_cookbook_name" } - let(:default_version) { "1.2.3" } - let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } + let(:cookbook_name) { "the_cookbook_name" } + let(:default_version) { "1.2.3" } + let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } - let(:request_url) { cookbook_artifact_version_url(cookbook_name, cookbook_identifier) } + let(:request_url) { cookbook_artifact_version_url(cookbook_name, cookbook_identifier) } - let(:recipe_name) { "test_recipe" } - let(:recipe_content) { "hello-#{unique_suffix}" } + let(:recipe_name) { "test_recipe" } + let(:recipe_content) { "hello-#{unique_suffix}" } - let(:recipe_spec) do + let(:recipe_spec) do { :name => recipe_name, :content => recipe_content } - end + end - before(:each) do - make_cookbook_artifact_with_recipes(cookbook_name, cookbook_identifier, [recipe_spec]) - end + before(:each) do + make_cookbook_artifact_with_recipes(cookbook_name, cookbook_identifier, [recipe_spec]) + end - after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } - - shared_examples_for "successful_cookbook_fetch" do - - let(:expected_cookbook_artifact_data) do - new_cookbook_artifact(cookbook_name, cookbook_identifier).tap do |cba| - cba.delete("json_class") - - # checksum and url are also present in the real data but they're not - # stable so we remove them before comparing - cba["recipes"] = [ - { - "name" => "#{recipe_name}.rb", - "path" => "recipes/#{recipe_name}.rb", - "specificity" => "default" - } - ] - cba["metadata"]["providing"] = { "#{cookbook_name}::#{recipe_name}"=>">= 0.0.0" } - cba["metadata"]["recipes"] = { "#{cookbook_name}::#{recipe_name}"=>"" } + after(:each) { delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) } + + shared_examples_for "successful_cookbook_fetch" do + + let(:expected_cookbook_artifact_data) do + new_cookbook_artifact(cookbook_name, cookbook_identifier).tap do |cba| + cba.delete("json_class") + + # checksum and url are also present in the real data but they're not + # stable so we remove them before comparing + target = platform.server_api_version >= 2 ? "all_files" : "recipes" + rn = platform.server_api_version >= 2 ? "recipes/#{recipe_name}.rb" : "#{recipe_name}.rb" + cba[target] = [ + { + "name" => rn, + "path" => "recipes/#{recipe_name}.rb", + "specificity" => "default" + } + ] + cba["metadata"]["providing"] = { "#{cookbook_name}::#{recipe_name}"=>">= 0.0.0" } + cba["metadata"]["recipes"] = { "#{cookbook_name}::#{recipe_name}"=>"" } + end end - end - it "returns a 200 response" do - get_response = get(request_url, requestor) - expect(get_response.code).to eq(200) - response_data = parse(get_response) - expect(response_data).to have_key("recipes") - expect(response_data["recipes"]).to be_a_kind_of(Array) - expect(response_data["recipes"].size).to eq(1) - expect(response_data["recipes"].first.keys).to match_array(%w[ name path checksum specificity url ]) + it "returns a 200 response" do + get_response = get(request_url, requestor) + expect(get_response.code).to eq(200) + response_data = parse(get_response) + recipes = extract_segment(response_data, "recipes") + expect(recipes).to be_a_kind_of(Array) + expect(recipes.size).to eq(1) + expect(recipes.first.keys).to match_array(%w[ name path checksum specificity url ]) + + # URL and checksum are not predictable. + target = platform.server_api_version >= 2 ? "all_files" : "recipes" + response_data[target].first.delete("url") + response_data[target].first.delete("checksum") + + expect(response_data).to eq(expected_cookbook_artifact_data) + end - # URL and checksum are not predictable. - response_data["recipes"].first.delete("url") - response_data["recipes"].first.delete("checksum") + # These tests verify that the pre-signed URL that comes back + # within cookbook_version responses is usable. Validating both + # the URL generation, but also the use of bookshelf via the + # load balancer. + it "returns valid file URLs" do + cookbook_artifact_data = parse(get(request_url, requestor)) + recipe_url = extract_segment(cookbook_artifact_data, "recipes").first["url"] + + uri = URI.parse(recipe_url) + http = Net::HTTP.new(uri.hostname, uri.port) + if uri.scheme == "https" + http.use_ssl = true + http.ssl_version = Pedant::Config.ssl_version + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + end + + response = http.get(uri.request_uri, {}) + response.body.should == recipe_content + end - expect(response_data).to eq(expected_cookbook_artifact_data) - end + end # as a normal user - # These tests verify that the pre-signed URL that comes back - # within cookbook_version responses is usable. Validating both - # the URL generation, but also the use of bookshelf via the - # load balancer. - it "returns valid file URLs" do - cookbook_artifact_data = parse(get(request_url, requestor)) - recipe_url = cookbook_artifact_data["recipes"].first["url"] - - uri = URI.parse(recipe_url) - http = Net::HTTP.new(uri.hostname, uri.port) - if uri.scheme == "https" - http.use_ssl = true - http.ssl_version = Pedant::Config.ssl_version - http.verify_mode = OpenSSL::SSL::VERIFY_NONE - end + context 'as a normal user' do + let(:requestor) { normal_user } - response = http.get(uri.request_uri, {}) - response.body.should == recipe_content + include_examples("successful_cookbook_fetch") end - end # as a normal user + context 'as an admin user' do + let(:requestor) { admin_user } - context 'as a normal user' do - let(:requestor) { normal_user } + include_examples("successful_cookbook_fetch") + end # as an admin user - include_examples("successful_cookbook_fetch") - end + context 'as an user outside of the organization', :authorization do + let(:request_method) { :GET } + let(:expected_response) { unauthorized_access_credential_response } + let(:requestor) { outside_user } - context 'as an admin user' do - let(:requestor) { admin_user } + should_respond_with 403 + end # as an outside user - include_examples("successful_cookbook_fetch") - end # as an admin user + context 'with invalid user', :authentication do + let(:request_method) { :GET } + let(:expected_response) { invalid_credential_exact_response } + let(:requestor) { invalid_user } - context 'as an user outside of the organization', :authorization do - let(:request_method) { :GET } - let(:expected_response) { unauthorized_access_credential_response } - let(:requestor) { outside_user } + should_respond_with 401 + end + end # context GET /cookbook_artifacts// + end - should_respond_with 403 - end # as an outside user + describe "API v0" do + it_behaves_like "reads cookbook artifacts", 0 + end - context 'with invalid user', :authentication do - let(:request_method) { :GET } - let(:expected_response) { invalid_credential_exact_response } - let(:requestor) { invalid_user } + describe "API v2" do + it_behaves_like "reads cookbook artifacts", 2 + end - should_respond_with 401 - end - end # context GET /cookbook_artifacts// end # describe cookbook_artifacts API endpoint diff --git a/oc-chef-pedant/spec/api/cookbook_artifacts/update_spec.rb b/oc-chef-pedant/spec/api/cookbook_artifacts/update_spec.rb index 6c8970eab4c..78d4d98e0b2 100644 --- a/oc-chef-pedant/spec/api/cookbook_artifacts/update_spec.rb +++ b/oc-chef-pedant/spec/api/cookbook_artifacts/update_spec.rb @@ -28,87 +28,70 @@ describe "Cookbook Artifacts API endpoint", :cookbook_artifacts, :cookbook_artifacts_update do - let(:cookbook_url_base) { "cookbook_artifacts" } + shared_examples "updates cookbook artifacts" do |api_version| + let(:api_version) { api_version } - include Pedant::RSpec::CookbookUtil - - context "PUT /cookbook_artifacts// [update]" do + before do + platform.server_api_version = api_version + end - # for respects_maximum_payload_size - let(:request_method) { :PUT } - let(:requestor){admin_user} - let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} - let(:cookbook_name) { "cookbook-to-be-modified" } - let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } - let(:default_version) { "1.2.3" } + after do + platform.reset_server_api_version + end - let(:updated_cookbook_artifact) do - new_cookbook_artifact(cookbook_name, cookbook_identifier).tap do |payload| - payload["metadata"]["description"] = "hi there" + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" end end - before(:each) { - make_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) - } + let(:cookbook_url_base) { "cookbook_artifacts" } - after(:each) { - delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) - } + include Pedant::RSpec::CookbookUtil - respects_maximum_payload_size + context "PUT /cookbook_artifacts// [update]" do + # for respects_maximum_payload_size + let(:request_method) { :PUT } + let(:requestor){admin_user} + let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_identifier}")} + let(:cookbook_name) { "cookbook-to-be-modified" } + let(:cookbook_identifier) { "1111111111111111111111111111111111111111" } + let(:default_version) { "1.2.3" } - context "as admin user" do - - let(:fetched_cookbook_artifact) do - new_cookbook_artifact(cookbook_name, cookbook_identifier).tap do |c| - c.delete("json_class") + let(:updated_cookbook_artifact) do + new_cookbook_artifact(cookbook_name, cookbook_identifier).tap do |payload| + payload["metadata"]["description"] = "hi there" end end - it "should respond with 409 Conflict", :smoke do - put(request_url, admin_user, :payload => updated_cookbook_artifact) do |response| - expect(response).to look_like({ - :status => 409, - :body_exact => {"error"=>"Cookbook artifact already exists"} - }) - end + before(:each) { + make_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) + } - # verify change didn't happen - get(request_url, admin_user) do |response| - expect(response).to look_like({ - :status => 200, - :body => fetched_cookbook_artifact - }) - end - end # it admin user returns 200 + after(:each) { + delete_cookbook_artifact(admin_user, cookbook_name, cookbook_identifier) + } + + respects_maximum_payload_size - context 'as a user outside of the organization', :authorization do - let(:expected_response) { unauthorized_access_credential_response } + context "as admin user" do - it "should respond with 403 (\"Forbidden\") and does not update cookbook" do - put(request_url, outside_user, :payload => updated_cookbook_artifact) do |response| - response.should look_like expected_response + let(:fetched_cookbook_artifact) do + new_cookbook_artifact(cookbook_name, cookbook_identifier).tap do |c| + c.delete("json_class") end + end - # verify change didn't happen - get(request_url, admin_user) do |response| + it "should respond with 409 Conflict", :smoke do + put(request_url, admin_user, :payload => updated_cookbook_artifact) do |response| expect(response).to look_like({ - :status => 200, - :body => fetched_cookbook_artifact + :status => 409, + :body_exact => {"error"=>"Cookbook artifact already exists"} }) end - end # it outside user returns 403 and does not update cookbook - end - - context 'with invalid user', :authentication do - let(:expected_response) { invalid_credential_exact_response } - - it "returns 401 and does not update cookbook" do - put(request_url, invalid_user, :payload => updated_cookbook_artifact) do |response| - response.should look_like expected_response - end # verify change didn't happen get(request_url, admin_user) do |response| @@ -117,9 +100,53 @@ :body => fetched_cookbook_artifact }) end - end # it invalid user returns 401 and does not update cookbook - end # with invalid user - end # context with permissions for - end # context PUT /cookbook_artifacts// [update] -end + end # it admin user returns 200 + + context 'as a user outside of the organization', :authorization do + let(:expected_response) { unauthorized_access_credential_response } + + it "should respond with 403 (\"Forbidden\") and does not update cookbook" do + put(request_url, outside_user, :payload => updated_cookbook_artifact) do |response| + response.should look_like expected_response + end + + # verify change didn't happen + get(request_url, admin_user) do |response| + expect(response).to look_like({ + :status => 200, + :body => fetched_cookbook_artifact + }) + end + end # it outside user returns 403 and does not update cookbook + end + context 'with invalid user', :authentication do + let(:expected_response) { invalid_credential_exact_response } + + it "returns 401 and does not update cookbook" do + put(request_url, invalid_user, :payload => updated_cookbook_artifact) do |response| + response.should look_like expected_response + end + + # verify change didn't happen + get(request_url, admin_user) do |response| + expect(response).to look_like({ + :status => 200, + :body => fetched_cookbook_artifact + }) + end + end # it invalid user returns 401 and does not update cookbook + end # with invalid user + end # context with permissions for + end # context PUT /cookbook_artifacts// [update] + end + + describe "API v0" do + it_behaves_like "updates cookbook artifacts", 0 + end + + describe "API v2" do + it_behaves_like "updates cookbook artifacts", 2 + end + +end # describe cookbook_artifacts API endpoint diff --git a/oc-chef-pedant/spec/api/cookbooks/create_spec.rb b/oc-chef-pedant/spec/api/cookbooks/create_spec.rb index e8c4360a9de..a2f80d6d34d 100644 --- a/oc-chef-pedant/spec/api/cookbooks/create_spec.rb +++ b/oc-chef-pedant/spec/api/cookbooks/create_spec.rb @@ -19,348 +19,382 @@ describe "Cookbooks API endpoint", :cookbooks, :cookbooks_create do - let(:cookbook_url_base) { "cookbooks" } + shared_examples "creates cookbooks" do |api_version| + let(:cookbook_url_base) { "cookbooks" } + let(:api_version) { api_version } - include Pedant::RSpec::CookbookUtil - - context "PUT /cookbooks// [create]" do - include Pedant::RSpec::Validations::Create - let(:request_method){:PUT} - let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}")} - shared(:requestor){admin_user} - - let(:default_resource_attributes){ new_cookbook(cookbook_name, cookbook_version)} + before do + platform.server_api_version = api_version + end - context 'with a basic cookbook', :smoke do - after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_version) } + after do + platform.reset_server_api_version + end - let(:request_payload) { default_resource_attributes } - let(:cookbook_name) { "pedant_basic" } - let(:cookbook_version) { "1.0.0" } - let(:created_resource) { default_resource_attributes } - it "creates a basic cookbook" do - should look_like created_exact_response + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" end end - # Start using the new validation macros - context "when validating" do + include Pedant::RSpec::CookbookUtil - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_version) { "1.2.3" } + context "PUT /cookbooks// [create]" do + include Pedant::RSpec::Validations::Create + let(:request_method){:PUT} + let(:request_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}")} + shared(:requestor){admin_user} - let(:resource_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}")} - let(:persisted_resource_response){ get(resource_url, requestor) } + let(:default_resource_attributes){ new_cookbook(cookbook_name, cookbook_version)} - after(:each){ delete_cookbook(requestor, cookbook_name, cookbook_version)} + context 'with a basic cookbook', :smoke do + after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_version) } - context "the 'json_class' field" do - let(:validate_attribute){"json_class"} - accepts_valid_value "Chef::CookbookVersion" - rejects_invalid_value "Chef::Node" + let(:request_payload) { default_resource_attributes } + let(:cookbook_name) { "pedant_basic" } + let(:cookbook_version) { "1.0.0" } + let(:created_resource) { default_resource_attributes } + it "creates a basic cookbook" do + should look_like created_exact_response + end end - context "the cookbook version" do - let(:request_payload) { default_resource_attributes } + # Start using the new validation macros + context "when validating" do - context "with negative versions", :validation do - let(:cookbook_version) { "1.2.-42" } - it { should look_like http_400_response } - end + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_version) { "1.2.3" } - context "with versions at exactly 4 bytes" do - int4_exact = "2147483647" - let(:cookbook_version) { "1.2.#{int4_exact}" } - it { should look_like http_201_response } - end + let(:resource_url){api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}")} + let(:persisted_resource_response){ get(resource_url, requestor) } - context "with versions larger than 4 bytes" do - int4_overflow = "2147483669" # max = 2147483647 (add 42) - let(:cookbook_version) { "1.2.#{int4_overflow}" } - it { should look_like http_201_response } - end + after(:each){ delete_cookbook(requestor, cookbook_name, cookbook_version)} - context "with versions larger than 8 bytes", :validation do - int8_overflow = "9223372036854775849" # max = 9223372036854775807 (add 42) - let(:cookbook_version) { "1.2.#{int8_overflow}" } - it { should look_like http_400_response } + context "the 'json_class' field" do + let(:validate_attribute){"json_class"} + accepts_valid_value "Chef::CookbookVersion" + rejects_invalid_value "Chef::Node" end - end - rejects_invalid_keys + context "the cookbook version" do + let(:request_payload) { default_resource_attributes } - end + context "with negative versions", :validation do + let(:cookbook_version) { "1.2.-42" } + it { should look_like http_400_response } + end - context "creating broken cookbooks to test validation and defaults", :validation do - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_version) { "1.2.3" } + context "with versions at exactly 4 bytes" do + int4_exact = "2147483647" + let(:cookbook_version) { "1.2.#{int4_exact}" } + it { should look_like http_201_response } + end - malformed_constraint = "s395dss@#" + context "with versions larger than 4 bytes" do + int4_overflow = "2147483669" # max = 2147483647 (add 42) + let(:cookbook_version) { "1.2.#{int4_overflow}" } + it { should look_like http_201_response } + end - context "basic tests" do - after(:each) do - delete_cookbook(admin_user, cookbook_name, cookbook_version) + context "with versions larger than 8 bytes", :validation do + int8_overflow = "9223372036854775849" # max = 9223372036854775807 (add 42) + let(:cookbook_version) { "1.2.#{int8_overflow}" } + it { should look_like http_400_response } + end end - should_create('json_class', :delete, true, 'Chef::CookbookVersion') - should_fail_to_create('json_class', 'Chef::Role', 400, "Field 'json_class' invalid") - should_fail_to_create('metadata', {}, 400, "Field 'metadata.version' missing") - end # context basic tests - - context "checking segments" do - %w{resources providers recipes definitions libraries attributes - files templates root_files}.each do |segment| + rejects_invalid_keys - should_fail_to_create(segment, "foo", 400, - "Field '#{segment}' invalid") - should_fail_to_create(segment, [ {} ], 400, - "Invalid element in array value of '#{segment}'.") - end - end # context checking segments + end - context "checking metadata sections" do - %w{platforms dependencies}.each do |section| - should_fail_to_create_metadata(section, "foo", 400, "Field 'metadata.#{section}' invalid") - should_fail_to_create_metadata(section, {"foo" => malformed_constraint}, - 400, "Invalid value '#{malformed_constraint}' for metadata.#{section}") - end + context "creating broken cookbooks to test validation and defaults", :validation do + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_version) { "1.2.3" } - def self.should_create_with_metadata(_attribute, _value) - context "when #{_attribute} is set to #{_value}" do - let(:cookbook_name) { Pedant::Utility.with_unique_suffix("pedant-cookbook") } + malformed_constraint = "s395dss@#" - # These macros need to be refactored and updated for flexibility. - # The cookbook endpoint uses PUT for both create and update, so this - # throws a monkey wrench into the mix. - should_change_metadata _attribute, _value, _value, 201 - end - end - - context "with metadata.dependencies" do + context "basic tests" do after(:each) do delete_cookbook(admin_user, cookbook_name, cookbook_version) end - ["> 1.0", "< 2.1.2", "3.3", "<= 4.6", "~> 5.6.2", ">= 6.0"].each do |dep| - should_create_with_metadata 'dependencies', {"chef-client" => "> 2.0.0", "apt" => dep} - end + should_create('json_class', :delete, true, 'Chef::CookbookVersion') + should_fail_to_create('json_class', 'Chef::Role', 400, "Field 'json_class' invalid") + should_fail_to_create('metadata', {}, 400, "Field 'metadata.version' missing") + end # context basic tests + + context "checking segments" do + segments = if api_version >= 2 + %w{all_files} + else + %w{resources providers recipes definitions libraries attributes files templates root_files} + end + + segments.each do |segment| - ["> 1", "< 2", "3", "<= 4", "~> 5", ">= 6", "= 7", ">= 1.2.3.4", "<= 5.6.7.8.9.0"].each do |dep| - should_fail_to_create_metadata( - 'dependencies', - {"chef-client" => "> 2.0.0", "apt" => dep}, - 400, "Invalid value '#{dep}' for metadata.dependencies") + should_fail_to_create(segment, "foo", 400, + "Field '#{segment}' invalid") + should_fail_to_create(segment, [ {} ], 400, + "Invalid element in array value of '#{segment}'.") end - end + end # context checking segments - context "with metadata.providing" do - after(:each) { delete_cookbook admin_user, cookbook_name, cookbook_version } - - # http://docs.chef.io/config_rb_metadata.html#provides - should_create_with_metadata 'providing', 'cats::sleep' - should_create_with_metadata 'providing', 'here(:kitty, :time_to_eat)' - should_create_with_metadata 'providing', 'service[snuggle]' - should_create_with_metadata 'providing', '' - should_create_with_metadata 'providing', 1 - should_create_with_metadata 'providing', true - should_create_with_metadata 'providing', ['cats', 'sleep', 'here'] - should_create_with_metadata 'providing', - { 'cats::sleep' => '0.0.1', - 'here(:kitty, :time_to_eat)' => '0.0.1', - 'service[snuggle]' => '0.0.1' } + context "checking metadata sections" do + %w{platforms dependencies}.each do |section| + should_fail_to_create_metadata(section, "foo", 400, "Field 'metadata.#{section}' invalid") + should_fail_to_create_metadata(section, {"foo" => malformed_constraint}, + 400, "Invalid value '#{malformed_constraint}' for metadata.#{section}") + end - end - end # context checking metadata sections + def self.should_create_with_metadata(_attribute, _value) + context "when #{_attribute} is set to #{_value}" do + let(:cookbook_name) { Pedant::Utility.with_unique_suffix("pedant-cookbook") } - context 'with invalid version in url' do - let(:expected_response) { invalid_cookbook_version_response } - let(:url) { named_cookbook_url } - let(:payload) { {} } - let(:cookbook_version) { 'abc' } + # These macros need to be refactored and updated for flexibility. + # The cookbook endpoint uses PUT for both create and update, so this + # throws a monkey wrench into the mix. + should_change_metadata _attribute, _value, _value, 201 + end + end - it "should respond with an error" do - put(url, admin_user, :payload => payload) do |response| - response.should look_like expected_response + context "with metadata.dependencies" do + after(:each) do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + end + + ["> 1.0", "< 2.1.2", "3.3", "<= 4.6", "~> 5.6.2", ">= 6.0"].each do |dep| + should_create_with_metadata 'dependencies', {"chef-client" => "> 2.0.0", "apt" => dep} + end + + ["> 1", "< 2", "3", "<= 4", "~> 5", ">= 6", "= 7", ">= 1.2.3.4", "<= 5.6.7.8.9.0"].each do |dep| + should_fail_to_create_metadata( + 'dependencies', + {"chef-client" => "> 2.0.0", "apt" => dep}, + 400, "Invalid value '#{dep}' for metadata.dependencies") + end end - end # it invalid version in URL is a 400 - end # with invalid version in url - it "invalid cookbook name in URL is a 400" do - payload = {} - put(api_url("/#{cookbook_url_base}/first@second/1.2.3"), admin_user, - :payload => payload) do |response| - error = "Invalid cookbook name 'first@second' using regex: 'Malformed cookbook name. Must only contain A-Z, a-z, 0-9, _, . or -'." - response.should look_like({ - :status => 400, - :body => { - "error" => [error] - } - }) - end - end # it invalid cookbook name in URL is a 400 + context "with metadata.providing" do + after(:each) { delete_cookbook admin_user, cookbook_name, cookbook_version } + + # http://docs.chef.io/config_rb_metadata.html#provides + should_create_with_metadata 'providing', 'cats::sleep' + should_create_with_metadata 'providing', 'here(:kitty, :time_to_eat)' + should_create_with_metadata 'providing', 'service[snuggle]' + should_create_with_metadata 'providing', '' + should_create_with_metadata 'providing', 1 + should_create_with_metadata 'providing', true + should_create_with_metadata 'providing', ['cats', 'sleep', 'here'] + should_create_with_metadata 'providing', + { 'cats::sleep' => '0.0.1', + 'here(:kitty, :time_to_eat)' => '0.0.1', + 'service[snuggle]' => '0.0.1' } - it "mismatched metadata.cookbook_version is a 400" do - payload = new_cookbook(cookbook_name, "0.0.1") - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, - :payload => payload) do |response| - error = "Field 'name' invalid" - response.should look_like({ - :status => 400, - :body => { - "error" => [error] - } - }) - end - end # it mismatched metadata.cookbook_version is a 400 + end + end # context checking metadata sections + + context 'with invalid version in url' do + let(:expected_response) { invalid_cookbook_version_response } + let(:url) { named_cookbook_url } + let(:payload) { {} } + let(:cookbook_version) { 'abc' } + + it "should respond with an error" do + put(url, admin_user, :payload => payload) do |response| + response.should look_like expected_response + end + end # it invalid version in URL is a 400 + end # with invalid version in url + + it "invalid cookbook name in URL is a 400" do + payload = {} + put(api_url("/#{cookbook_url_base}/first@second/1.2.3"), admin_user, + :payload => payload) do |response| + error = "Invalid cookbook name 'first@second' using regex: 'Malformed cookbook name. Must only contain A-Z, a-z, 0-9, _, . or -'." + response.should look_like({ + :status => 400, + :body => { + "error" => [error] + } + }) + end + end # it invalid cookbook name in URL is a 400 - it "mismatched cookbook_name is a 400" do - payload = new_cookbook("foobar", cookbook_version) - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, - :payload => payload) do |response| - error = "Field 'name' invalid" - response.should look_like({ - :status => 400, - :body => { - "error" => [error] - } - }) - end - end # it mismatched cookbook_name is a 400 + it "mismatched metadata.cookbook_version is a 400" do + payload = new_cookbook(cookbook_name, "0.0.1") + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, + :payload => payload) do |response| + error = "Field 'name' invalid" + response.should look_like({ + :status => 400, + :body => { + "error" => [error] + } + }) + end + end # it mismatched metadata.cookbook_version is a 400 - context "sandbox checks" do - after(:each) do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - end - it "specifying file not in sandbox is a 400" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["recipes"] = [ - { - "name" => "default.rb", - "path" => "recipes/default.rb", - "checksum" => "8288b67da0793b5abec709d6226e6b73", - "specificity" => "default" - } - ] - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - error = "Manifest has a checksum that hasn't been uploaded." + it "mismatched cookbook_name is a 400" do + payload = new_cookbook("foobar", cookbook_version) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, + :payload => payload) do |response| + error = "Field 'name' invalid" response.should look_like({ - :status => 400, - :body => { - "error" => [error] - } - }) + :status => 400, + :body => { + "error" => [error] + } + }) + end + end # it mismatched cookbook_name is a 400 + + context "sandbox checks" do + after(:each) do + delete_cookbook(admin_user, cookbook_name, cookbook_version) end - end # it specifying file not in sandbox is a 400 - end # context sandbox checks - end # context creating broken cookbooks to test validation and defaults - - context "creating good cookbooks to test defaults" do - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_version) { "1.2.3" } - - let(:description) { "my cookbook" } - let(:long_description) { "this is a great cookbook" } - let(:maintainer) { "This is my name" } - let(:maintainer_email) { "cookbook_author@example.com" } - let(:license) { "MPL" } - - let (:opts) { - { - :description => description, - :long_description => long_description, - :maintainer => maintainer, - :maintainer_email => maintainer_email, - :license => license + it "specifying file not in sandbox is a 400" do + payload = new_cookbook(cookbook_name, cookbook_version) + seg = platform.server_api_version >= 2 ? "all_files" : "recipes" + payload[seg] = [ + { + "name" => "default.rb", + "path" => "recipes/default.rb", + "checksum" => "8288b67da0793b5abec709d6226e6b73", + "specificity" => "default" + } + ] + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + error = "Manifest has a checksum that hasn't been uploaded." + response.should look_like({ + :status => 400, + :body => { + "error" => [error] + } + }) + end + end # it specifying file not in sandbox is a 400 + end # context sandbox checks + end # context creating broken cookbooks to test validation and defaults + + context "creating good cookbooks to test defaults" do + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_version) { "1.2.3" } + + let(:description) { "my cookbook" } + let(:long_description) { "this is a great cookbook" } + let(:maintainer) { "This is my name" } + let(:maintainer_email) { "cookbook_author@example.com" } + let(:license) { "MPL" } + + let (:opts) { + { + :description => description, + :long_description => long_description, + :maintainer => maintainer, + :maintainer_email => maintainer_email, + :license => license + } } - } - after :each do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - end + after :each do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + end - respects_maximum_payload_size + respects_maximum_payload_size - it "allows creation of a minimal cookbook with no data" do + it "allows creation of a minimal cookbook with no data" do - # Since PUT returns the same thing it was given, we'll just - # define the input in terms of the response, since we use that - # elsewhere in the test suite. - payload = retrieved_cookbook(cookbook_name, cookbook_version) + # Since PUT returns the same thing it was given, we'll just + # define the input in terms of the response, since we use that + # elsewhere in the test suite. + payload = retrieved_cookbook(cookbook_name, cookbook_version) - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, - :payload => payload) do |response| - response. - should look_like({ - :status => 201, - :body => payload - }) - end - end # it allows creation of a minimal cookbook with no data - - it "allows override of defaults" do - payload = new_cookbook(cookbook_name, cookbook_version, opts) - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - response. - should look_like({ - :status => 201, - :body => retrieved_cookbook(cookbook_name, cookbook_version, - opts) - }) - end - end # it allows override of defaults - end # context creating good gookbooks to test defaults - end # context PUT /cookbooks// [create] - - context "PUT multiple cookbooks" do - let(:cookbook_name) { "multiple_versions" } - let(:cookbook_version1) { "1.2.3" } - let(:cookbook_version2) { "1.3.0" } - - after :each do - [cookbook_version1, cookbook_version2].each do |v| - delete(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{v}"), admin_user) - end - end + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, + :payload => payload) do |response| + response. + should look_like({ + :status => 201, + :body => payload + }) + end + end # it allows creation of a minimal cookbook with no data + + it "allows override of defaults" do + payload = new_cookbook(cookbook_name, cookbook_version, opts) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 201, + :body => retrieved_cookbook(cookbook_name, cookbook_version, + opts) + }) + end + end # it allows override of defaults + end # context creating good gookbooks to test defaults + end # context PUT /cookbooks// [create] - it "allows us to create 2 versions of the same cookbook" do - payload = new_cookbook(cookbook_name, cookbook_version1, {}) - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version1}"), - admin_user, - :payload => payload) do |response| - response.should look_like({ - :status => 201, - :body => payload - }) - end + context "PUT multiple cookbooks" do + let(:cookbook_name) { "multiple_versions" } + let(:cookbook_version1) { "1.2.3" } + let(:cookbook_version2) { "1.3.0" } - payload2 = new_cookbook(cookbook_name, cookbook_version2, {}) - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version2}"), - admin_user, - :payload => payload2) do |response| - response.should look_like({ - :status => 201, - :body => payload2 - }) + after :each do + [cookbook_version1, cookbook_version2].each do |v| + delete(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{v}"), admin_user) + end end - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version1}"), - admin_user) do |response| - response.should look_like({ - :status => 200, - :body => retrieved_cookbook(cookbook_name, cookbook_version1) - }) - end + it "allows us to create 2 versions of the same cookbook" do + payload = new_cookbook(cookbook_name, cookbook_version1, {}) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version1}"), + admin_user, + :payload => payload) do |response| + response.should look_like({ + :status => 201, + :body => payload + }) + end + + payload2 = new_cookbook(cookbook_name, cookbook_version2, {}) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version2}"), + admin_user, + :payload => payload2) do |response| + response.should look_like({ + :status => 201, + :body => payload2 + }) + end + + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version1}"), + admin_user) do |response| + response.should look_like({ + :status => 200, + :body => retrieved_cookbook(cookbook_name, cookbook_version1) + }) + end + + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version2}"), + admin_user) do |response| + response.should look_like({ + :status => 200, + :body => retrieved_cookbook(cookbook_name, cookbook_version2) + }) + end + end # it allows us to create 2 versions of the same cookbook + end # context PUT multiple cookbooks + end + + describe "API v0" do + it_behaves_like "creates cookbooks", 0 + end + + describe "API v2" do + it_behaves_like "creates cookbooks", 2 + end - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version2}"), - admin_user) do |response| - response.should look_like({ - :status => 200, - :body => retrieved_cookbook(cookbook_name, cookbook_version2) - }) - end - end # it allows us to create 2 versions of the same cookbook - end # context PUT multiple cookbooks end # describe Cookbooks API endpoint diff --git a/oc-chef-pedant/spec/api/cookbooks/delete_spec.rb b/oc-chef-pedant/spec/api/cookbooks/delete_spec.rb index ef685758254..09308b31a4f 100644 --- a/oc-chef-pedant/spec/api/cookbooks/delete_spec.rb +++ b/oc-chef-pedant/spec/api/cookbooks/delete_spec.rb @@ -17,136 +17,163 @@ describe "Cookbooks API endpoint", :cookbooks, :cookbooks_delete do - let(:cookbook_url_base) { "cookbooks" } + shared_examples "deletes cookbooks" do |api_version| + let(:cookbook_url_base) { "cookbooks" } + let(:api_version) { api_version } - include Pedant::RSpec::CookbookUtil + before do + platform.server_api_version = api_version + end - context "DELETE /cookbooks//" do - let(:request_method) { :DELETE } - let(:request_url) { named_cookbook_url } - let(:requestor) { admin_user } + after do + platform.reset_server_api_version + end - let(:fetched_cookbook) { original_cookbook } - let(:original_cookbook) { new_cookbook(cookbook_name, cookbook_version) } + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" + end + end - context "for non-existent cookbooks" do - let(:expected_response) { cookbook_version_not_found_exact_response } + include Pedant::RSpec::CookbookUtil - let(:cookbook_name) { "non_existent" } - let(:cookbook_version) { "1.2.3" } + context "DELETE /cookbooks//" do + let(:request_method) { :DELETE } + let(:request_url) { named_cookbook_url } + let(:requestor) { admin_user } - should_respond_with 404 + let(:fetched_cookbook) { original_cookbook } + let(:original_cookbook) { new_cookbook(cookbook_name, cookbook_version) } - context 'with bad version', :validation do - let(:expected_response) { invalid_cookbook_version_exact_response } - let(:cookbook_version) { "1.2.3.4" } - should_respond_with 400 - end # with bad version - end # context for non-existent cookbooks + context "for non-existent cookbooks" do + let(:expected_response) { cookbook_version_not_found_exact_response } - context "for existing cookbooks" do - let(:cookbook_name) { "cookbook-to-be-deleted" } - let(:cookbook_version) { "1.2.3" } + let(:cookbook_name) { "non_existent" } + let(:cookbook_version) { "1.2.3" } - context "when deleting non-existent version of an existing cookbook" do - let(:expected_response) { cookbook_version_not_found_exact_response } - let(:cookbook_version_not_found_error_message) { ["Cannot find a cookbook named #{cookbook_name} with version #{non_existing_version}"] } - let(:non_existing_version) { "99.99.99" } - let(:non_existing_version_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{non_existing_version}") } - let(:fetched_cookbook) { original_cookbook } + should_respond_with 404 - before(:each) { make_cookbook(admin_user, cookbook_name, cookbook_version) } - after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_version) } + context 'with bad version', :validation do + let(:expected_response) { invalid_cookbook_version_exact_response } + let(:cookbook_version) { "1.2.3.4" } + should_respond_with 400 + end # with bad version + end # context for non-existent cookbooks - it "should respond with 404 (\"Not Found\") and not delete existing versions" do - delete(non_existing_version_url, admin_user) do |response| - response.should look_like expected_response - end + context "for existing cookbooks" do + let(:cookbook_name) { "cookbook-to-be-deleted" } + let(:cookbook_version) { "1.2.3" } + + context "when deleting non-existent version of an existing cookbook" do + let(:expected_response) { cookbook_version_not_found_exact_response } + let(:cookbook_version_not_found_error_message) { ["Cannot find a cookbook named #{cookbook_name} with version #{non_existing_version}"] } + let(:non_existing_version) { "99.99.99" } + let(:non_existing_version_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{non_existing_version}") } + let(:fetched_cookbook) { original_cookbook } + + before(:each) { make_cookbook(admin_user, cookbook_name, cookbook_version) } + after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_version) } - should_not_be_deleted - end - end # it doesn't delete the wrong version of an existing cookbook + it "should respond with 404 (\"Not Found\") and not delete existing versions" do + delete(non_existing_version_url, admin_user) do |response| + response.should look_like expected_response + end - context "when deleting existent version of an existing cookbook", :smoke do - let(:recipe_name) { "test_recipe" } - let(:recipe_content) { "hello-#{unique_suffix}-#{rand(1000)}-#{rand(1000)}" } - let(:recipe_spec) do + should_not_be_deleted + end + end # it doesn't delete the wrong version of an existing cookbook + + context "when deleting existent version of an existing cookbook", :smoke do + let(:recipe_name) { "test_recipe" } + let(:recipe_content) { "hello-#{unique_suffix}-#{rand(1000)}-#{rand(1000)}" } + let(:recipe_spec) do { :name => recipe_name, :content => recipe_content } - end + end - let(:cookbooks) do - { - cookbook_name => { - cookbook_version => [recipe_spec] + let(:cookbooks) do + { + cookbook_name => { + cookbook_version => [recipe_spec] + } } - } - end + end + + let(:fetched_cookbook) { original_cookbook } + let(:original_cookbook) { new_cookbook(cookbook_name, cookbook_version) } + + before(:each) { setup_cookbooks(cookbooks) } + after(:each) { remove_cookbooks(cookbooks) } + + it "should cleanup unused checksum data in s3/bookshelf" do + verify_checksum_cleanup(:recipes) do + response.should look_like(:status => 200) + end + end + + end # context when deleting existent version... + end # context for existing cookbooks + + context "with permissions for" do + let(:cookbook_name) {"delete-cookbook"} + let(:cookbook_version) { "0.0.1" } + let(:not_found_msg) { ["Cannot find a cookbook named delete-cookbook with version 0.0.1"] } + + before(:each) { make_cookbook(admin_user, cookbook_name, cookbook_version) } + after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_version) } + + context 'as admin user' do + let(:expected_response) { delete_cookbook_success_response } + + it "should respond with 200 (\"OK\") and be deleted" do + should look_like expected_response + should_be_deleted + end # it admin user returns 200 + end # as admin user - let(:fetched_cookbook) { original_cookbook } - let(:original_cookbook) { new_cookbook(cookbook_name, cookbook_version) } + context 'as normal user', :authorization do + let(:expected_response) { delete_cookbook_success_response } - before(:each) { setup_cookbooks(cookbooks) } - after(:each) { remove_cookbooks(cookbooks) } + let(:requestor) { normal_user } + it "should respond with 200 (\"OK\") and be deleted" do + should look_like expected_response + should_be_deleted + end # it admin user returns 200 + end # with normal user - it "should cleanup unused checksum data in s3/bookshelf" do - verify_checksum_cleanup(:recipes) do - response.should look_like(:status => 200) + context 'as a user outside of the organization', :authorization do + let(:expected_response) { unauthorized_access_credential_response } + let(:requestor) { outside_user } + + it "should respond with 403 (\"Forbidden\") and does not delete cookbook" do + response.should look_like expected_response + should_not_be_deleted end - end - - end # context when deleting existent version... - end # context for existing cookbooks - - context "with permissions for" do - let(:cookbook_name) {"delete-cookbook"} - let(:cookbook_version) { "0.0.1" } - let(:not_found_msg) { ["Cannot find a cookbook named delete-cookbook with version 0.0.1"] } - - before(:each) { make_cookbook(admin_user, cookbook_name, cookbook_version) } - after(:each) { delete_cookbook(admin_user, cookbook_name, cookbook_version) } - - context 'as admin user' do - let(:expected_response) { delete_cookbook_success_response } - - it "should respond with 200 (\"OK\") and be deleted" do - should look_like expected_response - should_be_deleted - end # it admin user returns 200 - end # as admin user - - context 'as normal user', :authorization do - let(:expected_response) { delete_cookbook_success_response } - - let(:requestor) { normal_user } - it "should respond with 200 (\"OK\") and be deleted" do - should look_like expected_response - should_be_deleted - end # it admin user returns 200 - end # with normal user - - context 'as a user outside of the organization', :authorization do - let(:expected_response) { unauthorized_access_credential_response } - let(:requestor) { outside_user } - - it "should respond with 403 (\"Forbidden\") and does not delete cookbook" do - response.should look_like expected_response - should_not_be_deleted - end - end # it outside user returns 403 - - context 'with invalid user', :authorization do - let(:expected_response) { invalid_credential_exact_response } - let(:requestor) { invalid_user } - - it "should respond with 401 (\"Unauthorized\") and does not delete cookbook" do - response.should look_like expected_response - should_not_be_deleted - end # responds with 401 - end # with invalid user - - end # context with permissions for - end # context DELETE /cookbooks// + end # it outside user returns 403 + + context 'with invalid user', :authorization do + let(:expected_response) { invalid_credential_exact_response } + let(:requestor) { invalid_user } + + it "should respond with 401 (\"Unauthorized\") and does not delete cookbook" do + response.should look_like expected_response + should_not_be_deleted + end # responds with 401 + end # with invalid user + + end # context with permissions for + end # context DELETE /cookbooks// + end + + describe "API v0" do + it_behaves_like "deletes cookbooks", 0 + end + + describe "API v2" do + it_behaves_like "deletes cookbooks", 2 + end end diff --git a/oc-chef-pedant/spec/api/cookbooks/named_filters_spec.rb b/oc-chef-pedant/spec/api/cookbooks/named_filters_spec.rb index 863cd4b233e..f48593847f8 100644 --- a/oc-chef-pedant/spec/api/cookbooks/named_filters_spec.rb +++ b/oc-chef-pedant/spec/api/cookbooks/named_filters_spec.rb @@ -17,188 +17,214 @@ describe "Cookbooks API endpoint, named filters", :cookbooks, :cookbooks_named_filters do - let(:cookbook_url_base) { "cookbooks" } - - include Pedant::RSpec::CookbookUtil - - let(:request_method) { :GET } - let(:request_url) { api_url "/#{cookbook_url_base}/#{named_filter}" } - let(:requestor) { admin_user } - - # Generates a hash of cookbook name -> cookbook version url for the - # most recent version of each cookbook in a cookbook spec - def expected_for_latest(cookbooks) - latest = get_latest_cookbooks(cookbooks) - latest.inject({}) do |body, cookbook_spec| - name, version_specs = cookbook_spec - latest_version = version_specs.first - version_string, _recipe_names = latest_version - body[name] = api_url("/#{cookbook_url_base}/#{name}/#{version_string}") - body + shared_examples "filters cookbooks" do |api_version| + let(:cookbook_url_base) { "cookbooks" } + let(:api_version) { api_version } + + before do + platform.server_api_version = api_version end - end - def expected_for_named_cookbook(cookbooks, named_cookbook) - cookbook = cookbooks.select{ |k,v| k == named_cookbook} + after do + platform.reset_server_api_version + end + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" + end + end - cookbook.inject({}) do |body, cookbook_spec| - name, version_specs = cookbook_spec - body[name] = { - "url" => api_url("/#{cookbook_url_base}/#{name}"), - "versions" => version_specs.map do |version, recipes| - { "version" => version, - "url" => api_url("/#{cookbook_url_base}/#{name}/#{version}") - } - end - } - body + include Pedant::RSpec::CookbookUtil + + let(:request_method) { :GET } + let(:request_url) { api_url "/#{cookbook_url_base}/#{named_filter}" } + let(:requestor) { admin_user } + + # Generates a hash of cookbook name -> cookbook version url for the + # most recent version of each cookbook in a cookbook spec + def expected_for_latest(cookbooks) + latest = get_latest_cookbooks(cookbooks) + latest.inject({}) do |body, cookbook_spec| + name, version_specs = cookbook_spec + latest_version = version_specs.first + version_string, _recipe_names = latest_version + body[name] = api_url("/#{cookbook_url_base}/#{name}/#{version_string}") + body + end end - end - # Generates a list of cookbook-qualified recipe names for the most - # recent cookbooks in a cookbook spec - def expected_for_recipes(cookbooks) - latest = get_latest_cookbooks(cookbooks) - nested = latest.map do |cookbook_spec| - cookbook_name, version_specs = cookbook_spec - latest_version = version_specs.first - _version_string, recipe_names =latest_version + def expected_for_named_cookbook(cookbooks, named_cookbook) + cookbook = cookbooks.select{ |k,v| k == named_cookbook} + + + cookbook.inject({}) do |body, cookbook_spec| + name, version_specs = cookbook_spec + body[name] = { + "url" => api_url("/#{cookbook_url_base}/#{name}"), + "versions" => version_specs.map do |version, recipes| + { "version" => version, + "url" => api_url("/#{cookbook_url_base}/#{name}/#{version}") + } + end + } + body + end + end + + # Generates a list of cookbook-qualified recipe names for the most + # recent cookbooks in a cookbook spec + def expected_for_recipes(cookbooks) + latest = get_latest_cookbooks(cookbooks) + nested = latest.map do |cookbook_spec| + cookbook_name, version_specs = cookbook_spec + latest_version = version_specs.first + _version_string, recipe_names =latest_version - # don't sort the recipes for the Ruby endpoint; CouchDB keeps them in insertion order - recipe_names.sort! + # don't sort the recipes for the Ruby endpoint; CouchDB keeps them in insertion order + recipe_names.sort! - recipe_names.map do |recipe_name| - "#{cookbook_name}::#{recipe_name}" + recipe_names.map do |recipe_name| + "#{cookbook_name}::#{recipe_name}" + end end + nested.flatten end - nested.flatten - end - let(:cookbooks) { raise "Must define a cookbook spec!" } - let(:cookbook_name) {"my_cookbook"} + let(:cookbooks) { raise "Must define a cookbook spec!" } + let(:cookbook_name) {"my_cookbook"} - # Changed from before(:all) to avoid some problems, but it slows down these tests - before(:each) { setup_cookbooks(cookbooks) } - after(:each) { remove_cookbooks(cookbooks) } + # Changed from before(:all) to avoid some problems, but it slows down these tests + before(:each) { setup_cookbooks(cookbooks) } + after(:each) { remove_cookbooks(cookbooks) } - # All these tests are basically the same, and just have different - # cookbooks in the system, so I'm pulling it all out into a shared - # set of examples - def self.should_respond_with_latest_cookbooks - context 'when requesting /cookbooks/_latest' do - let(:named_filter) { :_latest } - let(:expected_response) { fetch_cookbook_success_exact_response } - let(:fetched_cookbook) { expected_for_latest(cookbooks) } + # All these tests are basically the same, and just have different + # cookbooks in the system, so I'm pulling it all out into a shared + # set of examples + def self.should_respond_with_latest_cookbooks + context 'when requesting /cookbooks/_latest' do + let(:named_filter) { :_latest } + let(:expected_response) { fetch_cookbook_success_exact_response } + let(:fetched_cookbook) { expected_for_latest(cookbooks) } - it "should respond with the 'latest' cookbooks" do - should look_like expected_response + it "should respond with the 'latest' cookbooks" do + should look_like expected_response + end end end - end - def self.should_respond_with_recipes_from_latest_cookbooks - context 'when requesting /cookbooks/_recipes' do - let(:named_filter) { :_recipes } - let(:expected_response) { fetch_cookbook_success_exact_response } + def self.should_respond_with_recipes_from_latest_cookbooks + context 'when requesting /cookbooks/_recipes' do + let(:named_filter) { :_recipes } + let(:expected_response) { fetch_cookbook_success_exact_response } - it "shows the recipes from the latest cookbooks" do - should have_status_code 200 - parse(response).should == expected_for_recipes(cookbooks) + it "shows the recipes from the latest cookbooks" do + should have_status_code 200 + parse(response).should == expected_for_recipes(cookbooks) + end end end - end - def self.should_respond_with_single_cookbook - context 'when requesting /cookbooks/my_cookbook' do - let(:cookbook) { "my_cookbook" } - let(:named_filter) { "my_cookbook" } - let(:fetched_cookbook) { expected_for_named_cookbook(cookbooks, named_filter) } - let(:expected_response) { fetch_cookbook_success_exact_response } + def self.should_respond_with_single_cookbook + context 'when requesting /cookbooks/my_cookbook' do + let(:cookbook) { "my_cookbook" } + let(:named_filter) { "my_cookbook" } + let(:fetched_cookbook) { expected_for_named_cookbook(cookbooks, named_filter) } + let(:expected_response) { fetch_cookbook_success_exact_response } - it "should respond with the 'named' cookbook" do - should look_like expected_response + it "should respond with the 'named' cookbook" do + should look_like expected_response + end end end - end - def self.should_respond_with_single_cookbook_not_found - context 'when requesting /cookbooks/my_cookbook' do - let(:named_filter) { "my_cookbook" } - let(:expected_response) { fetch_cookbook_not_found_exact_response } + def self.should_respond_with_single_cookbook_not_found + context 'when requesting /cookbooks/my_cookbook' do + let(:named_filter) { "my_cookbook" } + let(:expected_response) { fetch_cookbook_not_found_exact_response } - it "should respond with 404" do - should look_like expected_response + it "should respond with 404" do + should look_like expected_response + end end end - end - ## Now for the actual tests + ## Now for the actual tests - context "with no cookbooks" do - let(:cookbooks) { {} } + context "with no cookbooks" do + let(:cookbooks) { {} } - should_respond_with_latest_cookbooks - should_respond_with_recipes_from_latest_cookbooks - should_respond_with_single_cookbook_not_found - end + should_respond_with_latest_cookbooks + should_respond_with_recipes_from_latest_cookbooks + should_respond_with_single_cookbook_not_found + end + + # TODO: This would be a good candidate for a smoke test + # if there were a way to turn off testing against exact body responses. + # We don't know what existing cookbooks are on the target server for + # smoke tests. + context "with one cookbook, one version" do + let(:cookbooks) do + {"my_cookbook" => {"1.0.0" => ["recipe1", "recipe2"]}} + end - # TODO: This would be a good candidate for a smoke test - # if there were a way to turn off testing against exact body responses. - # We don't know what existing cookbooks are on the target server for - # smoke tests. - context "with one cookbook, one version" do - let(:cookbooks) do - {"my_cookbook" => {"1.0.0" => ["recipe1", "recipe2"]}} + should_respond_with_latest_cookbooks + should_respond_with_recipes_from_latest_cookbooks + should_respond_with_single_cookbook end - should_respond_with_latest_cookbooks - should_respond_with_recipes_from_latest_cookbooks - should_respond_with_single_cookbook - end + context "with different cookbook, one version" do + let(:cookbooks) do + {"your_cookbook" => {"1.0.0" => ["recipe1", "recipe2"]}} + end - context "with different cookbook, one version" do - let(:cookbooks) do - {"your_cookbook" => {"1.0.0" => ["recipe1", "recipe2"]}} + should_respond_with_latest_cookbooks + should_respond_with_recipes_from_latest_cookbooks + should_respond_with_single_cookbook_not_found end - should_respond_with_latest_cookbooks - should_respond_with_recipes_from_latest_cookbooks - should_respond_with_single_cookbook_not_found - end + context "with multiple cookbooks, one version each" do + let(:cookbooks) do + { + "my_cookbook" => {"1.0.0" => ["recipe1", "recipe2"]}, + "your_cookbook" => {"1.3.0" => ["foo", "bar"]}, + } + end - context "with multiple cookbooks, one version each" do - let(:cookbooks) do - { - "my_cookbook" => {"1.0.0" => ["recipe1", "recipe2"]}, - "your_cookbook" => {"1.3.0" => ["foo", "bar"]}, - } + should_respond_with_latest_cookbooks + should_respond_with_recipes_from_latest_cookbooks + should_respond_with_single_cookbook end - should_respond_with_latest_cookbooks - should_respond_with_recipes_from_latest_cookbooks - should_respond_with_single_cookbook - end + context "with multiple cookbooks, multiple versions each" do + let(:cookbooks) do + { + "my_cookbook" => { + "1.0.0" => ["monitoring", "security"], + "1.5.0" => ["security", "users"] + }, + "your_cookbook" => { + "1.3.0" => ["webserver", "database"], + "2.0.0" => ["webserver", "database", "load_balancer"] + }, + } + end - context "with multiple cookbooks, multiple versions each" do - let(:cookbooks) do - { - "my_cookbook" => { - "1.0.0" => ["monitoring", "security"], - "1.5.0" => ["security", "users"] - }, - "your_cookbook" => { - "1.3.0" => ["webserver", "database"], - "2.0.0" => ["webserver", "database", "load_balancer"] - }, - } + should_respond_with_latest_cookbooks + should_respond_with_recipes_from_latest_cookbooks + should_respond_with_single_cookbook end - should_respond_with_latest_cookbooks - should_respond_with_recipes_from_latest_cookbooks - should_respond_with_single_cookbook + end + describe "API v0" do + it_behaves_like "filters cookbooks", 0 end + describe "API v2" do + it_behaves_like "filters cookbooks", 2 + end end diff --git a/oc-chef-pedant/spec/api/cookbooks/read_spec.rb b/oc-chef-pedant/spec/api/cookbooks/read_spec.rb index dc886b143ae..0f82192639f 100644 --- a/oc-chef-pedant/spec/api/cookbooks/read_spec.rb +++ b/oc-chef-pedant/spec/api/cookbooks/read_spec.rb @@ -22,316 +22,342 @@ describe "Cookbooks API endpoint", :cookbooks, :cookbooks_read do - let(:cookbook_url_base) { "cookbooks" } - - include Pedant::RSpec::CookbookUtil - - context "GET /cookbooks" do - let(:url) { cookbook_collection_url } - let(:request_url) { cookbook_collection_url } - let(:request_method) { :GET } - let(:requestor) { admin_user } - - let(:cookbook_collection_url) { api_url("/#{cookbook_url_base}") } - let(:fetch_cookbook_collection_success_exact_response) do - { - :status => 200, - :body_exact => fetched_cookbooks - } + shared_examples "reads cookbooks" do |api_version| + let(:cookbook_url_base) { "cookbooks" } + let(:api_version) { api_version } + + before do + platform.server_api_version = api_version end - context "with an operational server", :smoke do - it { should look_like ok_response } + after do + platform.reset_server_api_version end - context "without existing cookbooks" do - let(:expected_response) { fetch_cookbook_collection_success_exact_response } + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" + end + end - # Assert that there are no cookbooks - let(:fetched_cookbooks) { { } } + include Pedant::RSpec::CookbookUtil - # NOTE: This test must the first one dealing with cookbooks - # so that there are no cookbooks existing on the server + context "GET /cookbooks" do + let(:url) { cookbook_collection_url } + let(:request_url) { cookbook_collection_url } + let(:request_method) { :GET } + let(:requestor) { admin_user } - should_respond_with 200, "and an empty collection" + let(:cookbook_collection_url) { api_url("/#{cookbook_url_base}") } + let(:fetch_cookbook_collection_success_exact_response) do + { + :status => 200, + :body_exact => fetched_cookbooks + } + end - # Add test to check for empty case with different num_versions - context 'with a num_versions' do - let(:expected_response) { bad_request_response } - let(:request_url) { api_url("/#{cookbook_url_base}?num_versions=#{num_versions}") } - let(:error_message) { invalid_versions_msg } + context "with an operational server", :smoke do + it { should look_like ok_response } + end - def self.expects_response_of_400_with(message, _value) - context "with #{message}", :validation do - let(:num_versions) { _value } - should_respond_with 400 - end - end + context "without existing cookbooks" do + let(:expected_response) { fetch_cookbook_collection_success_exact_response } - expects_response_of_400_with "a negative num_versions", '-1' - expects_response_of_400_with "a missing num_versions", '' - expects_response_of_400_with "an invalid num_versions", 'foo' + # Assert that there are no cookbooks + let(:fetched_cookbooks) { { } } - end # when requesting with num_versions - end # without existing cookbooks + # NOTE: This test must the first one dealing with cookbooks + # so that there are no cookbooks existing on the server - context "with existing cookbooks and multiple versions" do - let(:expected_response) { fetch_cookbook_collection_success_exact_response } - let(:request_url) { api_url("/#{cookbook_url_base}?num_versions=#{num_versions}") } + should_respond_with 200, "and an empty collection" - let(:fetched_cookbooks) { cookbook_collection } + # Add test to check for empty case with different num_versions + context 'with a num_versions' do + let(:expected_response) { bad_request_response } + let(:request_url) { api_url("/#{cookbook_url_base}?num_versions=#{num_versions}") } + let(:error_message) { invalid_versions_msg } - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_name2) { "cookbook_name2" } - let(:version1) { "0.0.1" } - let(:version2) { "0.0.2" } - let(:cookbooks) do - { - cookbook_name => { - version1 => [], - version2 => [] - }, + def self.expects_response_of_400_with(message, _value) + context "with #{message}", :validation do + let(:num_versions) { _value } + should_respond_with 400 + end + end - cookbook_name2 => { - version1 => [] - } - } - end + expects_response_of_400_with "a negative num_versions", '-1' + expects_response_of_400_with "a missing num_versions", '' + expects_response_of_400_with "an invalid num_versions", 'foo' - before(:each) do - setup_cookbooks(cookbooks) - end + end # when requesting with num_versions + end # without existing cookbooks - after(:each) do - remove_cookbooks(cookbooks) - end + context "with existing cookbooks and multiple versions" do + let(:expected_response) { fetch_cookbook_collection_success_exact_response } + let(:request_url) { api_url("/#{cookbook_url_base}?num_versions=#{num_versions}") } + + let(:fetched_cookbooks) { cookbook_collection } - context 'with num_versions set to 0' do - let(:num_versions) { 0 } - let(:cookbook_collection) do + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_name2) { "cookbook_name2" } + let(:version1) { "0.0.1" } + let(:version2) { "0.0.2" } + let(:cookbooks) do { - cookbook_name => { "url" => cookbook_url(cookbook_name), "versions" => [] }, - cookbook_name2 => { "url" => cookbook_url(cookbook_name2), "versions" => [] } + cookbook_name => { + version1 => [], + version2 => [] + }, + + cookbook_name2 => { + version1 => [] + } } end - it "should respond with cookbook collection with no version" do - should look_like expected_response + before(:each) do + setup_cookbooks(cookbooks) end - end # with num_versions set to 0 - let(:cookbook_collection_with_one_version) do - { - cookbook_name => { - "url" => cookbook_url(cookbook_name), - "versions" => - [{ "version" => version2, - "url" => cookbook_version_url(cookbook_name,version2) }]}, - cookbook_name2 => { - "url" => cookbook_url(cookbook_name2), - "versions" => - [{ "version" => version1, - "url" => cookbook_version_url(cookbook_name2, version1) }]} - } - end - - context 'when num_versions is not set' do - let(:request_url) { cookbook_collection_url } - let(:cookbook_collection) { cookbook_collection_with_one_version } - it 'should return cookbook collection with one version per cookbook' do - should look_like expected_response + after(:each) do + remove_cookbooks(cookbooks) end - end # without num_versions - - context 'when num_versions is set to 1' do - let(:num_versions) { 1 } - let(:cookbook_collection) { cookbook_collection_with_one_version } - it 'should return cookbook collection with one version per cookbook' do - should look_like expected_response - end - end + context 'with num_versions set to 0' do + let(:num_versions) { 0 } + let(:cookbook_collection) do + { + cookbook_name => { "url" => cookbook_url(cookbook_name), "versions" => [] }, + cookbook_name2 => { "url" => cookbook_url(cookbook_name2), "versions" => [] } + } + end - context 'when num_versions is set to "all"' do - let(:num_versions) { 'all' } - let(:cookbook_collection) do + it "should respond with cookbook collection with no version" do + should look_like expected_response + end + end # with num_versions set to 0 + let(:cookbook_collection_with_one_version) do { cookbook_name => { "url" => cookbook_url(cookbook_name), - "versions" => [ - { "version" => version2, - "url" => cookbook_version_url(cookbook_name, version2) }, - { "version" => version1, - "url" => cookbook_version_url(cookbook_name, version1) } ]}, + "versions" => + [{ "version" => version2, + "url" => cookbook_version_url(cookbook_name,version2) }]}, cookbook_name2 => { - "url" => cookbook_url(cookbook_name2), - "versions" => [ - { "version" => version1, - "url" => cookbook_version_url(cookbook_name2, version1) }]} + "url" => cookbook_url(cookbook_name2), + "versions" => + [{ "version" => version1, + "url" => cookbook_version_url(cookbook_name2, version1) }]} } end - it 'should respond with a cookbook collection containing all versions of each cookbook' do - should look_like expected_response - end - - end - end # context returns different results depending on num_versions + context 'when num_versions is not set' do + let(:request_url) { cookbook_collection_url } + let(:cookbook_collection) { cookbook_collection_with_one_version } - context "with varying numbers of existing cookbooks" do - let(:expected_response) { fetch_cookbook_success_exact_response } - let(:request_url) { api_url("/#{cookbook_url_base}?num_versions=all") } + it 'should return cookbook collection with one version per cookbook' do + should look_like expected_response + end + end # without num_versions - let(:fetched_cookbook) { cookbook_collection } - let(:cookbook_name) { "cookbook_name" } - let(:cookbook_version) { "1.2.3" } + context 'when num_versions is set to 1' do + let(:num_versions) { 1 } + let(:cookbook_collection) { cookbook_collection_with_one_version } - context 'with a single, existing cookbook' do - let(:cookbook_collection) do - { - cookbook_name => { - "url" => cookbook_url(cookbook_name), - "versions" => [ - { "version" => cookbook_version, - "url" => cookbook_version_url(cookbook_name, cookbook_version) }]} - } + it 'should return cookbook collection with one version per cookbook' do + should look_like expected_response + end end - it 'should respond with a single cookbook in the collection' do - make_cookbook(admin_user, cookbook_name, cookbook_version) - should look_like expected_response - delete_cookbook(admin_user, cookbook_name, cookbook_version) + context 'when num_versions is set to "all"' do + let(:num_versions) { 'all' } + let(:cookbook_collection) do + { + cookbook_name => { + "url" => cookbook_url(cookbook_name), + "versions" => [ + { "version" => version2, + "url" => cookbook_version_url(cookbook_name, version2) }, + { "version" => version1, + "url" => cookbook_version_url(cookbook_name, version1) } ]}, + cookbook_name2 => { + "url" => cookbook_url(cookbook_name2), + "versions" => [ + { "version" => version1, + "url" => cookbook_version_url(cookbook_name2, version1) }]} + } + end + + it 'should respond with a cookbook collection containing all versions of each cookbook' do + should look_like expected_response + end + end - end + end # context returns different results depending on num_versions + + context "with varying numbers of existing cookbooks" do + let(:expected_response) { fetch_cookbook_success_exact_response } + let(:request_url) { api_url("/#{cookbook_url_base}?num_versions=all") } + + let(:fetched_cookbook) { cookbook_collection } + let(:cookbook_name) { "cookbook_name" } + let(:cookbook_version) { "1.2.3" } + + context 'with a single, existing cookbook' do + let(:cookbook_collection) do + { + cookbook_name => { + "url" => cookbook_url(cookbook_name), + "versions" => [ + { "version" => cookbook_version, + "url" => cookbook_version_url(cookbook_name, cookbook_version) }]} + } + end - context 'with multiple, existing cookbooks' do - let(:cookbook_collection) do - { - "cb1" => { - "url" => cookbook_url("cb1"), - "versions" => [ - { "version" => "0.0.1", - "url" => cookbook_version_url("cb1", "0.0.1") }]}, - "cb2" => { - "url" => cookbook_url("cb2"), - "versions" => [ - { "version" => "0.0.2", - "url" => cookbook_version_url("cb2", "0.0.2") }]} - } + it 'should respond with a single cookbook in the collection' do + make_cookbook(admin_user, cookbook_name, cookbook_version) + should look_like expected_response + delete_cookbook(admin_user, cookbook_name, cookbook_version) + end end - it "multiple cookbooks can be listed" do - # Upload cookbook - make_cookbook(admin_user, "cb1", "0.0.1") - make_cookbook(admin_user, "cb2", "0.0.2") + context 'with multiple, existing cookbooks' do + let(:cookbook_collection) do + { + "cb1" => { + "url" => cookbook_url("cb1"), + "versions" => [ + { "version" => "0.0.1", + "url" => cookbook_version_url("cb1", "0.0.1") }]}, + "cb2" => { + "url" => cookbook_url("cb2"), + "versions" => [ + { "version" => "0.0.2", + "url" => cookbook_version_url("cb2", "0.0.2") }]} + } + end - should look_like expected_response + it "multiple cookbooks can be listed" do + # Upload cookbook + make_cookbook(admin_user, "cb1", "0.0.1") + make_cookbook(admin_user, "cb2", "0.0.2") - # cleanup - delete_cookbook(admin_user, "cb1", "0.0.1") - delete_cookbook(admin_user, "cb2", "0.0.2") - end # it multiple cookbooks can be listed - end # with multiple, existing cookbooks + should look_like expected_response - end # with varying numbers of existing cookbooks - end # context GET /cookbooks + # cleanup + delete_cookbook(admin_user, "cb1", "0.0.1") + delete_cookbook(admin_user, "cb2", "0.0.2") + end # it multiple cookbooks can be listed + end # with multiple, existing cookbooks - context "GET /cookbooks//" do - let(:request_method) { :GET } - let(:request_url) { named_cookbook_url } + end # with varying numbers of existing cookbooks + end # context GET /cookbooks - let(:cookbook_name) { "the_cookbook_name" } - let(:cookbook_version) { "1.2.3" } - let(:recipe_name) { "test_recipe" } - let(:recipe_content) { "hello-#{unique_suffix}" } - let(:recipe_spec) do + context "GET /cookbooks//" do + let(:request_method) { :GET } + let(:request_url) { named_cookbook_url } + + let(:cookbook_name) { "the_cookbook_name" } + let(:cookbook_version) { "1.2.3" } + let(:recipe_name) { "test_recipe" } + let(:recipe_content) { "hello-#{unique_suffix}" } + let(:recipe_spec) do { :name => recipe_name, :content => recipe_content } - end + end - let(:cookbooks) do - { - cookbook_name => { - cookbook_version => [recipe_spec] + let(:cookbooks) do + { + cookbook_name => { + cookbook_version => [recipe_spec] + } } - } - end + end - before(:each) { setup_cookbooks(cookbooks) } - after(:each) { remove_cookbooks(cookbooks) } - - let(:fetched_cookbook) do - # NOTE: the cookbook returned will actually have some recipe - # data. We don't yet have a nice way to do the required soft - # verification for the URL and checksum. - # - # To get around this, we'll just pass in a proc that asserts - # that the "recipes" key is an array of hashes with the correct - # keys. - retrieved_cookbook(cookbook_name, - cookbook_version, - :recipes => lambda{|recipes| - recipes.is_a?(Array) && - recipes.all?{|recipe| - recipe.is_a?(Hash) && - recipe.keys.sort == ["name", "path", "checksum", "specificity", "url"].sort - } - }) - end + before(:each) { setup_cookbooks(cookbooks) } + after(:each) { remove_cookbooks(cookbooks) } + + let(:fetched_cookbook) do + # NOTE: the cookbook returned will actually have some recipe + # data. We don't yet have a nice way to do the required soft + # verification for the URL and checksum. + # + # To get around this, we'll just pass in a proc that asserts + # that the "recipes" key is an array of hashes with the correct + # keys. + retrieved_cookbook(cookbook_name, + cookbook_version, + :recipes => lambda{|recipes| + recipes.is_a?(Array) && + recipes.all?{|recipe| + recipe.is_a?(Hash) && + recipe.keys.sort == ["name", "path", "checksum", "specificity", "url"].sort + } + }) + end - context 'as a normal user' do - let(:expected_response) { fetch_cookbook_success_exact_response } - let(:requestor) { normal_user } - - should_respond_with 200 - - context "allows access to cookbook recipe files via" do - # These tests verify that the pre-signed URL that comes back - # within cookbook_version responses is usable. Validating both - # the URL generation, but also the use of bookshelf via the - # load balancer. - let(:cbv) { parse(response) } - let(:recipe_url) { cbv["recipes"].first["url"] } - - it "net/http" do - uri = URI.parse(recipe_url) - http = Net::HTTP.new(uri.hostname, uri.port) - if uri.scheme == "https" - http.use_ssl = true - http.ssl_version = Pedant::Config.ssl_version - http.verify_mode = OpenSSL::SSL::VERIFY_NONE + context 'as a normal user' do + let(:expected_response) { fetch_cookbook_success_exact_response } + let(:requestor) { normal_user } + + should_respond_with 200 + + context "allows access to cookbook recipe files via" do + # These tests verify that the pre-signed URL that comes back + # within cookbook_version responses is usable. Validating both + # the URL generation, but also the use of bookshelf via the + # load balancer. + let(:cbv) { parse(response) } + let(:recipe_url) { extract_segment(cbv, "recipes").first["url"] } + + it "net/http" do + uri = URI.parse(recipe_url) + http = Net::HTTP.new(uri.hostname, uri.port) + if uri.scheme == "https" + http.use_ssl = true + http.ssl_version = Pedant::Config.ssl_version + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + end + + response = http.get(uri.request_uri, {}) + response.body.should == recipe_content end - response = http.get(uri.request_uri, {}) - response.body.should == recipe_content - end + end # access to recipe file content - end # access to recipe file content + end # as a normal user - end # as a normal user + context 'as an admin user' do + let(:expected_response) { fetch_cookbook_success_exact_response } + let(:requestor) { admin_user } - context 'as an admin user' do - let(:expected_response) { fetch_cookbook_success_exact_response } - let(:requestor) { admin_user } + should_respond_with 200 + end # as an admin user - should_respond_with 200 - end # as an admin user + context 'as an user outside of the organization', :authorization do + let(:expected_response) { unauthorized_access_credential_response } + let(:requestor) { outside_user } - context 'as an user outside of the organization', :authorization do - let(:expected_response) { unauthorized_access_credential_response } - let(:requestor) { outside_user } + should_respond_with 403 + end # as an outside user - should_respond_with 403 - end # as an outside user + context 'with invalid user', :authentication do + let(:expected_response) { invalid_credential_exact_response } + let(:requestor) { invalid_user } - context 'with invalid user', :authentication do - let(:expected_response) { invalid_credential_exact_response } - let(:requestor) { invalid_user } + should_respond_with 401 + end + end # context GET /cookbooks// + end # describe Cookbooks API endpoint - should_respond_with 401 - end - end # context GET /cookbooks// -end # describe Cookbooks API endpoint + describe "API v0" do + it_behaves_like "reads cookbooks", 0 + end + describe "API v2" do + it_behaves_like "reads cookbooks", 2 + end +end diff --git a/oc-chef-pedant/spec/api/cookbooks/update_spec.rb b/oc-chef-pedant/spec/api/cookbooks/update_spec.rb index 15853be1ec5..36f892a518a 100644 --- a/oc-chef-pedant/spec/api/cookbooks/update_spec.rb +++ b/oc-chef-pedant/spec/api/cookbooks/update_spec.rb @@ -28,875 +28,901 @@ describe "Cookbooks API endpoint", :cookbooks, :cookbooks_update do - let(:cookbook_url_base) { "cookbooks" } + shared_examples "updates cookbooks" do |api_version| + let(:cookbook_url_base) { "cookbooks" } + let(:api_version) { api_version } - include Pedant::RSpec::CookbookUtil - - context "PUT /cookbooks// [update]" do - - let(:request_method){:PUT} - shared(:requestor){admin_user} - let(:request_url) { named_cookbook_url } - let(:cookbook_name) { "cookbook-to-be-modified" } - let(:cookbook_version) { self.class.cookbook_version } - let(:fetched_cookbook) { retrieved_cookbook(cookbook_name, cookbook_version) } - let(:original_cookbook) { new_cookbook(cookbook_name, cookbook_version) } + before do + platform.server_api_version = api_version + end - # This requires deep dup - let(:updated_cookbook) do - original_cookbook.dup.tap do |cookbook| - cookbook["metadata"] = cookbook["metadata"].dup.tap { |c| c["description"] = "hi there #{rand(10000)}" } - end + after do + platform.reset_server_api_version end - # TODO: KLUDGE: Cop-out, because I am too tired to refactor the macros correctly - def self.cookbook_version - "11.2.3" + let(:segment_type) do + select_segment("files") end - before(:each) { - make_cookbook(admin_user, cookbook_name, cookbook_version) - } - - after(:each) { - delete_cookbook(admin_user, cookbook_name, cookbook_version) - } - - respects_maximum_payload_size - - context "as admin user" do - it "should respond with 200 Ok", :smoke do - payload = new_cookbook(cookbook_name, cookbook_version) - metadata = payload["metadata"] - metadata["description"] = "hi there" - payload["metadata"] = metadata - - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - response. - should look_like({ - :status => 200, - :body_exact => payload - }) - end + include Pedant::RSpec::CookbookUtil - # verify change happened - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) - end - end # it admin user returns 200 + context "PUT /cookbooks// [update]" do - context 'as a user outside of the organization', :authorization do - let(:expected_response) { unauthorized_access_credential_response } + let(:request_method){:PUT} + shared(:requestor){admin_user} + let(:request_url) { named_cookbook_url } + let(:cookbook_name) { "cookbook-to-be-modified" } + let(:cookbook_version) { self.class.cookbook_version } + let(:fetched_cookbook) { retrieved_cookbook(cookbook_name, cookbook_version) } + let(:original_cookbook) { new_cookbook(cookbook_name, cookbook_version) } - it "should respond with 403 (\"Forbidden\") and does not update cookbook" do - put(request_url, outside_user, :payload => updated_cookbook) do |response| - response.should look_like expected_response - end - - should_not_be_updated - end # it outside user returns 403 and does not update cookbook + # This requires deep dup + let(:updated_cookbook) do + original_cookbook.dup.tap do |cookbook| + cookbook["metadata"] = cookbook["metadata"].dup.tap { |c| c["description"] = "hi there #{rand(10000)}" } + end end - context 'with invalid user', :authentication do - let(:expected_response) { invalid_credential_exact_response } - - it "returns 401 and does not update cookbook" do - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), invalid_user, :payload => updated_cookbook) do |response| - response.should look_like expected_response - end - - # Verified change did not happen - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| - response. - should look_like({ - :status => 200, - :body_exact => original_cookbook - }) - end - end # it invalid user returns 401 and does not update cookbook - end # with invalid user - end # context with permissions for + # TODO: KLUDGE: Cop-out, because I am too tired to refactor the macros correctly + def self.cookbook_version + "11.2.3" + end - context "for checksums" do - include Pedant::RSpec::CookbookUtil + before(:each) { + make_cookbook(admin_user, cookbook_name, cookbook_version) + } - let(:sandbox) { create_sandbox(files) } - let(:upload) { ->(file) { upload_to_sandbox(file, sandbox) } } - let(:files) { (0..3).to_a.map { Pedant::Utility.new_random_file } } + after(:each) { + delete_cookbook(admin_user, cookbook_name, cookbook_version) + } - let(:committed_files) do - files.each(&upload) - result = commit_sandbox(sandbox) - result - end + respects_maximum_payload_size - let(:checksums) { parse(committed_files)["checksums"] } - - it "adding all new checksums should succeed" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "files/default/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "files/default/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "files/default/name3", - "checksum" => checksums[2], - "specificity" => "default"}, - {"name" => "name4", "path" => "files/default/name4", - "checksum" => checksums[3], - "specificity" => "default"}] - - verify_checksum_cleanup(:files) do + context "as admin user" do + it "should respond with 200 Ok", :smoke do + payload = new_cookbook(cookbook_name, cookbook_version) + metadata = payload["metadata"] + metadata["description"] = "hi there" + payload["metadata"] = metadata put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, :payload => payload) do |response| response. should look_like({ - :status => 200, - :body_exact => payload - }) + :status => 200, + :body_exact => payload + }) end # verify change happened - # TODO make this match on body when URLs are parsable get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| response. should look_like({ - :status => 200 - #:body_exact => payload - }) + :status => 200, + :body => payload + }) end - end # verify_checksum_cleanup - - end # it adding all new checksums should succeed - - it "should return url when adding checksums" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "files/default/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "files/default/name2", - "checksum" => checksums[1], - "specificity" => "default"}] - - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) - end - # TODO original description indicated ruby returned URI, and also b ody_exact was commented out below. - # Look into it ... - # verify change happened - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) - end - end + end # it admin user returns 200 - it "adding invalid checksum should fail", :validation do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "files/path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "files/path/name2", - "checksum" => "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "specificity" => "default"}] - - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - - error = ["Manifest has checksum aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa but it hasn't yet been uploaded"] - response. - should look_like({ - :status => 400, - :body_exact => { - "error" => error - } - }) - end + context 'as a user outside of the organization', :authorization do + let(:expected_response) { unauthorized_access_credential_response } - # Verify change did not happen - payload.delete("files") + it "should respond with 403 (\"Forbidden\") and does not update cookbook" do + put(request_url, outside_user, :payload => updated_cookbook) do |response| + response.should look_like expected_response + end - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) + should_not_be_updated + end # it outside user returns 403 and does not update cookbook end - end # it adding invalid checksum should fail - it "deleting all checksums should succeed" do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "files/default/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "files/default/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "files/default/name3", - "checksum" => checksums[2], - "specificity" => "default"}, - {"name" => "name4", "path" => "files/default/name4", - "checksum" => checksums[3], - "specificity" => "default"}] - upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) - - # Verified initial cookbook - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200 - #:body_exact => payload - }) + context 'with invalid user', :authentication do + let(:expected_response) { invalid_credential_exact_response } + + it "returns 401 and does not update cookbook" do + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), invalid_user, :payload => updated_cookbook) do |response| + response.should look_like expected_response + end + + # Verified change did not happen + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| + response. + should look_like({ + :status => 200, + :body_exact => original_cookbook + }) + end + end # it invalid user returns 401 and does not update cookbook + end # with invalid user + end # context with permissions for + + context "for checksums" do + include Pedant::RSpec::CookbookUtil + + let(:sandbox) { create_sandbox(files) } + let(:upload) { ->(file) { upload_to_sandbox(file, sandbox) } } + let(:files) { (0..3).to_a.map { Pedant::Utility.new_random_file } } + + let(:committed_files) do + files.each(&upload) + result = commit_sandbox(sandbox) + result end - verify_checksum_cleanup(:files) do + let(:checksums) { parse(committed_files)["checksums"] } + + it "adding all new checksums should succeed" do + payload = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "files/default/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "files/default/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "files/default/name3", + "checksum" => checksums[2], + "specificity" => "default"}, + {"name" => "name4", "path" => "files/default/name4", + "checksum" => checksums[3], + "specificity" => "default"}]}) + + verify_checksum_cleanup(segment_type.to_sym) do + + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 200, + :body_exact => payload + }) + end + + # verify change happened + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end + end # verify_checksum_cleanup + + end # it adding all new checksums should succeed + + it "should return url when adding checksums" do + payload = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "files/default/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "files/default/name2", + "checksum" => checksums[1], + "specificity" => "default"}]}) - payload.delete("files") put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, :payload => payload) do |response| response. should look_like({ - :status => 200, - :body_exact => payload - }) + :status => 200, + :body => payload + }) end - + # TODO original description indicated ruby returned URI, and also b ody_exact was commented out below. + # Look into it ... # verify change happened # TODO make this match on body when URLs are parsable get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| response. should look_like({ - :status => 200 - #:body_exact => payload - }) + :status => 200, + :body => payload + }) end - end # verify_checksum_cleanup - - end # it deleting all checksums should succeed - - it "deleting some checksums should succeed" do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "path/name3", - "checksum" => checksums[2], - "specificity" => "default"}, - {"name" => "name4", "path" => "path/name4", - "checksum" => checksums[3], - "specificity" => "default"}] - - upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) - - # Verified initial cookbook - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200 - #:body_exact => payload - }) end - verify_checksum_cleanup(:files) do + it "adding invalid checksum should fail", :validation do + payload = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "files/path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "files/path/name2", + "checksum" => "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "specificity" => "default"}]}) - payload["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}] put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, :payload => payload) do |response| + + error = ["Manifest has checksum aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa but it hasn't yet been uploaded"] response. should look_like({ - :status => 200, - :body_exact => payload - }) + :status => 400, + :body_exact => { + "error" => error + } + }) end - # verify change happened + # Verify change did not happen + payload.delete(segment_type) + + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200, + :body => payload + }) + end + end # it adding invalid checksum should fail + + it "deleting all checksums should succeed" do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + payload = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "files/default/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "files/default/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "files/default/name3", + "checksum" => checksums[2], + "specificity" => "default"}, + {"name" => "name4", "path" => "files/default/name4", + "checksum" => checksums[3], + "specificity" => "default"}]}) + upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) + + # Verified initial cookbook # TODO make this match on body when URLs are parsable get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| response. should look_like({ - :status => 200 - #:body_exact => payload - }) + :status => 200 + #:body_exact => payload + }) end - end # verify_checksum_cleanup - end # it deleting some checksums should succeed - it "changing all different checksums should succeed" do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}] - upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) - - # Verified initial cookbook - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200 - #:body_exact => payload - }) - end - - verify_checksum_cleanup(:files) do + verify_checksum_cleanup(segment_type.to_sym) do + + payload.delete(segment_type) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 200, + :body_exact => payload + }) + end + + # verify change happened + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end + end # verify_checksum_cleanup + + end # it deleting all checksums should succeed + + it "deleting some checksums should succeed" do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + payload = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "path/name3", + "checksum" => checksums[2], + "specificity" => "default"}, + {"name" => "name4", "path" => "path/name4", + "checksum" => checksums[3], + "specificity" => "default"}]}) + + upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) + + # Verified initial cookbook + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end - payload["files"] = [{"name" => "name3", "path" => "path/name3", - "checksum" => checksums[2], - "specificity" => "default"}, - {"name" => "name4", "path" => "path/name4", - "checksum" => checksums[3], - "specificity" => "default"}] - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| + verify_checksum_cleanup(segment_type.to_sym) do + + payload[segment_type] = [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}] + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 200, + :body_exact => payload + }) + end + + # verify change happened + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end + end # verify_checksum_cleanup + end # it deleting some checksums should succeed + + it "changing all different checksums should succeed" do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + payload = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}]}) + upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) + + # Verified initial cookbook + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| response. should look_like({ - :status => 200, - :body_exact => payload - }) + :status => 200 + #:body_exact => payload + }) end - # verify change happened + verify_checksum_cleanup(segment_type.to_sym) do + + payload[segment_type] = [{"name" => "name3", "path" => "path/name3", + "checksum" => checksums[2], + "specificity" => "default"}, + {"name" => "name4", "path" => "path/name4", + "checksum" => checksums[3], + "specificity" => "default"}] + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 200, + :body_exact => payload + }) + end + + # verify change happened + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end + end # verify_checksum_cleanup + end # it changing all different checksums should succeed + + it "changing some different checksums should succeed" do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + payload = new_cookbook(cookbook_name, cookbook_version) + payload[segment_type] = [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "path/name3", + "checksum" => checksums[2], + "specificity" => "default"}] + upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) + + # Verified initial cookbook # TODO make this match on body when URLs are parsable get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| response. should look_like({ - :status => 200 - #:body_exact => payload - }) + :status => 200 + #:body_exact => payload + }) end - end # verify_checksum_cleanup - end # it changing all different checksums should succeed - it "changing some different checksums should succeed" do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "path/name3", - "checksum" => checksums[2], - "specificity" => "default"}] - upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) - - # Verified initial cookbook - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200 - #:body_exact => payload - }) - end + verify_checksum_cleanup(:files) do + + payload[segment_type] = [{"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "path/name3", + "checksum" => checksums[2], + "specificity" => "default"}, + {"name" => "name4", "path" => "path/name4", + "checksum" => checksums[3], + "specificity" => "default"}] + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 200, + :body_exact => payload + }) + end + + # verify change happened + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end + end # verify_checksum_cleanup + end # it changing some different checksums should succeed + + it "changing to invalid checksums should fail", :validation do + delete_cookbook(admin_user, cookbook_name, cookbook_version) - verify_checksum_cleanup(:files) do + payload = new_cookbook(cookbook_name, cookbook_version) + + # Make changes to the files in cookbook version 2. This effectively + # deletes all the old files. - payload["files"] = [{"name" => "name2", "path" => "path/name2", + payload[segment_type] = [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", "checksum" => checksums[1], "specificity" => "default"}, - {"name" => "name3", "path" => "path/name3", + {"name" => "name3", "path" => "path/name3", + "checksum" => checksums[2], + "specificity" => "default"}] + upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) + + # Verified initial cookbook + # TODO make this match on body when URLs are parsable + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200 + #:body_exact => payload + }) + end + + payload[segment_type] = [{"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "path/name3", "checksum" => checksums[2], "specificity" => "default"}, - {"name" => "name4", "path" => "path/name4", - "checksum" => checksums[3], - "specificity" => "default"}] + {"name" => "name4", "path" => "path/name4", + "checksum" => "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "specificity" => "default"}] put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user, :payload => payload) do |response| + + error = ["Manifest has checksum aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa but it hasn't yet been uploaded"] response. should look_like({ - :status => 200, - :body_exact => payload - }) + :status => 400, + :body_exact => { + "error" => error + } + }) end - # verify change happened + # verify change did not happen + payload[segment_type] = [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}, + {"name" => "name3", "path" => "path/name3", + "checksum" => checksums[2], + "specificity" => "default"}] + # TODO make this match on body when URLs are parsable get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), admin_user) do |response| response. should look_like({ - :status => 200 - #:body_exact => payload - }) + :status => 200, + #:body_exact => payload + }) + end + end # it changing to invalid checksums should fail + + # Coverge for CHEF-3716 + # + # The ultimate problem was that we were inadvertently deleting some files + # from S3 / Bookshelf on cookbook updates. When a file was no longer + # referenced by that cookbook version, we would delete it without first + # checking that it wasn't being referenced by any other cookbooks. The + # database was internally consistent (modulo some "garbage" checksums + # remaining), but it was inconsistent with S3 / Bookshelf, which resulted + # in the 404 errors when trying to download. + context "CHEF-3716 coverage" do + let(:cookbook_version2) { "11.2.4" } + + after(:each) { + delete_cookbook(admin_user, cookbook_name, cookbook_version2) + } + + it "it does not delete checksums in use by another version" do + + # Might be a little paranoid, but lets ensure the version strings + # versions are indeed different since the whole test hinges on this. + cookbook_version.should_not eq cookbook_version2 + + # Create two cookbook versions that share a single file + payload1 = new_cookbook(cookbook_name, cookbook_version, + payload: {"files" => [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[1], + "specificity" => "default"}]}) + + payload2 = new_cookbook(cookbook_name, cookbook_version2, + payload: { "files" => [{"name" => "name1", "path" => "path/name1", + "checksum" => checksums[0], + "specificity" => "default"}, + {"name" => "name2", "path" => "path/name2", + "checksum" => checksums[2], + "specificity" => "default"}]}) + + upload_cookbook(admin_user, cookbook_name, cookbook_version, payload1) + upload_cookbook(admin_user, cookbook_name, cookbook_version2, payload2) + + # compute an intersection and difference + cbv_1_checksums = checksums_for_segment_type(:files, cookbook_version) + cbv_2_checksums = checksums_for_segment_type(:files, cookbook_version2) + intersection_checksums = cbv_1_checksums.keys & cbv_2_checksums.keys + cbv_2_difference_checksums = cbv_2_checksums.keys - cbv_1_checksums.keys + + # Make changes to the files in cookbook version 2. This effectively + # deletes all the old files. + payload2[segment_type] = [{"name" => "name5", "path" => "path/name5", + "checksum" => checksums[3], + "specificity" => "default"}] + upload_cookbook(admin_user, cookbook_name, cookbook_version2, payload2) + + # Checksums unique to first iteration of cookbook version 2 should + # have been deleted + cbv_2_difference_checksums.each do |checksum| + verify_checksum_url(cbv_2_checksums[checksum], 404) + end + + # Checksums shared between the original iterations of the cookbook + # versions should still exist + intersection_checksums.each do |checksum| + verify_checksum_url(cbv_2_checksums[checksum], 200) + end end - end # verify_checksum_cleanup - end # it changing some different checksums should succeed - - it "changing to invalid checksums should fail", :validation do - delete_cookbook(admin_user, cookbook_name, cookbook_version) - - payload = new_cookbook(cookbook_name, cookbook_version) - payload["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "path/name3", - "checksum" => checksums[2], - "specificity" => "default"}] - upload_cookbook(admin_user, cookbook_name, cookbook_version, payload) - - # Verified initial cookbook - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200 - #:body_exact => payload - }) end - payload["files"] = [{"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "path/name3", - "checksum" => checksums[2], - "specificity" => "default"}, - {"name" => "name4", "path" => "path/name4", - "checksum" => "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - "specificity" => "default"}] - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - - error = ["Manifest has checksum aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa but it hasn't yet been uploaded"] - response. - should look_like({ - :status => 400, - :body_exact => { - "error" => error - } - }) - end + end # context for checksums - # verify change did not happen - payload["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}, - {"name" => "name3", "path" => "path/name3", - "checksum" => checksums[2], - "specificity" => "default"}] - - # TODO make this match on body when URLs are parsable - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200 - #:body_exact => payload - }) - end - end # it changing to invalid checksums should fail - - # Coverge for CHEF-3716 - # - # The ultimate problem was that we were inadvertently deleting some files - # from S3 / Bookshelf on cookbook updates. When a file was no longer - # referenced by that cookbook version, we would delete it without first - # checking that it wasn't being referenced by any other cookbooks. The - # database was internally consistent (modulo some "garbage" checksums - # remaining), but it was inconsistent with S3 / Bookshelf, which resulted - # in the 404 errors when trying to download. - context "CHEF-3716 coverage" do - let(:cookbook_version2) { "11.2.4" } - - after(:each) { - delete_cookbook(admin_user, cookbook_name, cookbook_version2) - } - - it "it does not delete checksums in use by another version" do - - # Might be a little paranoid, but lets ensure the version strings - # versions are indeed different since the whole test hinges on this. - cookbook_version.should_not eq cookbook_version2 - - # Create two cookbook versions that share a single file - payload1 = new_cookbook(cookbook_name, cookbook_version) - payload1["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[1], - "specificity" => "default"}] - payload2 = new_cookbook(cookbook_name, cookbook_version2) - payload2["files"] = [{"name" => "name1", "path" => "path/name1", - "checksum" => checksums[0], - "specificity" => "default"}, - {"name" => "name2", "path" => "path/name2", - "checksum" => checksums[2], - "specificity" => "default"}] - upload_cookbook(admin_user, cookbook_name, cookbook_version, payload1) - upload_cookbook(admin_user, cookbook_name, cookbook_version2, payload2) - - # compute an intersection and difference - cbv_1_checksums = checksums_for_segment_type(:files, cookbook_version) - cbv_2_checksums = checksums_for_segment_type(:files, cookbook_version2) - intersection_checksums = cbv_1_checksums.keys & cbv_2_checksums.keys - cbv_2_difference_checksums = cbv_2_checksums.keys - cbv_1_checksums.keys + context "for frozen?" do + before(:each) do + payload = new_cookbook(cookbook_name, cookbook_version) + payload["frozen?"] = true - # Make changes to the files in cookbook version 2. This effectively - # deletes all the old files. - payload2["files"] = [{"name" => "name5", "path" => "path/name5", - "checksum" => checksums[3], - "specificity" => "default"}] - upload_cookbook(admin_user, cookbook_name, cookbook_version2, payload2) - - # Checksums unique to first iteration of cookbook version 2 should - # have been deleted - cbv_2_difference_checksums.each do |checksum| - verify_checksum_url(cbv_2_checksums[checksum], 404) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 200, + :body_exact => payload + }) end + end # before :each - # Checksums shared between the original iterations of the cookbook - # versions should still exist - intersection_checksums.each do |checksum| - verify_checksum_url(cbv_2_checksums[checksum], 200) + it "can set frozen? to true" do + payload = new_cookbook(cookbook_name, cookbook_version) + payload["frozen?"] = true + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200, + :body => payload + }) end - end - end + end # it can set frozen? to true - end # context for checksums + it "can not edit cookbook when frozen? is set to true" do + payload = new_cookbook(cookbook_name, cookbook_version) + payload["frozen?"] = false + metadata = payload["metadata"] + metadata["description"] = "this is different" + payload["metadata"] = metadata - context "for frozen?" do - before(:each) do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = true + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user, :payload => payload) do |response| + response. + should look_like({ + :status => 409, + :body_exact => { + "error" => ["The cookbook #{cookbook_name} at version #{cookbook_version} is frozen. Use the 'force' option to override."] + } + }) + end - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - response. - should look_like({ - :status => 200, - :body_exact => payload - }) - end - end # before :each - - it "can set frozen? to true" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = true - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) - end - end # it can set frozen? to true - - it "can not edit cookbook when frozen? is set to true" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = false - metadata = payload["metadata"] - metadata["description"] = "this is different" - payload["metadata"] = metadata - - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user, :payload => payload) do |response| - response. - should look_like({ - :status => 409, - :body_exact => { - "error" => ["The cookbook #{cookbook_name} at version #{cookbook_version} is frozen. Use the 'force' option to override."] - } - }) - end + # Verify that change did not occur + payload = new_cookbook(cookbook_name, cookbook_version) + payload["frozen?"] = true + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200, + :body => payload + }) + end + end # it can not edit cookbook when frozen? is set to true + + it "can override frozen? with force set to true" do + payload = new_cookbook(cookbook_name, cookbook_version) + payload["frozen?"] = false + metadata = payload["metadata"] + metadata["description"] = "this is different" + payload["metadata"] = metadata - # Verify that change did not occur - payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = true - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}?force=true"), + admin_user, :payload => payload) do |response| + # You can modify things, but you can't unfreeze the cookbook + payload["frozen?"] = true + response. + should look_like({ + :status => 200, + :body_exact => payload + }) end - end # it can not edit cookbook when frozen? is set to true - - it "can override frozen? with force set to true" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = false - metadata = payload["metadata"] - metadata["description"] = "this is different" - payload["metadata"] = metadata - - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}?force=true"), - admin_user, :payload => payload) do |response| - # You can modify things, but you can't unfreeze the cookbook - payload["frozen?"] = true - response. - should look_like({ - :status => 200, - :body_exact => payload - }) - end - # Verify that change did occur - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| - response. - should look_like({ - :status => 200, - :body => payload - }) - end - end # it can override frozen? with force set to true - - it "can not override frozen? with force set to false" do - payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = false - metadata = payload["metadata"] - metadata["description"] = "this is different" - payload["metadata"] = metadata - - put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}?force=false"), - admin_user, :payload => payload) do |response| - # You can modify things, but you can't unfreeze the cookbook - payload["frozen?"] = true - response. - should look_like({ - :status => 409, - :body_exact => { - "error" => ["The cookbook #{cookbook_name} at version #{cookbook_version} is frozen. Use the 'force' option to override."] - } - }) - end + # Verify that change did occur + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + response. + should look_like({ + :status => 200, + :body => payload + }) + end + end # it can override frozen? with force set to true - # Verify that change did occur - get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), - admin_user) do |response| + it "can not override frozen? with force set to false" do payload = new_cookbook(cookbook_name, cookbook_version) - payload["frozen?"] = true - response. - should look_like({ - :status => 200, - :body => payload - }) - end - end # it can not override frozen? with force set to false - end # context for frozen? + payload["frozen?"] = false + metadata = payload["metadata"] + metadata["description"] = "this is different" + payload["metadata"] = metadata - context "when modifying data" do - context "for cookbook_name" do - [1, true, [], {}].each do |value| - should_fail_to_change('cookbook_name', value, 400, "Field 'cookbook_name' invalid") - end - ['new_cookbook_name', 'with a space', '外国語'].each do |value| - should_fail_to_change('cookbook_name', value, 400, "Field 'cookbook_name' invalid") - end - should_fail_to_change('cookbook_name', :delete, 400, "Field 'cookbook_name' missing") - end # context for cookbook_name - - context "for json_class" do - should_not_change('json_class', :delete, 'Chef::CookbookVersion') - should_fail_to_change('json_class', 1, 400, "Field 'json_class' invalid") - should_fail_to_change('json_class', 'Chef::NonCookbook', 400, "Field 'json_class' invalid") - should_fail_to_change('json_class', 'all wrong', 400, "Field 'json_class' invalid") - end # context for json_class - - context "for chef_type" do - should_not_change('chef_type', :delete, 'cookbook_version') - should_fail_to_change('chef_type', 'not_cookbook', 400, "Field 'chef_type' invalid") - should_fail_to_change('chef_type', false, 400, "Field 'chef_type' invalid") - should_fail_to_change('chef_type', ['just any', 'old junk'], 400, "Field 'chef_type' invalid") - end # context for chef_type - - context "for version" do - should_change('version', :delete) - error = "Field 'version' invalid" - should_fail_to_change('version', 1, 400, error) - should_fail_to_change('version', ['all', 'ignored'], 400, error) - should_fail_to_change('version', {}, 400, error) - - error = "Field 'version' invalid" - should_fail_to_change('version', '0.0', 400, error) - should_fail_to_change('version', 'something invalid', 400, error) - end # context for version - - context "for collections" do - ['attributes', 'definitions', 'files', 'libraries', 'providers', 'recipes', - 'resources', 'root_files', 'templates'].each do |segment| - context "for #{segment}" do - should_fail_to_change(segment, 'foo', 400, "Field '#{segment}' invalid") - error = "Invalid element in array value of '#{segment}'." - should_fail_to_change(segment, ['foo'], 400, error) - should_change(segment, []) - should_fail_to_change(segment, [{}, {}], 400, error) - should_fail_to_change(segment, [{'foo' => 'bar'}], 400, error) - end # context for #{segment} - end # [loop over attributes, definitions, files, libraries, providers, + put(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}?force=false"), + admin_user, :payload => payload) do |response| + # You can modify things, but you can't unfreeze the cookbook + payload["frozen?"] = true + response. + should look_like({ + :status => 409, + :body_exact => { + "error" => ["The cookbook #{cookbook_name} at version #{cookbook_version} is frozen. Use the 'force' option to override."] + } + }) + end + + # Verify that change did occur + get(api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}"), + admin_user) do |response| + payload = new_cookbook(cookbook_name, cookbook_version) + payload["frozen?"] = true + response. + should look_like({ + :status => 200, + :body => payload + }) + end + end # it can not override frozen? with force set to false + end # context for frozen? + + context "when modifying data" do + context "for cookbook_name" do + [1, true, [], {}].each do |value| + should_fail_to_change('cookbook_name', value, 400, "Field 'cookbook_name' invalid") + end + ['new_cookbook_name', 'with a space', '外国語'].each do |value| + should_fail_to_change('cookbook_name', value, 400, "Field 'cookbook_name' invalid") + end + should_fail_to_change('cookbook_name', :delete, 400, "Field 'cookbook_name' missing") + end # context for cookbook_name + + context "for json_class" do + should_not_change('json_class', :delete, 'Chef::CookbookVersion') + should_fail_to_change('json_class', 1, 400, "Field 'json_class' invalid") + should_fail_to_change('json_class', 'Chef::NonCookbook', 400, "Field 'json_class' invalid") + should_fail_to_change('json_class', 'all wrong', 400, "Field 'json_class' invalid") + end # context for json_class + + context "for chef_type" do + should_not_change('chef_type', :delete, 'cookbook_version') + should_fail_to_change('chef_type', 'not_cookbook', 400, "Field 'chef_type' invalid") + should_fail_to_change('chef_type', false, 400, "Field 'chef_type' invalid") + should_fail_to_change('chef_type', ['just any', 'old junk'], 400, "Field 'chef_type' invalid") + end # context for chef_type + + context "for version" do + should_change('version', :delete) + error = "Field 'version' invalid" + should_fail_to_change('version', 1, 400, error) + should_fail_to_change('version', ['all', 'ignored'], 400, error) + should_fail_to_change('version', {}, 400, error) + + error = "Field 'version' invalid" + should_fail_to_change('version', '0.0', 400, error) + should_fail_to_change('version', 'something invalid', 400, error) + end # context for version + + context "for collections", if: api_version.to_i < 2 do + ['attributes', 'definitions', 'files', 'libraries', 'providers', 'recipes', + 'resources', 'root_files', 'templates'].each do |segment| + context "for #{segment}" do + should_fail_to_change(segment, 'foo', 400, "Field '#{segment}' invalid") + error = "Invalid element in array value of '#{segment}'." + should_fail_to_change(segment, ['foo'], 400, error) + should_change(segment, []) + should_fail_to_change(segment, [{}, {}], 400, error) + should_fail_to_change(segment, [{'foo' => 'bar'}], 400, error) + end # context for #{segment} + end # [loop over attributes, definitions, files, libraries, providers, # recipes, resources, root_files, templates - end # context for collections - - context "for other stuff" do - should_change('frozen?', true) - end # context for other stuff - end # context when modifying data - - context "when modifying metadata" do - should_fail_to_change('metadata', {'new_name' => 'foo'}, 400, "Field 'metadata.version' missing") - - context "for name" do - should_change_metadata('name', 'new_name', nil, 200, :validation) - should_change_metadata('name', :delete) - [[1, 'number'], [true, 'boolean'], [{}, 'object'], - [[], 'array']].each do |error| - json_error = "Field 'metadata.name' invalid" - should_fail_to_change_metadata('name', error[0], 400, json_error) - end - ['invalid name', 'ダメよ'].each do |name| - should_fail_to_change_metadata('name', name, 400, "Field 'metadata.name' invalid") - end - end # context for name - - context "for description" do - should_change_metadata('description', 'new description') - should_change_metadata('description', :delete) - should_fail_to_change_metadata('description', 1, 400, "Field 'metadata.description' invalid") - end # context for description - - context "for long description" do - should_change_metadata('long_description', 'longer description') - - # Deleting the long description results in it being "reset" to - # the empty string - should_change_metadata('long_description', :delete, "") - should_fail_to_change_metadata('long_description', false, 400, "Field 'metadata.long_description' invalid") - end # context for long description - - context "for version" do - should_fail_to_change_metadata('version', '0.0', 400, "Field 'metadata.version' invalid") - should_fail_to_change_metadata('version', 'not a version', 400, "Field 'metadata.version' invalid") - should_fail_to_change_metadata('version', :delete, 400, "Field 'metadata.version' missing") - should_fail_to_change_metadata('version', 1, 400, "Field 'metadata.version' invalid") - end # context for version - - context "for maintainer" do - should_change_metadata('maintainer', 'Captain Stupendous') - should_change_metadata('maintainer', :delete) - should_fail_to_change_metadata('maintainer', true, 400, "Field 'metadata.maintainer' invalid") - should_change_metadata('maintainer_email', 'cap@awesome.com') - should_change_metadata('maintainer_email', 'not really an email') - should_change_metadata('maintainer_email', :delete) - should_fail_to_change_metadata('maintainer_email', false, 400, "Field 'metadata.maintainer_email' invalid") - end # context for maintainer - - context "for license" do - should_change_metadata('license', 'to_kill') - should_change_metadata('license', :delete) - should_fail_to_change_metadata('license', 1, 400, "Field 'metadata.license' invalid") - end # context for license - - context "for collections" do - context "for platforms" do - json_error = "Field 'metadata.platforms' invalid" - should_fail_to_change_metadata('platforms', [], 400, json_error) - should_change_metadata('platforms', {}) - should_change_metadata('platforms', :delete) - should_fail_to_change_metadata('platforms', "foo", 400, json_error) - should_fail_to_change_metadata('platforms', ["foo"], 400, json_error) - should_fail_to_change_metadata('platforms', {"foo" => {}}, 400, "Invalid value '{[]}' for metadata.platforms") - end + end # context for collections + + context "for other stuff" do + should_change('frozen?', true) + end # context for other stuff + end # context when modifying data + + context "when modifying metadata" do + should_fail_to_change('metadata', {'new_name' => 'foo'}, 400, "Field 'metadata.version' missing") + + context "for name" do + should_change_metadata('name', 'new_name', nil, 200, :validation) + should_change_metadata('name', :delete) + [[1, 'number'], [true, 'boolean'], [{}, 'object'], + [[], 'array']].each do |error| + json_error = "Field 'metadata.name' invalid" + should_fail_to_change_metadata('name', error[0], 400, json_error) + end + ['invalid name', 'ダメよ'].each do |name| + should_fail_to_change_metadata('name', name, 400, "Field 'metadata.name' invalid") + end + end # context for name + + context "for description" do + should_change_metadata('description', 'new description') + should_change_metadata('description', :delete) + should_fail_to_change_metadata('description', 1, 400, "Field 'metadata.description' invalid") + end # context for description + + context "for long description" do + should_change_metadata('long_description', 'longer description') + + # Deleting the long description results in it being "reset" to + # the empty string + should_change_metadata('long_description', :delete, "") + should_fail_to_change_metadata('long_description', false, 400, "Field 'metadata.long_description' invalid") + end # context for long description + + context "for version" do + should_fail_to_change_metadata('version', '0.0', 400, "Field 'metadata.version' invalid") + should_fail_to_change_metadata('version', 'not a version', 400, "Field 'metadata.version' invalid") + should_fail_to_change_metadata('version', :delete, 400, "Field 'metadata.version' missing") + should_fail_to_change_metadata('version', 1, 400, "Field 'metadata.version' invalid") + end # context for version + + context "for maintainer" do + should_change_metadata('maintainer', 'Captain Stupendous') + should_change_metadata('maintainer', :delete) + should_fail_to_change_metadata('maintainer', true, 400, "Field 'metadata.maintainer' invalid") + should_change_metadata('maintainer_email', 'cap@awesome.com') + should_change_metadata('maintainer_email', 'not really an email') + should_change_metadata('maintainer_email', :delete) + should_fail_to_change_metadata('maintainer_email', false, 400, "Field 'metadata.maintainer_email' invalid") + end # context for maintainer + + context "for license" do + should_change_metadata('license', 'to_kill') + should_change_metadata('license', :delete) + should_fail_to_change_metadata('license', 1, 400, "Field 'metadata.license' invalid") + end # context for license + + context "for collections" do + context "for platforms" do + json_error = "Field 'metadata.platforms' invalid" + should_fail_to_change_metadata('platforms', [], 400, json_error) + should_change_metadata('platforms', {}) + should_change_metadata('platforms', :delete) + should_fail_to_change_metadata('platforms', "foo", 400, json_error) + should_fail_to_change_metadata('platforms', ["foo"], 400, json_error) + should_fail_to_change_metadata('platforms', {"foo" => {}}, 400, "Invalid value '{[]}' for metadata.platforms") + end - def self.should_change_with_metadata(_attribute, _value) - context "when #{_attribute} is set to #{_value}" do - let(:cookbook_name) { Pedant::Utility.with_unique_suffix("pedant-cookbook") } - # These macros need to be refactored and updated for flexibility. - # The cookbook endpoint uses PUT for both create and update, so this - # throws a monkey wrench into the mix. - should_change_metadata _attribute, _value, _value, 200 + def self.should_change_with_metadata(_attribute, _value) + context "when #{_attribute} is set to #{_value}" do + let(:cookbook_name) { Pedant::Utility.with_unique_suffix("pedant-cookbook") } + # These macros need to be refactored and updated for flexibility. + # The cookbook endpoint uses PUT for both create and update, so this + # throws a monkey wrench into the mix. + should_change_metadata _attribute, _value, _value, 200 + end end - end - context "with metadata.providing" do - # In erchef, we are not validating the "providing" metadata - # See: http://tickets.chef.io/browse/CHEF-3976 - - after(:each) { delete_cookbook admin_user, cookbook_name, cookbook_version } - - # http://docs.chef.io/config_rb_metadata.html#provides - should_change_with_metadata 'providing', 'cats::sleep' - should_change_with_metadata 'providing', 'here(:kitty, :time_to_eat)' - should_change_with_metadata 'providing', 'service[snuggle]' - should_change_with_metadata 'providing', '' - should_change_with_metadata 'providing', 1 - should_change_with_metadata 'providing', true - should_change_with_metadata 'providing', ['cats', 'sleep', 'here'] - should_change_with_metadata 'providing', - { 'cats::sleep' => '0.0.1', - 'here(:kitty, :time_to_eat)' => '0.0.1', - 'service[snuggle]' => '0.0.1' } + context "with metadata.providing" do + # In erchef, we are not validating the "providing" metadata + # See: http://tickets.chef.io/browse/CHEF-3976 + + after(:each) { delete_cookbook admin_user, cookbook_name, cookbook_version } + + # http://docs.chef.io/config_rb_metadata.html#provides + should_change_with_metadata 'providing', 'cats::sleep' + should_change_with_metadata 'providing', 'here(:kitty, :time_to_eat)' + should_change_with_metadata 'providing', 'service[snuggle]' + should_change_with_metadata 'providing', '' + should_change_with_metadata 'providing', 1 + should_change_with_metadata 'providing', true + should_change_with_metadata 'providing', ['cats', 'sleep', 'here'] + should_change_with_metadata 'providing', + { 'cats::sleep' => '0.0.1', + 'here(:kitty, :time_to_eat)' => '0.0.1', + 'service[snuggle]' => '0.0.1' } - end + end context "for dependencies" do json_error = "Field 'metadata.dependencies' invalid" should_fail_to_change_metadata("dependencies", [], 400, json_error) should_change_metadata("dependencies", {}) - # Attempting to delete dependencies will result in it - # getting set to an empty hash, since we need to have - # something present for that key should_change_metadata("dependencies", :delete, {}) should_fail_to_change_metadata("dependencies", "foo", 400, json_error) should_fail_to_change_metadata("dependencies", ["foo"], 400, json_error) should_fail_to_change_metadata("dependencies", {"foo" => {}}, 400, "Invalid value '{[]}' for metadata.dependencies") - end # [loop over dependencies, recommendations, suggestions, - # conflicting, replacing] - end # context for collections - end # context when modifying metadata - end # context PUT /cookbooks// [update] + end # context for #{type} + end # context for collections + end # context when modifying metadata + end # context PUT /cookbooks// [update] + end + + describe "API v0" do + it_behaves_like "updates cookbooks", 0 + end + + describe "API v2" do + it_behaves_like "updates cookbooks", 2 + end + end diff --git a/oc-chef-pedant/spec/api/cookbooks/version_conversion_spec.rb b/oc-chef-pedant/spec/api/cookbooks/version_conversion_spec.rb new file mode 100644 index 00000000000..8d391768c1e --- /dev/null +++ b/oc-chef-pedant/spec/api/cookbooks/version_conversion_spec.rb @@ -0,0 +1,116 @@ +# -*- coding: utf-8 -*- +# Copyright: Copyright (c) 2018 Chef Software, Inc. +# License: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require "pedant/rspec/cookbook_util" + +describe "Cookbooks API endpoint", :cookbooks, :cookbooks_conversion do + let(:cookbook_url_base) { "cookbooks" } + + before do + platform.reset_server_api_version + end + + after do + platform.reset_server_api_version + end + + include Pedant::RSpec::CookbookUtil + + context "requests with different API versions" do + include Pedant::RSpec::Validations::Create + + let(:sandbox) { create_sandbox(files) } + let(:upload) { ->(file) { upload_to_sandbox(file, sandbox) } } + let(:files) { (0..3).to_a.map { Pedant::Utility.new_random_file } } + + let(:committed_files) do + files.each(&upload) + result = commit_sandbox(sandbox) + result + end + + let(:checksums) { parse(committed_files)["checksums"] } + + let(:request_method) { :PUT } + shared(:requestor) { admin_user } + let(:request_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/#{cookbook_version}") } + let(:cookbook_name) { "vconv" } + let(:cookbook_version) { self.class.cookbook_version } + let(:cookbook_payload) do + { + "recipes" => [{ + "name" => "default.rb", + "path" => "recipes/default.rb", + "checksum" => checksums[0], + "specificity" => "default" + }], + "root_files" => [{ + "name" => "CHANGELOG", + "path" => "CHANGELOG", + "checksum" => checksums[1], + "specificity" => "default", + }] + } + end + let(:cb_v0) { new_cookbook_v0(cookbook_name, cookbook_version, payload: cookbook_payload) } + let(:cb_v2) { new_cookbook_v2(cookbook_name, cookbook_version, payload: cookbook_payload) } + + after do + delete_cookbook(admin_user, cookbook_name, cookbook_version) + end + + # TODO: KLUDGE: Cop-out, because I am too tired to refactor the macros correctly + def self.cookbook_version + "1.2.3" + end + + it "uploads as v0, downloads as v2" do + platform.server_api_version = 0 + put(request_url, admin_user, payload: cb_v0) do |response| + response.should look_like({ + status: 201, + body: cb_v0, + }) + end + + platform.server_api_version = 2 + get(request_url, admin_user) do |response| + response.should look_like({ + status: 200, + body: cb_v2, + }) + end + end + + it "uploads as v2, downloads as v0" do + platform.server_api_version = 2 + put(request_url, admin_user, payload: cb_v2) do |response| + response.should look_like({ + status: 201, + body: cb_v2, + }) + end + + platform.server_api_version = 0 + get(request_url, admin_user) do |response| + response.should look_like({ + status: 200, + body: cb_v0, + }) + end + end + end +end diff --git a/oc-chef-pedant/spec/api/cookbooks/version_spec.rb b/oc-chef-pedant/spec/api/cookbooks/version_spec.rb index 06e5d9d8481..5f462784df5 100644 --- a/oc-chef-pedant/spec/api/cookbooks/version_spec.rb +++ b/oc-chef-pedant/spec/api/cookbooks/version_spec.rb @@ -17,100 +17,126 @@ describe "Cookbook Versions API endpoint, GET", :cookbooks, :cookbooks_version do - let(:cookbook_url_base) { "cookbooks" } + shared_examples "versions cookbooks" do |api_version| + let(:cookbook_url_base) { "cookbooks" } + let(:api_version) { api_version } - include Pedant::RSpec::CookbookUtil + before do + platform.server_api_version = api_version + end - let(:request_method) { :GET } - let(:request_url) { named_cookbook_url } - let(:requestor) { admin_user } + after do + platform.reset_server_api_version + end - let(:non_existent_cookbook){ "fakecookbook" } - let(:fake_version){ "1.0.0" } - let(:latest_cookbook_version_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/_latest") } + let(:segment_type) do + if api_version.to_i < 2 + "files" + else + "all_files" + end + end - let(:fetch_cookbook_version_success_response) do - { - :status => 200, - :body => retrieved_cookbook(cookbook_name, cookbook_version) - } - end - let(:cookbook_version_not_found_exact_response) do - { - :status => 404, - :body_exact => { "error" => ["Cannot find a cookbook named #{cookbook_name} with version #{cookbook_version}"] } - } - end + include Pedant::RSpec::CookbookUtil - context "with no cookbooks on the server" do - let(:expected_response) { cookbook_version_not_found_exact_response } - let(:cookbook_name) { non_existent_cookbook } - let(:cookbook_version) { fake_version } + let(:request_method) { :GET } + let(:request_url) { named_cookbook_url } + let(:requestor) { admin_user } - should_respond_with 404 - end # with no cookbooks on the server + let(:non_existent_cookbook){ "fakecookbook" } + let(:fake_version){ "1.0.0" } + let(:latest_cookbook_version_url) { api_url("/#{cookbook_url_base}/#{cookbook_name}/_latest") } - context "with cookbooks on the server" do + let(:fetch_cookbook_version_success_response) do + { + :status => 200, + :body => retrieved_cookbook(cookbook_name, cookbook_version) + } + end + let(:cookbook_version_not_found_exact_response) do + { + :status => 404, + :body_exact => { "error" => ["Cannot find a cookbook named #{cookbook_name} with version #{cookbook_version}"] } + } + end - let(:cookbook_name) { existing_cookbook_name } - let(:cookbook_version) { existing_cookbook_version } - let(:existing_cookbook_name){ "the_art_of_french_cooking" } - let(:existing_cookbook_version){ "1.0.0" } - let(:latest){ "1.0.1" } + context "with no cookbooks on the server" do + let(:expected_response) { cookbook_version_not_found_exact_response } + let(:cookbook_name) { non_existent_cookbook } + let(:cookbook_version) { fake_version } - before :each do - make_cookbook(admin_user, existing_cookbook_name, existing_cookbook_version) - # Erchef doesn't handle multipe versions yet - make_cookbook(admin_user, existing_cookbook_name, latest) - end + should_respond_with 404 + end # with no cookbooks on the server - after :each do - delete_cookbook(admin_user, existing_cookbook_name, existing_cookbook_version) - delete_cookbook(admin_user, existing_cookbook_name, latest) - end + context "with cookbooks on the server" do - context 'when fetching existing version of cookbook' do - let(:expected_response) { fetch_cookbook_version_success_response } + let(:cookbook_name) { existing_cookbook_name } + let(:cookbook_version) { existing_cookbook_version } + let(:existing_cookbook_name){ "the_art_of_french_cooking" } + let(:existing_cookbook_version){ "1.0.0" } + let(:latest){ "1.0.1" } - should_respond_with 200, 'and the cookbook version' - end + before :each do + make_cookbook(admin_user, existing_cookbook_name, existing_cookbook_version) + # Erchef doesn't handle multipe versions yet + make_cookbook(admin_user, existing_cookbook_name, latest) + end - context 'when fetching non-existant version of cookbook' do - let(:expected_response) { cookbook_version_not_found_exact_response } - let(:cookbook_version) { '6.6.6' } + after :each do + delete_cookbook(admin_user, existing_cookbook_name, existing_cookbook_version) + delete_cookbook(admin_user, existing_cookbook_name, latest) + end - should_respond_with 404 - end + context 'when fetching existing version of cookbook' do + let(:expected_response) { fetch_cookbook_version_success_response } - context 'as a non-admin user' do - let(:expected_response) { fetch_cookbook_version_success_response } - let(:requestor) { normal_user } + should_respond_with 200, 'and the cookbook version' + end - should_respond_with 200, 'and the cookbook version' - end + context 'when fetching non-existant version of cookbook' do + let(:expected_response) { cookbook_version_not_found_exact_response } + let(:cookbook_version) { '6.6.6' } - context 'as a user outside the organization', :authorization do - let(:expected_response) { unauthorized_access_credential_response } - let(:requestor) { outside_user } + should_respond_with 404 + end - should_respond_with 403 - end + context 'as a non-admin user' do + let(:expected_response) { fetch_cookbook_version_success_response } + let(:requestor) { normal_user } - context "when requesting the 'latest' Cookbook version", :smoke do - let(:expected_response) { fetch_cookbook_version_success_response } - let(:request_url) { latest_cookbook_version_url } - let(:cookbook_version) { latest } + should_respond_with 200, 'and the cookbook version' + end - should_respond_with 200, 'and the latest cookbook version' - end # when requesting the 'latest' cookbook version + context 'as a user outside the organization', :authorization do + let(:expected_response) { unauthorized_access_credential_response } + let(:requestor) { outside_user } - context "when requesting the 'latest' version of a non-existent cookbook" do - let(:expected_response) { cookbook_version_not_found_exact_response } - let(:cookbook_name) { non_existent_cookbook } - let(:cookbook_version) { '_latest' } + should_respond_with 403 + end - should_respond_with 404 - end # when requesting the 'latest' version of a non-existent cookbook - end # with existing cookbook -end + context "when requesting the 'latest' Cookbook version", :smoke do + let(:expected_response) { fetch_cookbook_version_success_response } + let(:request_url) { latest_cookbook_version_url } + let(:cookbook_version) { latest } + + should_respond_with 200, 'and the latest cookbook version' + end # when requesting the 'latest' cookbook version + context "when requesting the 'latest' version of a non-existent cookbook" do + let(:expected_response) { cookbook_version_not_found_exact_response } + let(:cookbook_name) { non_existent_cookbook } + let(:cookbook_version) { '_latest' } + + should_respond_with 404 + end # when requesting the 'latest' version of a non-existent cookbook + end # with existing cookbook + end + + describe "API v0" do + it_behaves_like "versions cookbooks", 0 + end + + describe "API v2" do + it_behaves_like "versions cookbooks", 2 + end +end diff --git a/src/oc_erchef/apps/chef_objects/src/chef_cookbook_version.erl b/src/oc_erchef/apps/chef_objects/src/chef_cookbook_version.erl index 8c265e4d52a..3605e748ce2 100644 --- a/src/oc_erchef/apps/chef_objects/src/chef_cookbook_version.erl +++ b/src/oc_erchef/apps/chef_objects/src/chef_cookbook_version.erl @@ -27,24 +27,28 @@ -export([ assemble_cookbook_ejson/2, - annotate_with_s3_urls/3, + assemble_cookbook_ejson/3, + annotate_with_urls/3, authz_id/1, base_cookbook_name/1, constraint_map_spec/1, single_cookbook_version_spec/0, valid_cookbook_constraint/1, ejson_for_indexing/2, + ensure_v0_metadata/1, + ensure_v2_metadata/1, extract_checksums/1, fields_for_update/1, fields_for_insert/1, fields_for_fetch/1, id/1, is_indexed/1, - minimal_cookbook_ejson/2, + minimal_cookbook_ejson/3, name/1, org_id/1, new_record/4, parse_binary_json/2, + parse_binary_json/3, parse_version/1, qualified_recipe_names/2, record_fields/1, @@ -54,6 +58,7 @@ type_name/1, update_from_ejson/2, version_to_binary/1, + wants_all_files/1, list/2 ]). @@ -87,6 +92,10 @@ {<<"frozen?">>, false} ]). +-define(VALID_KEYS_V2, + [<<"all_files">>, <<"chef_type">>, <<"cookbook_name">>, <<"frozen?">>, + <<"json_class">>, <<"metadata">>, <<"name">>, <<"version">>]). + -define(VALID_KEYS, [<<"attributes">>, <<"chef_type">>, <<"cookbook_name">>, <<"definitions">>, <<"files">>, <<"frozen?">>, <<"json_class">>, <<"libraries">>, @@ -186,15 +195,17 @@ compress_maybe(Data, Type) -> %% @doc Convert a binary JSON string representing a Chef Cookbook Version into an %% EJson-encoded Erlang data structure. %% @end --spec parse_binary_json(binary(), {binary(), binary()}) -> { ok, ejson_term() }. % or throw -parse_binary_json(Bin, {UrlName, UrlVersion}) -> +-spec parse_binary_json(binary(), {binary(), binary()}, boolean()) -> { ok, ejson_term() }. % or throw +parse_binary_json(Bin, {UrlName, UrlVersion}, AllFiles) -> %% avoid parsing the cookbook JSON if name or version from URL is invalid valid_name(UrlName), valid_version(UrlVersion), Cookbook0 = chef_json:decode(Bin), Cookbook = set_default_values(Cookbook0), - validate_cookbook(Cookbook, {UrlName, UrlVersion}). + validate_cookbook(Cookbook, {UrlName, UrlVersion}, AllFiles). +parse_binary_json(Bin, {UrlName, UrlVersion}) -> + parse_binary_json(Bin, {UrlName, UrlVersion}, false). %% TODO: merge set_default_values and validate_role? @@ -205,11 +216,19 @@ set_default_values(Cookbook) -> chef_object_base:set_default_values(Cookbook, ?DEFAULT_FIELD_VALUES). -spec validate_cookbook(Cookbook :: ej:json_object(), - {UrlName :: binary(), - UrlVersion :: binary()}) -> {ok, ej:json_object()}. -validate_cookbook(Cookbook, {UrlName, UrlVersion}) -> + {UrlName :: binary(), UrlVersion :: binary()}, + AllFiles :: boolean()) -> {ok, ej:json_object()}. +validate_cookbook(Cookbook, {UrlName, UrlVersion}, AllFiles) when AllFiles =:= false -> + Spec = cookbook_spec(UrlName, UrlVersion), + case chef_object_base:strictly_valid(Spec, ?VALID_KEYS, Cookbook) of + ok -> {ok, Cookbook}; + Bad -> throw(Bad) + end; +validate_cookbook(Cookbook, {UrlName, UrlVersion}, _) -> %% WARNING: UrlName and UrlVersion are assumed to be valid - case chef_object_base:strictly_valid(cookbook_spec(UrlName, UrlVersion), ?VALID_KEYS, Cookbook) of + Spec = cookbook_spec_v2(UrlName, UrlVersion), + + case chef_object_base:strictly_valid(Spec, ?VALID_KEYS_V2, Cookbook) of ok -> {ok, Cookbook}; Bad -> throw(Bad) end. @@ -271,6 +290,31 @@ cookbook_spec(CBName, CBVersion) -> {{opt, <<"version">>}, CBVersion} ]}. +cookbook_spec_v2(CBName, CBVersion) -> + {[ + {<<"name">>, <>}, + {<<"cookbook_name">>, CBName}, + {<<"json_class">>, <<"Chef::CookbookVersion">>}, + {<<"chef_type">>, <<"cookbook_version">>}, + {<<"metadata">>, {[ + {<<"version">>, CBVersion}, + {{opt, <<"name">>}, {string_match, chef_regex:regex_for(cookbook_name)}}, + {{opt, <<"description">>}, string}, + {{opt, <<"long_description">>}, string}, + {{opt, <<"maintainer">>}, string}, + %% do we want to regex on email address or at least verify that we + %% have a '@'? + {{opt, <<"maintainer_email">>}, string}, + {{opt, <<"license">>}, string}, + + {{opt, <<"platforms">>}, constraint_map_spec(cookbook_name)}, + + {{opt, <<"dependencies">>}, constraint_map_spec(cookbook_name)} + ]}}, + {{opt, <<"all_files">>}, file_list_spec()}, + {{opt, <<"version">>}, CBVersion} + ]}. + file_list_spec() -> {array_map, {[{<<"name">>, string}, {<<"path">>, string}, @@ -290,10 +334,11 @@ file_list_spec() -> -spec extract_checksums(ejson_term()) -> [binary()]. extract_checksums(CBJson) -> + Segments = [ <<"all_files">> | ?COOKBOOK_SEGMENTS ], Sums = [ begin Segment = ej:get({SegName}, CBJson, []), extract_checksums_from_segment(Segment) - end || SegName <- ?COOKBOOK_SEGMENTS ], + end || SegName <- Segments ], lists:usort(lists:append(Sums)). extract_checksums_from_segment(Segment) -> @@ -398,7 +443,7 @@ version_to_binary({Major, Minor, Patch}) -> %% %% The Ejson structure is a 1-1 mapping from the #chef_cookbook_version{} and does %% not contain any extra information, e.g. S3 URLs. --spec assemble_cookbook_ejson(#chef_cookbook_version{}, string()) -> ejson_term(). +-spec assemble_cookbook_ejson(#chef_cookbook_version{}, string(), boolean()) -> ejson_term(). assemble_cookbook_ejson(#chef_cookbook_version{ org_id = OrgId, frozen=Frozen, @@ -406,7 +451,7 @@ assemble_cookbook_ejson(#chef_cookbook_version{ metadata=XMetadataJSON, meta_attributes=XMetaAttributesJSON, meta_long_desc=XLongDescription, - meta_deps=DependenciesJSON}, ExternalUrl) -> + meta_deps=DependenciesJSON}, ExternalUrl, AllFiles) -> %% The serialized_object is everything but the metadata, and metadata in turn is all the %% metadata except the attributes, long description, and dependencies. All need to be %% merged back together. @@ -422,7 +467,12 @@ assemble_cookbook_ejson(#chef_cookbook_version{ [{<<"attributes">>, XMetaAttributesJSON}, {<<"dependencies">>, DependenciesJSON}, {<<"long_description">>, XLongDescription}]), - CookbookJSON = annotate_with_s3_urls(inflate(<<"cookbook">>, XCookbookJSON), OrgId, ExternalUrl), + CBJson = inflate(<<"cookbook">>, XCookbookJSON), + CBJson1 = case AllFiles of + true -> ensure_v2_metadata(CBJson); + false -> ensure_v0_metadata(CBJson) + end, + CookbookJSON = annotate_with_urls(CBJson1, OrgId, ExternalUrl), %% Now that the metadata is assembled, piece the final cookbook together lists:foldl(fun({Key, Data}, CB) -> @@ -432,13 +482,17 @@ assemble_cookbook_ejson(#chef_cookbook_version{ [{<<"frozen?">>, Frozen}, {<<"metadata">>, Metadata}]). --spec minimal_cookbook_ejson(#chef_cookbook_version{}, string()) -> ejson_term(). +assemble_cookbook_ejson(CB, ExternalUrl) -> + assemble_cookbook_ejson(CB, ExternalUrl, false). + +-spec minimal_cookbook_ejson(#chef_cookbook_version{}, string(), integer()) -> ejson_term(). minimal_cookbook_ejson(#chef_cookbook_version{org_id = OrgId, frozen=Frozen, serialized_object=XCookbookJSON, metadata=XMetadataJSON, meta_deps=DependenciesJSON}, - ExternalUrl) -> + ExternalUrl, + ApiVersion) -> %% The serialized_object is everything but the metadata, and metadata in turn is all the %% metadata except the attributes and long description. We do not add in the sub pieces %% of the metadata when merging @@ -447,7 +501,13 @@ minimal_cookbook_ejson(#chef_cookbook_version{org_id = OrgId, Metadata = ej:set({<<"dependencies">>}, Metadata0, inflate(<<"dependencies">>, DependenciesJSON)), - CookbookJSON = annotate_with_s3_urls(inflate(<<"cookbook">>, XCookbookJSON), OrgId, ExternalUrl), + AllFiles = wants_all_files(ApiVersion), + CBJson = inflate(<<"cookbook">>, XCookbookJSON), + CBJson1 = case AllFiles of + true -> ensure_v2_metadata(CBJson); + false -> ensure_v0_metadata(CBJson) + end, + CookbookJSON = annotate_with_urls(CBJson1, OrgId, ExternalUrl), lists:foldl(fun({Key, Data}, CB) -> ej:set({Key}, CB, Data) @@ -456,21 +516,31 @@ minimal_cookbook_ejson(#chef_cookbook_version{org_id = OrgId, [{<<"frozen?">>, Frozen}, {<<"metadata">>, Metadata}]). -%% @doc Add S3 download URLs for all files in the cookbook -%% Expects the whole cookbook JSON --spec annotate_with_s3_urls(ejson_term(), object_id(), string()) -> ejson_term(). -annotate_with_s3_urls(Ejson, OrgId, ExternalUrl) -> +%% Annotate a set of segments with URLs, for v0 cookbook requests +annotate_segments_with_urls(Ejson, OrgId, ExternalUrl) -> lists:foldl(fun(Segment, CB) -> - case ej:get({Segment}, CB) of - undefined -> CB; - Data -> - WithUrls = add_urls_for_segment(OrgId, Data, ExternalUrl), - ej:set({Segment}, CB, WithUrls) - end + case ej:get({Segment}, CB) of + undefined -> CB; + Data -> + WithUrls = add_urls_for_segment(OrgId, Data, ExternalUrl), + ej:set({Segment}, CB, WithUrls) + end end, Ejson, ?COOKBOOK_SEGMENTS). +%% @doc Add download URLs for all files in the cookbook +%% Expects the whole cookbook JSON +-spec annotate_with_urls(ejson_term(), object_id(), string()) -> ejson_term(). +annotate_with_urls(Ejson, OrgId, ExternalUrl) -> + case ej:get({<<"all_files">>}, Ejson) of + undefined -> + annotate_segments_with_urls(Ejson, OrgId, ExternalUrl); + Data -> + WithUrls = add_urls_for_segment(OrgId, Data, ExternalUrl), + ej:set({<<"all_files">>}, Ejson, WithUrls) + end. + %% @doc Return a sorted list of cookbook-qualified names, given a cookbook name and its %% compressed 'serialized object' representation. %% @@ -525,12 +595,25 @@ extract_recipe_names(<<31, 139, _Rest/binary>>=XCookbookJSON) -> EJson = chef_db_compression:decompress_and_decode(XCookbookJSON), %% Pull out just the recipe segment of the serialized object - Manifest = ej:get({<<"recipes">>}, EJson), + Manifest = case ej:get({<<"all_files">>}, EJson) of + undefined -> + ej:get({<<"recipes">>}, EJson); + Data -> + get_specific_segment(<<"recipes">>, Data) + end, %% Collect just the name of each recipe in the manifest. Results are NOT sorted in the %% end. [ ej:get({<<"name">>}, Recipe) || Recipe <- Manifest]. +get_specific_segment(Segment, Data) -> + IsSegment = fun(Record) -> [Seg | _ ] = get_segment_from_record(Record), Seg == Segment end, + lists:filter(IsSegment, Data). + +get_segment_from_record(Record) -> + Name = ej:get({<<"name">>}, Record), + binary:split(Name, [<<"/">>]). + %% @doc Return the s3_url_ttl from the application environment, if it is %% undefined return the default value set in ?DEFAULT_S3_URL_TTL url_ttl() -> @@ -589,6 +672,75 @@ list_query(_ObjectRec) -> bulk_get_query(_ObjectRec) -> error(not_implemented). +%% for v2 metadata, the "segment" is encoded in to the name of file +%% so for data that is v0 on disk we have to prepend the segment to the name +add_segment_to_filename(Segment, File) -> + Fn = ej:get({<<"name">>}, File), + Fn1 = iolist_to_binary([Segment, "/", Fn]), + ej:set({<<"name">>}, File, Fn1). + +remove_segment_from_filename(File) -> + [Segment | Name ] = get_segment_from_record(File), + case Name of + % if Name is nil, then we have a root file (like metadata.rb) - so we'll set the name to the segment, and the segment to <<"root_files">> + [] -> + { <<"root_files">>, ej:set({<<"name">>}, File, Segment)}; + _ -> + Record = ej:set({<<"name">>}, File, lists:last(Name)), + { Segment, Record } + end. + +%% A v2 cookbook version contains only the "all_files" key, which is a list of all the file parts +%% { "all_files": [ { "name": "recipes/default.rb", "path": "recipes/default.rb", … } ] } +%% We have to transform that into segments (?COOKBOOK_SEGMENTS) containing a list of file parts for that segment +%% { "recipes": [ { "name": "default.rb", path: "recipes/default.rb", … } ] } +%% +%% Note: We silently drop files that land in segments not listed in ?COOKBOOK_SEGMENTS +populate_segments(Data, Metadata) -> + BySegment = lists:foldl(fun file_by_segment/2, #{}, Data), + + lists:foldl(fun(Segment, CB) -> + ej:set({Segment}, CB, maps:get(Segment, BySegment, [] )) + end, + Metadata, + ?COOKBOOK_SEGMENTS). + +file_by_segment(File, Map) -> + { Segment, Record } = remove_segment_from_filename(File), + case Map of + #{ Segment := List } -> + Map#{Segment => [Record | List]}; + _ -> + Map#{Segment => [Record]} + end. + +%% Minor note: +%% The old populate_all_files code replaced the all_files entry; if there were multiple segments we would end up with only the last +%% Check test coverage to make sure that makes sense. +ensure_v2_metadata(Ejson) -> + case ej:get({<<"all_files">>}, Ejson) of + undefined -> + AllFiles = lists:flatten([ expand_segment(Segment, ej:get({Segment}, Ejson, []) ) || Segment <- ?COOKBOOK_SEGMENTS ] ), + Ejson1 = ej:set({<<"all_files">>}, Ejson, AllFiles), + + lists:foldl(fun(Segment, CB) -> ej:delete({Segment}, CB) end, + Ejson1, + ?COOKBOOK_SEGMENTS); + _ -> + Ejson + end. + +expand_segment(Segment, Data) -> + [ add_segment_to_filename(Segment, File) || File <- Data ]. + +ensure_v0_metadata(Ejson) -> + case ej:get({<<"all_files">>}, Ejson) of + undefined -> Ejson; + Data -> + Ejson1 = populate_segments(Data, Ejson), + ej:delete({<<"all_files">>}, Ejson1) + end. + update_from_ejson(#chef_cookbook_version{server_api_version = ApiVersion, org_id = OrgId, authz_id = AuthzId, @@ -670,3 +822,9 @@ list(#chef_cookbook_version{org_id = OrgId} = CBV, CallbackFun) -> set_api_version(ObjectRec, Version) -> ObjectRec#chef_cookbook_version{server_api_version = Version}. + +-spec wants_all_files('bad_value_requested' | integer()) -> boolean(). +wants_all_files(Version) when Version =:= 0 orelse Version =:= 1 -> + false; +wants_all_files(_) -> + true. diff --git a/src/oc_erchef/apps/chef_objects/test/chef_cookbook_version_tests.erl b/src/oc_erchef/apps/chef_objects/test/chef_cookbook_version_tests.erl index 9b1ba52875c..6693d11e468 100644 --- a/src/oc_erchef/apps/chef_objects/test/chef_cookbook_version_tests.erl +++ b/src/oc_erchef/apps/chef_objects/test/chef_cookbook_version_tests.erl @@ -53,11 +53,17 @@ basic_cookbook(Name, Version, Options) -> ]}} ]}. -minimal_cookbook_is_valid_test() -> +minimal_cookbook_v0_is_valid_test() -> CookbookEjson = basic_cookbook(<<"php">>, <<"1.2.3">>), - Got = chef_cookbook_version:validate_cookbook(CookbookEjson, {<<"php">>, <<"1.2.3">>}), + Got = chef_cookbook_version:validate_cookbook(CookbookEjson, {<<"php">>, <<"1.2.3">>}, false), ?assertEqual({ok, CookbookEjson}, Got). +minimal_cookbook_v2_is_valid_test() -> + CookbookEjson = basic_cookbook(<<"php">>, <<"1.2.3">>), + Got = chef_cookbook_version:validate_cookbook(CookbookEjson, {<<"php">>, <<"1.2.3">>}, true), + ?assertEqual({ok, CookbookEjson}, Got). + + valid_resources_test() -> CB0 = basic_cookbook(<<"php">>, <<"1.2.3">>), NameVer = {<<"php">>, <<"1.2.3">>}, @@ -68,7 +74,7 @@ valid_resources_test() -> {<<"checksum">>, <<"abababababababababababababababab">>}, {<<"specificity">>, <<"default">>} ]}]), - ?assertEqual({ok, CB}, chef_cookbook_version:validate_cookbook(CB, NameVer)). + ?assertEqual({ok, CB}, chef_cookbook_version:validate_cookbook(CB, NameVer, false)). top_level_key_validation_test() -> Name = <<"test_name">>, @@ -79,7 +85,7 @@ top_level_key_validation_test() -> fun() -> C = ej:set({<<"not_valid_key">>}, Cookbook, <<"some junk">>), ?assertThrow({invalid_key, <<"not_valid_key">>}, - chef_cookbook_version:validate_role(C, {Name, Version})) + chef_cookbook_version:validate_cookbook(C, {Name, Version}, false)) end} ]. @@ -91,21 +97,21 @@ bad_resources_test_() -> fun() -> CB = ej:set({<<"resources">>}, CB0, {[]}), ?assertThrow(#ej_invalid{type = json_type}, - chef_cookbook_version:validate_cookbook(CB, NameVer)) + chef_cookbook_version:validate_cookbook(CB, NameVer, false)) end}, {"resource value must be an array not a string", fun() -> CB = ej:set({<<"resources">>}, CB0, <<"not-this">>), ?assertThrow(#ej_invalid{type = json_type}, - chef_cookbook_version:validate_cookbook(CB, NameVer)) + chef_cookbook_version:validate_cookbook(CB, NameVer, false)) end}, {"resource array value must not be empty", fun() -> CB = ej:set({<<"resources">>}, CB0, [{[]}]), ?assertThrow(#ej_invalid{type = array_elt}, - chef_cookbook_version:validate_cookbook(CB, NameVer)) + chef_cookbook_version:validate_cookbook(CB, NameVer, false)) end} ]. @@ -123,7 +129,7 @@ valid_dependencies_test() -> {<<"cc">>, <<"<= 1.2.3">>}, {<<"dd">>, <<"4.4.4">>} ]}), - ?assertEqual({ok, CB}, chef_cookbook_version:validate_cookbook(CB, NameVer)). + ?assertEqual({ok, CB}, chef_cookbook_version:validate_cookbook(CB, NameVer, false)). bad_dependencies_test_() -> CB0 = basic_cookbook(<<"php">>, <<"1.2.3">>), @@ -136,7 +142,7 @@ bad_dependencies_test_() -> {<<"not valid name">>, <<"> 1.0.0">>} ]}), ?assertThrow(#ej_invalid{type = object_key}, - chef_cookbook_version:validate_cookbook(CB, NameVer)) + chef_cookbook_version:validate_cookbook(CB, NameVer, false)) end}, @@ -147,7 +153,7 @@ bad_dependencies_test_() -> {<<"apache2">>, <<"1b">>} ]}), ?assertThrow(#ej_invalid{type = object_value}, - chef_cookbook_version:validate_cookbook(CB, NameVer)) + chef_cookbook_version:validate_cookbook(CB, NameVer, false)) end} @@ -174,7 +180,7 @@ providing_constraint_test_() -> [ fun() -> CB = SetProviding(P), - ?assertEqual({ok, CB}, chef_cookbook_version:validate_cookbook(CB, NameVer)) + ?assertEqual({ok, CB}, chef_cookbook_version:validate_cookbook(CB, NameVer, false)) end || P <- AllowedValues ]. @@ -219,7 +225,7 @@ assemble_cookbook_ejson_test_() -> AuthzId, CBEJson), - MinCb = chef_cookbook_version:minimal_cookbook_ejson(Record, VHostUrl), + MinCb = chef_cookbook_version:minimal_cookbook_ejson(Record, VHostUrl, 1), ?assertEqual(undefined, ej:get({"metadata", "attributes"}, MinCb)), ?assertEqual(undefined, ej:get({"metadata", "long_description"}, MinCb)), ?assertEqual({[{<<"ruby">>, []}]}, ej:get({"metadata", "dependencies"}, MinCb)) diff --git a/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_cookbook_artifact_version.erl b/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_cookbook_artifact_version.erl index 4eb7cdd74e6..f4d7164c407 100644 --- a/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_cookbook_artifact_version.erl +++ b/src/oc_erchef/apps/oc_chef_authz/src/oc_chef_cookbook_artifact_version.erl @@ -184,14 +184,24 @@ validation_constraints() -> -spec to_json(#oc_chef_cookbook_artifact_version{}, string()) -> ejson_term(). to_json(#oc_chef_cookbook_artifact_version{metadata = RawMetadata, serialized_object = SerializedObject, - org_id = OrgId}, + org_id = OrgId, + server_api_version = ApiVersion}, ExternalUrl) -> + + CBJson = chef_db_compression:decompress_and_decode(SerializedObject), + AllFiles = chef_cookbook_version:wants_all_files(ApiVersion), + CBJson1 = case AllFiles of + true -> chef_cookbook_version:ensure_v2_metadata(CBJson); + false -> chef_cookbook_version:ensure_v0_metadata(CBJson) + end, + BaseJson = ej:set({<<"metadata">>}, - chef_db_compression:decompress_and_decode(SerializedObject), + CBJson1, chef_db_compression:decompress_and_decode(RawMetadata)), + JSONWithoutHacks = ej:delete({<<"json_class">>}, BaseJson), %% add the download URLs - chef_cookbook_version:annotate_with_s3_urls(JSONWithoutHacks, OrgId, ExternalUrl). + chef_cookbook_version:annotate_with_urls(JSONWithoutHacks, OrgId, ExternalUrl). set_api_version(ObjectRec, Version) -> ObjectRec#oc_chef_cookbook_artifact_version{server_api_version = Version}. diff --git a/src/oc_erchef/apps/oc_chef_wm/itest/oc_chef_wm_cookbook_artifact_SUITE.erl b/src/oc_erchef/apps/oc_chef_wm/itest/oc_chef_wm_cookbook_artifact_SUITE.erl index a6047607838..c787549d597 100644 --- a/src/oc_erchef/apps/oc_chef_wm/itest/oc_chef_wm_cookbook_artifact_SUITE.erl +++ b/src/oc_erchef/apps/oc_chef_wm/itest/oc_chef_wm_cookbook_artifact_SUITE.erl @@ -169,7 +169,7 @@ http_round_trip(Config) -> %% and the only difference with the original JSON that we sent should %% be the download URLs on each file - ExpectedGetBody = chef_cookbook_version:annotate_with_s3_urls(CreateEjson, OrgId, ""), + ExpectedGetBody = chef_cookbook_version:annotate_with_urls(CreateEjson, OrgId, ""), ?assertEqual(ok, ej:valid(ExpectedGetBody, GetBody)). http_get_nonexistant(_Config) -> diff --git a/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_cookbook_version.erl b/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_cookbook_version.erl index 88a40ea70e8..24bf4fabdcd 100644 --- a/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_cookbook_version.erl +++ b/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_cookbook_version.erl @@ -64,7 +64,8 @@ allowed_methods(Req, State) -> -spec validate_request(chef_wm:http_verb(), wm_req(), chef_wm:base_state()) -> {wm_req(), chef_wm:base_state()}. -validate_request(Method, Req, #base_state{resource_state = CBState0} = State) -> +validate_request(Method, Req, #base_state{server_api_version = ApiVersion, + resource_state = CBState0} = State) -> UrlName = chef_wm_util:extract_from_path(cookbook_name, Req), UrlVersion = chef_wm_util:extract_from_path(cookbook_version, Req), Version = parse_cookbook_version(Method, UrlVersion), @@ -73,7 +74,7 @@ validate_request(Method, Req, #base_state{resource_state = CBState0} = State) -> CBState = case Method of 'PUT' -> Body = wrq:req_body(Req), - {ok, Cookbook} = chef_cookbook_version:parse_binary_json(Body, {UrlName, UrlVersion}), + {ok, Cookbook} = chef_cookbook_version:parse_binary_json(Body, {UrlName, UrlVersion}, chef_cookbook_version:wants_all_files(ApiVersion)), CBState1#cookbook_state{cookbook_data = Cookbook}; _ -> CBState1 @@ -120,8 +121,9 @@ fetch_cookbook_version(DbContext, OrgId, Name, latest) -> fetch_cookbook_version(DbContext, OrgId, Name, Version) -> chef_db:fetch_cookbook_version(DbContext, OrgId, {Name, Version}). -to_json(Req, #base_state{resource_state=#cookbook_state{chef_cookbook_version=CBV}}=State) -> - CompleteEJson = chef_cookbook_version:assemble_cookbook_ejson(CBV, chef_wm_util:base_uri(Req)), +to_json(Req, #base_state{server_api_version = Version, + resource_state=#cookbook_state{chef_cookbook_version=CBV}}=State) -> + CompleteEJson = chef_cookbook_version:assemble_cookbook_ejson(CBV, chef_wm_util:base_uri(Req), chef_cookbook_version:wants_all_files(Version)), {chef_json:encode(CompleteEJson), Req, State}. from_json(Req, #base_state{resource_state = CookbookState} = State) -> diff --git a/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_depsolver.erl b/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_depsolver.erl index 4b97596f9a4..71a50cc80db 100644 --- a/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_depsolver.erl +++ b/src/oc_erchef/apps/oc_chef_wm/src/chef_wm_depsolver.erl @@ -277,7 +277,7 @@ wm_halt(Code, Req, State, ErrorData, LogMsg) -> %% Note the cookbook object we return back is a stripped-down version, %% removing large fields such as long_description and attributes in %% the metadata that are not required by chef-client -assemble_response(Req, State, CookbookVersions) -> +assemble_response(Req, #base_state{server_api_version = ApiVersion} = State, CookbookVersions) -> case oc_chef_wm_base:check_cookbook_authz(CookbookVersions, Req, State) of ok -> %% We iterate over the list again since we only want to construct the s3urls @@ -285,7 +285,7 @@ assemble_response(Req, State, CookbookVersions) -> %% cookbook which has just enough information for chef-client to run JsonList = { [ { CBV#chef_cookbook_version.name, - chef_cookbook_version:minimal_cookbook_ejson(CBV, chef_wm_util:base_uri(Req)) } + chef_cookbook_version:minimal_cookbook_ejson(CBV, chef_wm_util:base_uri(Req), ApiVersion) } || CBV <- CookbookVersions ] }, CBMapJson = chef_json:encode(JsonList), diff --git a/src/oc_erchef/include/server_api_version.hrl b/src/oc_erchef/include/server_api_version.hrl index 87a5e0dcdb1..dc9982d1837 100644 --- a/src/oc_erchef/include/server_api_version.hrl +++ b/src/oc_erchef/include/server_api_version.hrl @@ -10,12 +10,15 @@ % and not product version. -define(API_v1, 1). +% Deprecates cookbook segments +-define(API_v2, 2). + %% Highest level of deprecated API version that will be removed in the nxt major product release. %% This is used in eunit tests for version range testing across objects. -define(API_DEPRECATED_VER, ?API_v0). -define(API_MIN_VER, ?API_v0). --define(API_MAX_VER, ?API_v1). +-define(API_MAX_VER, ?API_v2). -type api_version() :: non_neg_integer() | -1.