From 280e4da9ce030db4afd03a8bb5ebd60962b05a35 Mon Sep 17 00:00:00 2001 From: Anandaraju Coimbatore Sivaraju Date: Thu, 16 Jul 2020 12:49:38 -0700 Subject: [PATCH 1/4] [Agent 1.1.2] COE AI to fix unzip fallback to ruby unzip util Why is this change needed? --------- Prior to this change, customers had issue with fallback to ruby unzip util, because there were partially unzipped files and ruby unzip didn't override them. How does it address the issue? --------- This change will rescue any exit code other than 0 and use override parameter set to true, so it would not throw Zip::DestinationFileExistsError during ruby unzip --- lib/instance_agent/platform/linux_util.rb | 12 +----------- .../plugins/codedeploy/command_executor.rb | 5 ++--- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/instance_agent/platform/linux_util.rb b/lib/instance_agent/platform/linux_util.rb index 42509923..6708f39f 100644 --- a/lib/instance_agent/platform/linux_util.rb +++ b/lib/instance_agent/platform/linux_util.rb @@ -133,22 +133,12 @@ def self.execute_zip_command(cmd) log(:debug, "Command status: #{$?}") log(:debug, "Command output: #{output}") - if !validZipExitStatus(exit_status) + if exit_status != 0 msg = "Error extracting zip archive: #{exit_status}" log(:error, msg) raise msg end end - - private - def self.validZipExitStatus(exit_status) - exit_status == 0 || isWarningStatus(exit_status) - end - - private - def self.isWarningStatus(exit_status) - exit_status == 1 - end private def self.log(severity, message) diff --git a/lib/instance_agent/plugins/codedeploy/command_executor.rb b/lib/instance_agent/plugins/codedeploy/command_executor.rb index a97a58be..e8d2e12b 100644 --- a/lib/instance_agent/plugins/codedeploy/command_executor.rb +++ b/lib/instance_agent/plugins/codedeploy/command_executor.rb @@ -384,17 +384,16 @@ def unpack_bundle(cmd, bundle_file, deployment_spec) begin InstanceAgent::Platform.util.extract_zip(bundle_file, dst) rescue + log(:warn, "Encountered non-zero exit code with default system unzip util. Hence falling back to ruby unzip to mitigate any partially unzipped or skipped zip files.") Zip::File.open(bundle_file) do |zipfile| zipfile.each do |f| file_dst = File.join(dst, f.name) FileUtils.mkdir_p(File.dirname(file_dst)) - zipfile.extract(f, file_dst) + zipfile.extract(f, file_dst) { true } end end end else - # If the bundle was a generated through a Sabini Repository - # it will be in tar format, and it won't have a bundle type InstanceAgent::Platform.util.extract_tar(bundle_file, dst) end From 3fb5ac3ad7550e1ec4f98b47d6044fa72e40b7d4 Mon Sep 17 00:00:00 2001 From: Anandaraju Coimbatore Sivaraju Date: Fri, 17 Jul 2020 10:01:38 -0700 Subject: [PATCH 2/4] [Agent 1.1.2] Update version of the agent to 1.1.2 Why is this change needed? --------- Prior to this change, Agent version was set to 1.1.1, so for the new release we need to set to 1.1.2 How does it address the issue? --------- This change will update the patch version of the agent, for new release 1.1.2 --- codedeploy_agent.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codedeploy_agent.gemspec b/codedeploy_agent.gemspec index 0cd70d36..409be663 100644 --- a/codedeploy_agent.gemspec +++ b/codedeploy_agent.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'aws_codedeploy_agent' - spec.version = '1.1.1' + spec.version = '1.1.2' spec.summary = 'Packages AWS CodeDeploy agent libraries' spec.description = 'AWS CodeDeploy agent is responsible for doing the actual work of deploying software on an individual EC2 instance' spec.author = 'Amazon Web Services' From 72e0cbd1c0cff9632c080bb645e7514e67610733 Mon Sep 17 00:00:00 2001 From: Anandaraju Coimbatore Sivaraju Date: Fri, 17 Jul 2020 12:40:54 -0700 Subject: [PATCH 3/4] [Agent 1.1.2] Change windows unzip exit status logging from debug to error Why is this change needed? --------- Prior to this change, windows unzip util had debug log level instead of error How does it address the issue? --------- This change will log the exit status as error instead of debug --- lib/instance_agent/platform/windows_util.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/instance_agent/platform/windows_util.rb b/lib/instance_agent/platform/windows_util.rb index baa1a899..6238b9a4 100644 --- a/lib/instance_agent/platform/windows_util.rb +++ b/lib/instance_agent/platform/windows_util.rb @@ -100,7 +100,7 @@ def self.execute_zip_command(cmd) if exit_status != 0 msg = "Error extracting zip archive: #{exit_status}" - log(:debug, msg) + log(:error, msg) raise msg end end From 8bc7f07f9c73887b9135d18c97aa352f676b6162 Mon Sep 17 00:00:00 2001 From: Anandaraju Coimbatore Sivaraju Date: Wed, 22 Jul 2020 15:19:43 -0700 Subject: [PATCH 4/4] [Agent 1.1.2] Add unit test to verify unpack_bundle ruby unzip mechanism Why is this change needed? --------- Prior to this change, we didn't have a way to test the "overwrite" logic in ruby unzip during the fallback mechanism of unpack_bundle method in DownloadBundle command. How does it address the issue? --------- This change adds a unit test to verify if the fallback mechanism works with a debris file in the deployment archive directory. --- .../plugins/codedeploy/unpack_bundle_test.rb | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 test/instance_agent/plugins/codedeploy/unpack_bundle_test.rb diff --git a/test/instance_agent/plugins/codedeploy/unpack_bundle_test.rb b/test/instance_agent/plugins/codedeploy/unpack_bundle_test.rb new file mode 100644 index 00000000..ad55f47d --- /dev/null +++ b/test/instance_agent/plugins/codedeploy/unpack_bundle_test.rb @@ -0,0 +1,121 @@ +require 'test_helper' +require 'certificate_helper' +require 'stringio' +require 'aws/codedeploy/local/deployer' + +class UnpackBundleTest < InstanceAgentTestCase + include InstanceAgent::Plugins::CodeDeployPlugin + def generate_signed_message_for(map) + message = @cert_helper.sign_message(map.to_json) + spec = OpenStruct.new({ :payload => message }) + spec.format = "PKCS7/JSON" + + return spec + end + + # method to create a local source bundle file as zip + def setup_local_file_bundle + @local_file_directory = File.join(@root_dir, @deployment_group_id.to_s, 'LocalFileDirectory') + FileUtils.rm_rf(@local_file_directory) + FileUtils.mkdir_p(@local_file_directory) + + input_filenames = %w(file1.txt file2.txt) + input_filenames.each do |filename| + File.open(File.join(@local_file_directory, filename), "w") do |f| + f.write("content of #{filename}") + f.close + end + end + # create the bundle as a local zip file + @local_file_location = File.join(@local_file_directory, "bundle.zip") + Zip::File.open(@local_file_location, Zip::File::CREATE) do |zipfile| + input_filenames.each do |filename| + zipfile.add(filename, File.join(@local_file_directory, filename)) + end + end + end + + context 'The CodeDeploy Plugin Command Executor' do + setup do + @test_hook_mapping = { + "BeforeBlockTraffic"=>["BeforeBlockTraffic"], + "AfterBlockTraffic"=>["AfterBlockTraffic"], + "ApplicationStop"=>["ApplicationStop"], + "BeforeInstall"=>["BeforeInstall"], + "AfterInstall"=>["AfterInstall"], + "ApplicationStart"=>["ApplicationStart"], + "BeforeAllowTraffic"=>["BeforeAllowTraffic"], + "AfterAllowTraffic"=>["AfterAllowTraffic"], + "ValidateService"=>["ValidateService"] + } + @deploy_control_client = mock + @command_executor = InstanceAgent::Plugins::CodeDeployPlugin::CommandExecutor.new( + { + :deploy_control_client => @deploy_control_client, + :hook_mapping => @test_hook_mapping + }) + @aws_region = 'us-east-1' + InstanceMetadata.stubs(:region).returns(@aws_region) + end + + context "when executing a command" do + setup do + @cert_helper = CertificateHelper.new + @deployment_id = SecureRandom.uuid + @deployment_group_name = "TestDeploymentGroup" + @application_name = "TestApplicationName" + @deployment_group_id = "foobar" + @command = Aws::CodeDeployCommand::Types::HostCommandInstance.new( + :host_command_identifier => "command-1", + :deployment_execution_id => "test-execution") + @root_dir = '/tmp/codedeploy/' + @deployment_root_dir = File.join(@root_dir, @deployment_group_id.to_s, @deployment_id.to_s) + @archive_root_dir = File.join(@deployment_root_dir, 'deployment-archive') + ProcessManager::Config.config[:root_dir] = @root_dir + end + + context "test fallback mechanism in unpack_bundle in DownloadBundle" do + setup do + setup_local_file_bundle + + # Create a debris file in the deployment-archive directory to simulate Zip::DestinationFileExistsError. + # This error will be thrown, if the ruby unzip overwrite option is not enabled and when the @archive_root_dir already has the same file. + # With the ruby unzip overwrite fix, the unpack_bundle should succeed even with debris files. + FileUtils.rm_rf(@archive_root_dir) + FileUtils.mkdir_p(@archive_root_dir) + FileUtils.cp(File.join(@local_file_directory, 'file1.txt'), @archive_root_dir) + + # We need to avoid removing @archive_root_dir in the actual logic, to avoid debris file to be deleted. + FileUtils.stubs(:rm_rf).with(@archive_root_dir) + # This exception will let the unpack_bundle method to use the rubyzip fallback mechanism + InstanceAgent::LinuxUtil.expects(:extract_zip) + .with(File.join(@deployment_root_dir, 'bundle.tar'), @archive_root_dir) + .raises("Exception: System unzip throws exception with non-zero exit code") + + @command.command_name = 'DownloadBundle' + @bundle_type = 'zip' + @deployment_spec = generate_signed_message_for( + { + "DeploymentId" => @deployment_id.to_s, + "DeploymentGroupId" => @deployment_group_id.to_s, + "ApplicationName" => @application_name, + "DeploymentGroupName" => @deployment_group_name, + "Revision" => { + "RevisionType" => "Local File", + "LocalRevision" => { + "Location" => @local_file_location, + "BundleType" => @bundle_type + } + } + }) + end + + should 'execute DownloadBundle command with debris file in deployment-archive' do + assert_equal 1, (Dir.entries(@archive_root_dir) - [".", ".."]).size + @command_executor.execute_command(@command, @deployment_spec) + assert_equal 2, (Dir.entries(@archive_root_dir) - [".", ".."]).size + end + end + end + end +end