Genrates metadata.json if not present and uploads it to chef server and deletes the local copy of it from chef repo

Signed-off-by: Vasu1105 <vasundhara.jagdale@msystechnologies.com>

6 files changed, 127 insertions, 32 deletions

diff --git a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb
index 21b7cdaff8..09b6e0669f 100644
--- a/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb
+++ b/lib/chef/chef_fs/file_system/chef_server/cookbooks_dir.rb
@@ -72,12 +72,34 @@ class Chef
           end
 
           def upload_cookbook(other, options)
-            cookbook_to_upload = other.chef_object
-            cookbook_to_upload.freeze_version if options[:freeze]
-            uploader = Chef::CookbookUploader.new(cookbook_to_upload, force: options[:force], rest: chef_rest)
+            compiled_metadata = other.chef_object.compile_metadata
 
-            with_actual_cookbooks_dir(other.parent.file_path) do
-              uploader.upload_cookbooks
+            Dir.mktmpdir do |tmp_dir_path|
+              begin
+                if compiled_metadata
+                  proxy_cookbook_path = "#{tmp_dir_path}/#{other.name}"
+                  # Make a symlink
+                  file_class.symlink other.chef_object.root_dir, proxy_cookbook_path
+                  proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path)
+                  proxy_loader.load_cookbooks
+                  cookbook_to_upload = proxy_loader.cookbook_version
+                else
+                  cookbook_to_upload = other.chef_object
+                end
+
+                cookbook_to_upload.freeze_version if options[:freeze]
+                uploader = Chef::CookbookUploader.new(cookbook_to_upload, force: options[:force], rest: chef_rest)
+
+                with_actual_cookbooks_dir(other.parent.file_path) do
+                  uploader.upload_cookbooks
+                end
+              ensure
+                # deletes the generated metadata from local repo.
+                if compiled_metadata
+                  GC.start
+                  File.unlink(compiled_metadata)
+                end
+              end
             end
           end
 
diff --git a/lib/chef/cookbook_uploader.rb b/lib/chef/cookbook_uploader.rb
index 825ac354da..1745bcad78 100644
--- a/lib/chef/cookbook_uploader.rb
+++ b/lib/chef/cookbook_uploader.rb
@@ -46,12 +46,6 @@ class Chef
     end
 
     def upload_cookbooks
-      cookbooks.each do |cookbook|
-        next if cookbook.all_files.include?("#{cookbook.root_paths[0]}/metadata.json")
-
-        generate_metadata_json(cookbook.name.to_s, cookbook)
-      end
-
       # Syntax Check
       validate_cookbooks
       # generate checksums of cookbook files and create a sandbox
diff --git a/lib/chef/cookbook_version.rb b/lib/chef/cookbook_version.rb
index d546724fa0..8de9ee0d17 100644
--- a/lib/chef/cookbook_version.rb
+++ b/lib/chef/cookbook_version.rb
@@ -513,6 +513,19 @@ class Chef
       @cookbook_manifest ||= CookbookManifest.new(self)
     end
 
+    def compile_metadata(path = root_dir)
+      json_file = "#{path}/metadata.json"
+      rb_file = "#{path}/metadata.rb"
+      return nil if File.exist?(json_file)
+
+      md = Chef::Cookbook::Metadata.new
+      md.from_file(rb_file)
+      f = File.open(json_file, "w")
+      f.write(Chef::JSONCompat.to_json_pretty(md))
+      f.close
+      f.path
+    end
+
     private
 
     def find_preferred_manifest_record(node, segment, filename)
diff --git a/lib/chef/knife/cookbook_upload.rb b/lib/chef/knife/cookbook_upload.rb
index d73fa9ae68..9f255bf2cc 100644
--- a/lib/chef/knife/cookbook_upload.rb
+++ b/lib/chef/knife/cookbook_upload.rb
@@ -20,11 +20,15 @@
 
 require_relative "../knife"
 require_relative "../cookbook_uploader"
+require_relative "../mixin/file_class"
+
 
 class Chef
   class Knife
     class CookbookUpload < Knife
 
+      include Chef::Mixin::FileClass
+
       CHECKSUM = "checksum".freeze
       MATCH_CHECKSUM = /[0-9a-f]{32,}/.freeze
 
@@ -202,15 +206,46 @@ class Chef
       end
 
       def upload(cookbooks, justify_width)
