Merge pull request #250 from chef/tm/rfc_67

Remove cookbook segments

12 files changed, 112 insertions, 96 deletions

diff --git a/.travis.yml b/.travis.yml
index 415a0e3..ab474a3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -18,11 +18,11 @@ script:
 
 matrix:
   include:
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       env: PEDANT_KNIFE_TESTS=true PEDANT_ALLOW_RVM=1
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       env: SINGLE_ORG=true
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       env: CHEF_FS=true
 # Commented out until we work with a released version again
 #    - rvm: 2.2.5
@@ -33,17 +33,17 @@ matrix:
 #      env:
 #        - CHEF_FS=true
 #        - "GEMFILE_MOD=\"gem 'chef', github: 'chef/chef'\""
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       env: FILE_STORE=true
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       script: bundle exec rake chef_spec
       env: TEST=chef_spec
-    - rvm: 2.2.5
+    - rvm: 2.3.3
       script: bundle exec rake spec
       env: TEST=rake_spec
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       script: bundle exec rake spec
       env: TEST=rake_spec
-    - rvm: 2.3.1
+    - rvm: 2.4.1
       script: bundle exec rake style
       env: TEST=chefstyle
diff --git a/chef-zero.gemspec b/chef-zero.gemspec
index 5152af2..fc788e7 100644
--- a/chef-zero.gemspec
+++ b/chef-zero.gemspec
@@ -12,7 +12,7 @@ Gem::Specification.new do |s|
   s.homepage = "http://www.chef.io"
   s.license = "Apache 2.0"
 
-  s.required_ruby_version = ">= 2.2.2"
+  s.required_ruby_version = ">= 2.3.1"
 
   s.add_dependency "mixlib-log", "~> 1.3"
   s.add_dependency "hashie", ">= 2.0", "< 4.0"
diff --git a/lib/chef_zero.rb b/lib/chef_zero.rb
index 2db3392..bdb6511 100644
--- a/lib/chef_zero.rb
+++ b/lib/chef_zero.rb
@@ -2,7 +2,7 @@ module ChefZero
   require "chef_zero/log"
 
   MIN_API_VERSION = 0
-  MAX_API_VERSION = 1
+  MAX_API_VERSION = 2
 
   CERTIFICATE = "-----BEGIN CERTIFICATE-----\nMIIDMzCCApygAwIBAgIBATANBgkqhkiG9w0BAQUFADCBnjELMAkGA1UEBhMCVVMx\nEzARBgNVBAgMCldhc2hpbmd0b24xEDAOBgNVBAcMB1NlYXR0bGUxFjAUBgNVBAoM\nDU9wc2NvZGUsIEluYy4xHDAaBgNVBAsME0NlcnRpZmljYXRlIFNlcnZpY2UxMjAw\nBgNVBAMMKW9wc2NvZGUuY29tL2VtYWlsQWRkcmVzcz1hdXRoQG9wc2NvZGUuY29t\nMB4XDTEyMTEyMTAwMzQyMVoXDTIyMTExOTAwMzQyMVowgZsxEDAOBgNVBAcTB1Nl\nYXR0bGUxEzARBgNVBAgTCldhc2hpbmd0b24xCzAJBgNVBAYTAlVTMRwwGgYDVQQL\nExNDZXJ0aWZpY2F0ZSBTZXJ2aWNlMRYwFAYDVQQKEw1PcHNjb2RlLCBJbmMuMS8w\nLQYDVQQDFCZVUkk6aHR0cDovL29wc2NvZGUuY29tL0dVSURTL3VzZXJfZ3VpZDCC\nASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANLDmPbR71bS2esZlZh/HfC6\n0azXFjl2677wq2ovk9xrUb0Ui4ZLC66TqQ9C/RBzOjXU4TRf3hgPTqvlCgHusl0d\nIcLCrsSl6kPEhJpYWWfRoroIAwf82A9yLQekhqXZEXu5EKkwoUMqyF6m0ZCasaE1\ny8niQxdLAsk3ady/CGQlFqHTPKFfU5UASR2LRtYC1MCIvJHDFRKAp9kPJbQo9P37\nZ8IU7cDudkZFgNLmDixlWsh7C0ghX8fgAlj1P6FgsFufygam973k79GhIP54dELB\nc0S6E8ekkRSOXU9jX/IoiXuFglBvFihAdhvED58bMXzj2AwXUyeAlxItnvs+NVUC\nAwEAATANBgkqhkiG9w0BAQUFAAOBgQBkFZRbMoywK3hb0/X7MXmPYa7nlfnd5UXq\nr2n32ettzZNmEPaI2d1j+//nL5qqhOlrWPS88eKEPnBOX/jZpUWOuAAddnrvFzgw\nrp/C2H7oMT+29F+5ezeViLKbzoFYb4yECHBoi66IFXNae13yj7taMboBeUmE664G\nTB/MZpRr8g==\n-----END CERTIFICATE-----\n"
   PUBLIC_KEY = "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0sOY9tHvVtLZ6xmVmH8d\n8LrRrNcWOXbrvvCrai+T3GtRvRSLhksLrpOpD0L9EHM6NdThNF/eGA9Oq+UKAe6y\nXR0hwsKuxKXqQ8SEmlhZZ9GiuggDB/zYD3ItB6SGpdkRe7kQqTChQyrIXqbRkJqx\noTXLyeJDF0sCyTdp3L8IZCUWodM8oV9TlQBJHYtG1gLUwIi8kcMVEoCn2Q8ltCj0\n/ftnwhTtwO52RkWA0uYOLGVayHsLSCFfx+ACWPU/oWCwW5/KBqb3veTv0aEg/nh0\nQsFzRLoTx6SRFI5dT2Nf8iiJe4WCUG8WKEB2G8QPnxsxfOPYDBdTJ4CXEi2e+z41\nVQIDAQAB\n-----END PUBLIC KEY-----\n"