-        cookbooks.each do |cb|
-          ui.info("Uploading #{cb.name.to_s.ljust(justify_width + 10)} [#{cb.version}]")
-          check_for_broken_links!(cb)
-          check_for_dependencies!(cb)
+        require "tmpdir"
+
+        Dir.mktmpdir do |tmp_dir_path|
+          begin
+            cookbooks_to_upload = []
+            compiled_metadata = nil
+
+            cookbooks.each do |cb|
+              compiled_metadata = cb.compile_metadata
+              ui.info("Uploading #{cb.name.to_s.ljust(justify_width + 10)} [#{cb.version}]")
+              check_for_broken_links!(cb)
+              check_for_dependencies!(cb)
+
+              if compiled_metadata
+                proxy_cookbook_path = "#{tmp_dir_path}/#{cb.name}"
+                # Make a symlink
+                file_class.symlink cb.root_dir, proxy_cookbook_path
+                proxy_loader = Chef::Cookbook::CookbookVersionLoader.new(proxy_cookbook_path)
+
+                proxy_loader.load_cookbooks
+                cookbook_to_upload = proxy_loader.cookbook_version
+                cookbook_to_upload.freeze_version if options[:freeze]
+                cookbooks_to_upload << cookbook_to_upload
+              else
+                cookbooks_to_upload << cb
+              end
+            end
+
+            Chef::CookbookUploader.new(cookbooks_to_upload, force: config[:force], concurrency: config[:concurrency]).upload_cookbooks
+          rescue Chef::Exceptions::CookbookFrozen => e
+            ui.error e
+            raise
+          ensure
+            # deletes the generated metadata from local repo.
+            if compiled_metadata
+              GC.start
+              File.unlink(compiled_metadata)
+            end
+          end
         end
-        Chef::CookbookUploader.new(cookbooks, force: config[:force], concurrency: config[:concurrency]).upload_cookbooks
-      rescue Chef::Exceptions::CookbookFrozen => e
-        ui.error e
-        raise
       end
 
       def check_for_broken_links!(cookbook)
diff --git a/spec/integration/knife/upload_spec.rb b/spec/integration/knife/upload_spec.rb
index 78ed97f02a..6b85633345 100644
--- a/spec/integration/knife/upload_spec.rb
+++ b/spec/integration/knife/upload_spec.rb
@@ -209,7 +209,10 @@ describe "knife upload", :workstation do
               Created /roles/y.json
               Created /users/y.json
             EOM
-            knife("diff /").should_succeed ""
+            knife("diff --name-status /").should_succeed <<~EOM
+              D\t/cookbooks/x/metadata.json
+              D\t/cookbooks/y/metadata.json
+            EOM
           end
 
           it "knife upload --no-diff adds the new files" do
@@ -225,7 +228,10 @@ describe "knife upload", :workstation do
               Created /roles/y.json
               Created /users/y.json
             EOM
-            knife("diff --name-status /").should_succeed ""
+            knife("diff --name-status /").should_succeed <<~EOM
+              D\t/cookbooks/x/metadata.json
+              D\t/cookbooks/y/metadata.json
+            EOM
           end
         end
       end
@@ -462,6 +468,7 @@ describe "knife upload", :workstation do
           knife("upload /cookbooks/x/y.rb").should_fail "ERROR: /cookbooks/x cannot have a child created under it.\n"
           knife("upload --purge /cookbooks/x/z.rb").should_fail "ERROR: /cookbooks/x/z.rb cannot be deleted.\n"
         end
+
         # TODO this is a bit of an inconsistency: if we didn't specify --purge,
         # technically we shouldn't have deleted missing files.  But ... cookbooks
         # are a special case.
@@ -469,13 +476,18 @@ describe "knife upload", :workstation do
           knife("upload /cookbooks/x").should_succeed <<~EOM
             Updated /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
+
         it "knife upload --purge of the cookbook itself succeeds" do
           knife("upload /cookbooks/x").should_succeed <<~EOM
             Updated /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
       end
       when_the_repository "has a missing file for the cookbook" do
@@ -488,7 +500,9 @@ describe "knife upload", :workstation do
           knife("upload /cookbooks/x").should_succeed <<~EOM
             Updated /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
       end
       when_the_repository "has an extra file for the cookbook" do
@@ -503,7 +517,9 @@ describe "knife upload", :workstation do
           knife("upload /cookbooks/x").should_succeed <<~EOM
             Updated /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
       end
 
@@ -602,7 +618,6 @@ describe "knife upload", :workstation do
           knife("diff --name-status /cookbooks").should_succeed <<~EOM
             M\t/cookbooks/x/metadata.rb
             D\t/cookbooks/x/onlyin1.0.1.rb
-            A\t/cookbooks/x/metadata.json
             A\t/cookbooks/x/onlyin1.0.0.rb
           EOM
         end
@@ -614,11 +629,13 @@ describe "knife upload", :workstation do
           cookbook "x", "0.9.9", { "onlyin0.9.9.rb" => "hi" }
         end
 
-        it "knife upload /cookbooks/x uploads the local version" do
+        it "knife upload /cookbooks/x uploads the local version generates metadata.json and uploads it." do
           knife("upload --purge /cookbooks/x").should_succeed <<~EOM
             Updated /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
       end
 