diff --git a/lib/chef_zero/chef_data/cookbook_data.rb b/lib/chef_zero/chef_data/cookbook_data.rb
index bdad333..2766162 100644
--- a/lib/chef_zero/chef_data/cookbook_data.rb
+++ b/lib/chef_zero/chef_data/cookbook_data.rb
@@ -13,7 +13,9 @@ module ChefZero
         end
 
         result = files_from(cookbook)
-        recipe_names = result[:recipes].map do |recipe|
+        recipe_names = result[:all_files].select do |file|
+          file[:name].start_with?("recipes/")
+        end.map do |recipe|
           recipe_name = recipe[:name][0..-2]
           recipe_name == "default" ? name : "#{name}::#{recipe_name}"
         end
@@ -86,26 +88,10 @@ module ChefZero
           cookbook_arg(:supports, cookbook, version_constraints)
         end
 
-        def recommends(cookbook, *version_constraints)
-          cookbook_arg(:recommendations, cookbook, version_constraints)
-        end
-
-        def suggests(cookbook, *version_constraints)
-          cookbook_arg(:suggestions, cookbook, version_constraints)
-        end
-
-        def conflicts(cookbook, *version_constraints)
-          cookbook_arg(:conflicting, cookbook, version_constraints)
-        end
-
         def provides(cookbook, *version_constraints)
           cookbook_arg(:providing, cookbook, version_constraints)
         end
 
-        def replaces(cookbook, *version_constraints)
-          cookbook_arg(:replacing, cookbook, version_constraints)
-        end
-
         def gem(*opts)
           self[:gems] ||= []
           self[:gems] << opts
@@ -119,10 +105,6 @@ module ChefZero
           self[:attributes][name] = options
         end
 
-        def grouping(name, options)
-          self[:grouping][name] = options
-        end
-
         def cookbook_arg(key, cookbook, version_constraints)
           self[key][cookbook] = version_constraints.first || ">= 0.0.0"
         end
@@ -142,19 +124,14 @@ module ChefZero
 
       def self.files_from(directory)
         # TODO some support .rb only
+        result = load_files(directory)
+
+        set_specificity(result, :templates)
+        set_specificity(result, :files)
+
         result = {
-          :attributes => load_child_files(directory, "attributes", false),
-          :definitions => load_child_files(directory, "definitions", false),
-          :recipes => load_child_files(directory, "recipes", false),
-          :libraries => load_child_files(directory, "libraries", true),
-          :templates => load_child_files(directory, "templates", true),
-          :files => load_child_files(directory, "files", true),
-          :resources => load_child_files(directory, "resources", true),
-          :providers => load_child_files(directory, "providers", true),
-          :root_files => load_files(directory, false),
+          all_files: result,
         }
-        set_specificity(result[:templates])
-        set_specificity(result[:files])
         result
       end
 
@@ -199,45 +176,52 @@ module ChefZero
         end
       end
 
-      def self.load_child_files(parent, key, recursive)
-        result = load_files(get_directory(parent, key), recursive)
+      def self.load_child_files(parent, key, recursive, part)
+        result = load_files(get_directory(parent, key), recursive, part)
         result.each do |file|
           file[:path] = "#{key}/#{file[:path]}"
         end
         result
       end
 
-      def self.load_files(directory, recursive)
+      def self.load_files(directory, recursive = true, part = nil)
         result = []
         if directory
           list(directory).each do |child_name|
             dir = get_directory(directory, child_name)
             if dir
+              child_part = child_name if part.nil?
               if recursive
-                result += load_child_files(directory, child_name, recursive)
+                result += load_child_files(directory, child_name, recursive, child_part)
               end
             else
-              result += load_file(read_file(directory, child_name), child_name)
+              result += load_file(read_file(directory, child_name), child_name, part)
             end
           end
         end
         result
       end
 
-      def self.load_file(value, name)
+      def self.load_file(value, name, part = nil)
+        specific_name = part ? "#{part}/#{name}" : name
         [{
-          :name => name,
+          :name => specific_name,
           :path => name,
           :checksum => Digest::MD5.hexdigest(value),
           :specificity => "default",
         }]
       end
 
-      def self.set_specificity(files)
+      def self.set_specificity(files, type)
         files.each do |file|
+          next unless file[:name].split("/")[0] == type.to_s
+
           parts = file[:path].split("/")
-          raise "Only directories are allowed directly under templates or files: #{file[:path]}" if parts.size == 2
-          file[:specificity] = parts[1]
+          file[:specificity] = if parts.size == 2
+                                 "default"
+                               else
+                                 parts[1]
+                               end
         end
       end
     end
diff --git a/lib/chef_zero/chef_data/data_normalizer.rb b/lib/chef_zero/chef_data/data_normalizer.rb
index 939acc5..6e96f85 100644
--- a/lib/chef_zero/chef_data/data_normalizer.rb
+++ b/lib/chef_zero/chef_data/data_normalizer.rb
@@ -5,6 +5,9 @@ require "chef_zero/chef_data/default_creator"
 module ChefZero
   module ChefData
     class DataNormalizer
+
+      COOKBOOK_SEGMENTS = %w{ resources providers recipes definitions libraries attributes files templates root_files }
+
       def self.normalize_acls(acls)
         ChefData::DefaultCreator::PERMISSIONS.each do |perm|
           acls[perm] ||= {}
@@ -90,18 +93,51 @@ module ChefZero
       end
 
       def self.normalize_cookbook(endpoint, org_prefix, cookbook, name, version, base_uri, method,
-                                  is_cookbook_artifact = false)
+                                  is_cookbook_artifact = false, api_version: 2)
         # TODO I feel dirty
-        if method != "PUT"
-          cookbook.each_pair do |key, value|
-            if value.is_a?(Array)
-              value.each do |file|
-                if file.is_a?(Hash) && file.has_key?("checksum")
-                  file["url"] ||= endpoint.build_uri(base_uri, org_prefix + ["file_store", "checksums", file["checksum"]])
+        if method == "PUT" && api_version < 2
+          cookbook["all_files"] = cookbook.delete(["root_files"]) { [] }
+          COOKBOOK_SEGMENTS.each do |segment|
+            next unless cookbook.has_key? segment
+            cookbook[segment].each do |file|
+              file["name"] = "#{segment}/#{file['name']}"
+              cookbook["all_files"] << file
+            end
+            cookbook.delete(segment)
+          end
+        elsif method != "PUT"
+          if cookbook.key? "all_files"
+            cookbook["all_files"].each do |file|
+              if file.is_a?(Hash) && file.has_key?("checksum")
+                file["url"] ||= endpoint.build_uri(base_uri, org_prefix + ["file_store", "checksums", file["checksum"]])
+              end
+            end
+
+            # down convert to old style manifest, ensuring we don't send all_files on the wire and that we correctly divine segments
+            # any file that's not in an old segment is just dropped on the floor.
+            if api_version < 2
+
+              # the spec appears to think we should send empty arrays for each segment, so let's do that
+              COOKBOOK_SEGMENTS.each { |seg| cookbook[seg] ||= [] }
+
+              cookbook["all_files"].each do |file|
+                segment, name = file["name"].split("/")
+
+                # root_files have no segment prepended
+                if name.nil?
+                  name = segment
+                  segment = "root_files"
                 end
+
+                file.delete("full_path")
+                next unless COOKBOOK_SEGMENTS.include? segment
+                file["name"] = name
+                cookbook[segment] << file
               end
+              cookbook.delete("all_files")
             end
           end
+
           cookbook["name"] ||= "#{name}-#{version}"
           # TODO it feels wrong, but the real chef server doesn't expand 'version', so we don't either.
 