@@ -639,7 +656,6 @@ describe "knife upload", :workstation do
           knife("diff --name-status /cookbooks").should_succeed <<~EOM
             M\t/cookbooks/x/metadata.rb
             D\t/cookbooks/x/onlyin1.0.1.rb
-            A\t/cookbooks/x/metadata.json
             A\t/cookbooks/x/onlyin1.0.0.rb
           EOM
         end
@@ -654,7 +670,9 @@ describe "knife upload", :workstation do
           knife("upload --purge /cookbooks/x").should_succeed <<~EOM
             Updated /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
       end
     end
@@ -754,7 +772,9 @@ describe "knife upload", :workstation do
           knife("upload /cookbooks/x").should_succeed <<~EOM
             Created /cookbooks/x
           EOM
-          knife("diff --name-status /cookbooks").should_succeed ""
+          knife("diff --name-status /cookbooks").should_succeed <<~EOM
+            D\t/cookbooks/x/metadata.json
+          EOM
         end
       end
     end
diff --git a/spec/unit/knife/cookbook_upload_spec.rb b/spec/unit/knife/cookbook_upload_spec.rb
index 9c371c4140..84b16ae215 100644
--- a/spec/unit/knife/cookbook_upload_spec.rb
+++ b/spec/unit/knife/cookbook_upload_spec.rb
@@ -23,7 +23,7 @@ require "chef/cookbook_uploader"
 require "timeout"
 
 describe Chef::Knife::CookbookUpload do
-  let(:cookbook) { Chef::CookbookVersion.new("test_cookbook", "/tmp/blah.txt") }
+  let(:cookbook) { Chef::CookbookVersion.new("test_cookbook", "/tmp/blah") }
 
   let(:cookbooks_by_name) do
     { cookbook.name => cookbook }
@@ -56,6 +56,7 @@ describe Chef::Knife::CookbookUpload do
 
   describe "with --concurrency" do
     it "should upload cookbooks with predefined concurrency" do
+      allow(cookbook).to receive(:compile_metadata).and_return(nil)
       allow(Chef::CookbookVersion).to receive(:list_all_versions).and_return({})
       knife.config[:concurrency] = 3
       test_cookbook = Chef::CookbookVersion.new("test_cookbook", "/tmp/blah")
@@ -114,11 +115,13 @@ describe Chef::Knife::CookbookUpload do
       end
 
       it "should read only one cookbook" do
+        allow(cookbooks_by_name["test_cookbook1"]).to receive(:compile_metadata).and_return(nil)
         expect(cookbook_loader).to receive(:[]).once.with("test_cookbook1").and_call_original
         knife.run
       end
 
       it "should not read all cookbooks" do
+        allow(cookbooks_by_name["test_cookbook1"]).to receive(:compile_metadata).and_return(nil)
         expect(cookbook_loader).not_to receive(:load_cookbooks)
         knife.run
       end
@@ -178,6 +181,7 @@ describe Chef::Knife::CookbookUpload do
         allow(knife).to receive(:cookbook_names).and_return(%w{cookbook_dependency test_cookbook})
         @stdout, @stderr, @stdin = StringIO.new, StringIO.new, StringIO.new
         knife.ui = Chef::Knife::UI.new(@stdout, @stderr, @stdin, {})
+        allow(cookbook).to receive(:compile_metadata).and_return(nil)
       end
 
       it "should exit and not upload the cookbook" do
@@ -213,6 +217,7 @@ describe Chef::Knife::CookbookUpload do
     end
 
     it "should freeze the version of the cookbooks if --freeze is specified" do
+      allow(cookbook).to receive(:compile_metadata).and_return(nil)
       knife.config[:freeze] = true
       expect(cookbook).to receive(:freeze_version).once
       knife.run
@@ -229,6 +234,8 @@ describe Chef::Knife::CookbookUpload do
           @test_cookbook2 = Chef::CookbookVersion.new("test_cookbook2", "/tmp/blah")
           allow(cookbook_loader).to receive(:each).and_yield("test_cookbook1", @test_cookbook1).and_yield("test_cookbook2", @test_cookbook2)
           allow(cookbook_loader).to receive(:cookbook_names).and_return(%w{test_cookbook1 test_cookbook2})
+          allow(@test_cookbook1).to receive(:compile_metadata).and_return(nil)
+          allow(@test_cookbook2).to receive(:compile_metadata).and_return(nil)
         end
 
         it "should upload all cookbooks" do
@@ -283,6 +290,10 @@ describe Chef::Knife::CookbookUpload do
     end
 
     describe "when a frozen cookbook exists on the server" do
+      before(:each) do
+        allow(cookbook).to receive(:compile_metadata).and_return(nil)
+      end
+
       it "should fail to replace it" do
         exception = Chef::Exceptions::CookbookFrozen.new
         expect(cookbook_uploader).to receive(:upload_cookbooks)