diff --git a/lib/chef_zero/endpoints/cookbook_artifact_identifier_endpoint.rb b/lib/chef_zero/endpoints/cookbook_artifact_identifier_endpoint.rb
index 237f9c9..4c70252 100644
--- a/lib/chef_zero/endpoints/cookbook_artifact_identifier_endpoint.rb
+++ b/lib/chef_zero/endpoints/cookbook_artifact_identifier_endpoint.rb
@@ -9,7 +9,7 @@ module ChefZero
 
       # GET /organizations/ORG/cookbook_artifacts/NAME/IDENTIFIER
       def get(request)
-        cookbook_data = normalize(request, parse_json(get_data(request)))
+        cookbook_data = normalize(request, get_data(request))
         return json_response(200, cookbook_data)
       end
 
@@ -19,7 +19,8 @@ module ChefZero
           return error(409, "Cookbooks cannot be modified, and a cookbook with this identifier already exists.")
         end
 
-        set_data(request, nil, request.body, :create_dir)
+        cb_data = normalize(request, request.body)
+        set_data(request, nil, to_json(cb_data), :create_dir)
 
         return already_json_response(201, request.body)
       end
@@ -28,7 +29,7 @@ module ChefZero
       def delete(request)
         begin
           doomed_cookbook_json = get_data(request)
-          identified_cookbook_data = normalize(request, parse_json(doomed_cookbook_json))
+          identified_cookbook_data = normalize(request, doomed_cookbook_json)
           delete_data(request)
 
           # go through the recipes and delete stuff in the file store.
@@ -59,9 +60,10 @@ module ChefZero
       end
 
       def normalize(request, cookbook_artifact_data)
+        cookbook = parse_json(cookbook_artifact_data)
         ChefData::DataNormalizer.normalize_cookbook(self, request.rest_path[0..1],
-                                                    cookbook_artifact_data, request.rest_path[3], request.rest_path[4],
-                                                    request.base_uri, request.method, true)
+                                                    cookbook, request.rest_path[3], request.rest_path[4],
+                                                    request.base_uri, request.method, true, api_version: request.api_version)
       end
     end
   end
diff --git a/lib/chef_zero/endpoints/cookbook_version_endpoint.rb b/lib/chef_zero/endpoints/cookbook_version_endpoint.rb
index d22e5d9..8afde32 100644
--- a/lib/chef_zero/endpoints/cookbook_version_endpoint.rb
+++ b/lib/chef_zero/endpoints/cookbook_version_endpoint.rb
@@ -37,7 +37,7 @@ module ChefZero
         end
 
         # Set the cookbook
-        set_data(request, request.rest_path, request.body, :create_dir, :create)
+        set_data(request, request.rest_path, populate_defaults(request, request.body), :create_dir, :create)
 
         # If the cookbook was updated, check for deleted files and clean them up
         if existing_cookbook
@@ -47,7 +47,7 @@ module ChefZero
           end
         end
 
-        already_json_response(existing_cookbook ? 200 : 201, populate_defaults(request, request.body))
+        already_json_response(existing_cookbook ? 200 : 201, populate_defaults(request, request.body, normalize: false))
       end
 
       def delete(request)
@@ -116,10 +116,12 @@ module ChefZero
         end
       end
 
-      def populate_defaults(request, response_json)
+      def populate_defaults(request, response_json, normalize: true)
         # Inject URIs into each cookbook file
         cookbook = FFI_Yajl::Parser.parse(response_json)
-        cookbook = ChefData::DataNormalizer.normalize_cookbook(self, request.rest_path[0..1], cookbook, request.rest_path[3], request.rest_path[4], request.base_uri, request.method)
+        cookbook["chef_type"] ||= "cookbook_version"
+        cookbook["json_class"] ||= "Chef::CookbookVersion"
+        cookbook = ChefData::DataNormalizer.normalize_cookbook(self, request.rest_path[0..1], cookbook, request.rest_path[3], request.rest_path[4], request.base_uri, request.method, false, api_version: request.api_version) if normalize
         FFI_Yajl::Encoder.encode(cookbook, :pretty => true)
       end
 
diff --git a/lib/chef_zero/endpoints/cookbooks_base.rb b/lib/chef_zero/endpoints/cookbooks_base.rb
index 10d1b5b..f97c38c 100644
--- a/lib/chef_zero/endpoints/cookbooks_base.rb
+++ b/lib/chef_zero/endpoints/cookbooks_base.rb
@@ -47,18 +47,15 @@ module ChefZero
       end
 
       def recipe_names(cookbook_name, cookbook)
-        result = []
-        if cookbook["recipes"]
-          cookbook["recipes"].each do |recipe|
-            if recipe["path"] == "recipes/#{recipe['name']}" && recipe["name"][-3..-1] == ".rb"
-              if recipe["name"] == "default.rb"
-                result << cookbook_name
-              end
-              result << "#{cookbook_name}::#{recipe['name'][0..-4]}"
-            end
+        cookbook["all_files"].inject([]) do |acc, file|
+          part, name = file["name"].split("/")
+          next unless part == "recipes" || File.extname(name) != ".rb"
+          if name == "default.rb"
+            acc << cookbook_name
+          else
+            acc << "#{cookbook_name}::#{File.basename(name, ".rb")}"
           end
         end
-        result
       end
     end
   end
diff --git a/lib/chef_zero/endpoints/environment_cookbook_versions_endpoint.rb b/lib/chef_zero/endpoints/environment_cookbook_versions_endpoint.rb
index f1f38fe..669be9a 100644
--- a/lib/chef_zero/endpoints/environment_cookbook_versions_endpoint.rb
+++ b/lib/chef_zero/endpoints/environment_cookbook_versions_endpoint.rb
@@ -49,7 +49,7 @@ module ChefZero
         result = {}
         solved.each_pair do |name, versions|
           cookbook = FFI_Yajl::Parser.parse(get_data(request, request.rest_path[0..1] + ["cookbooks", name, versions[0]]))
-          result[name] = ChefData::DataNormalizer.normalize_cookbook(self, request.rest_path[0..1], cookbook, name, versions[0], request.base_uri, "MIN")
+          result[name] = ChefData::DataNormalizer.normalize_cookbook(self, request.rest_path[0..1], cookbook, name, versions[0], request.base_uri, "MIN", false, api_version: request.api_version)
         end
         json_response(200, result)
       end
diff --git a/lib/chef_zero/endpoints/server_api_version_endpoint.rb b/lib/chef_zero/endpoints/server_api_version_endpoint.rb
index a66d3f3..ba32257 100644
--- a/lib/chef_zero/endpoints/server_api_version_endpoint.rb
+++ b/lib/chef_zero/endpoints/server_api_version_endpoint.rb
@@ -4,7 +4,7 @@ module ChefZero
   module Endpoints
     # /server_api_version
     class ServerAPIVersionEndpoint < RestBase
-      API_VERSION = 1
+      API_VERSION = 2
       def get(request)
         json_response(200, { "min_api_version" => MIN_API_VERSION, "max_api_version" => MAX_API_VERSION },
           request_version: request.api_version, response_version: API_VERSION)
diff --git a/lib/chef_zero/rest_base.rb b/lib/chef_zero/rest_base.rb
index 976fade..367ce70 100644
--- a/lib/chef_zero/rest_base.rb
+++ b/lib/chef_zero/rest_base.rb
@@ -19,15 +19,7 @@ module ChefZero
     end
 
     def check_api_version(request)
-      return if request.api_version.nil? # Not present in headers
-      version = request.api_version.to_i
-
-      unless version.to_s == request.api_version.to_s # Version is not an Integer
-        return json_response(406,
-          { "username" => request.requestor },
-          request_version: -1, response_version: -1
-        )
-      end
+      version = request.api_version
 
       if version > MAX_API_VERSION || version < MIN_API_VERSION
         response = {
@@ -38,10 +30,15 @@ module ChefZero
         }
 
         return json_response(406,
-          response,
-          request_version: version, response_version: -1
-        )
+                             response,
+                             request_version: version, response_version: -1
+                            )
       end
+    rescue ArgumentError
+      return json_response(406,
+                           { "username" => request.requestor },
+                           request_version: -1, response_version: -1
+                          )
     end
 
     def call(request)
diff --git a/lib/chef_zero/rest_request.rb b/lib/chef_zero/rest_request.rb
index 4e82fb4..127ec09 100644
--- a/lib/chef_zero/rest_request.rb
+++ b/lib/chef_zero/rest_request.rb
@@ -3,8 +3,6 @@ require "rack/request"
 module ChefZero
   class RestRequest
 
-    ZERO = "0".freeze
-
     def initialize(env, rest_base_prefix = [])
       @env = env
       @rest_base_prefix = rest_base_prefix
@@ -28,11 +26,11 @@ module ChefZero
     end
 
     def api_version
-      @env["HTTP_X_OPS_SERVER_API_VERSION"] || ZERO
+      Integer(@env["HTTP_X_OPS_SERVER_API_VERSION"] || 0)
     end
 
     def api_v0?
-      api_version == ZERO
+      api_version == 0
     end
 
     def requestor