Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows_zipfile Archives with Subdirectories and Path Declaration #459

Open
michaeltlombardi opened this issue Mar 29, 2017 · 10 comments
Open
Labels
Type: Bug Doesn't work as expected.

Comments

@michaeltlombardi
Copy link

michaeltlombardi commented Mar 29, 2017

Cookbook version

3.0.3

Chef-client version

12.19.36

Platform Details

Windows 2012R2x64, x64-mingw32

Scenario:

Attempting to download a zip of a directory with subdirectories from S3 and automatically unzip into C:/Program Files/WindowsPowerShell/Modules or C:\Program Files\WindowsPowerShell\Modules

Method One Gist

windows_zipfile 'C:/Program Files/WindowsPowerShell/Modules' do
  source 's3_url_to_module_zip'
  checksum 'module_zip_checksum
  action :unzip
  not_if {::File.directory?("C:/Program Files/WindowsPowerShell/Modules/cCDROMdriveletter")}
end

Method Two gist

windows_zipfile 'C:\Program Files\WindowsPowerShell\Modules' do
  source 's3_url_to_module_zip'
  checksum 'module_zip_checksum
  action :unzip
  not_if {::File.directory?("C:/Program Files/WindowsPowerShell/Modules/cCDROMdriveletter")}
end

Steps to Reproduce:

Create a minimal recipe which attempts to use windows_zipfile to download the module zip and expand it.

Expected Result:

The file is automatically downloaded and unzipped.

Actual Result:

Invalid argument @ rb_sysopen - C:/Program Files/WindowsPowerShell/Modules/cCDROMdriveletter\1.1.0\DSCResources\

Per @smurawski's suggestion, I retested with the following:

windows_zipfile 'C:\Program Files\WindowsPowerShell\Modules' do
  source 's3_url_to_module_zip'
  checksum 'module_zip_checksum
  action :unzip
  not_if {::File.directory?("C:/Program Files/WindowsPowerShell/Modules/cCDROMdriveletter")}
end

This was successful.
Upon further testing, this is also a failure mode, I was mistaken. The toplevel folder was created and so thereafter passed the not_if condition and skipped this resource. The error for this item is the same as before:

Invalid argument @ rb_sysopen - C:\Program Files\WindowsPowerShell\Modules/cCDROMdriveletter\1.1.0\DSCResources

Suggestion

We should update the behavior of the resource to function with both forward and backward slashes for zipped archives which include subfolders.

I'm absolutely willing to submit a PR for the docs update in the next 24h or and try my hand at updating the resource itself in the next week or so.

@smurawski smurawski added the Type: Bug Doesn't work as expected. label Mar 29, 2017
@michaeltlombardi
Copy link
Author

Here's the debug output for reference.

@smurawski
Copy link
Contributor

@michaeltlombardi I cannot replicate this with the same resource with forward or backword slashes when downloading the same module from github.

windows_zipfile 'C:/Program Files/WindowsPowerShell/Modules' do
  source 'https://github.com/kewalaka/cCDROMdriveletter/archive/1.1.0.zip'
  action :unzip
end

or

windows_zipfile 'C:\Program Files\WindowsPowerShell\Modules' do
  source 'https://github.com/kewalaka/cCDROMdriveletter/archive/1.1.0.zip'
  action :unzip
end

worked with the latest version of the cookbook on supermarket (3.0.4).

Can you validate your zipfile has the correct contents/is a valid zip file?

@smurawski smurawski self-assigned this Mar 30, 2017
@michaeltlombardi
Copy link
Author

Just validated the zip, everything is alright with it - unzips fine for me. I'll test with the same URL you have listed here and see if there's any difference.

@smurawski
Copy link
Contributor

@michaeltlombardi and I were just testing further and found that the unzip function does not like archives made with PowerShellGet's Save-Module. This seems more like a problem with the upstream rubyzip gem.

We could (depending on the version of PowerShell/.NET on the box, attempt to extract with either Expand-Archive (PowerShell 5 or newer) or [System.IO.Compression] (if .NET 4 or greater), then fall back to rubyzip.

@michaeltlombardi
Copy link
Author

The way I generated the zip:

Find-Module -Name cCDROMdriveletter -Repository PSGallery | Save-Module -Path ./Modules
Compress-Archive -Path ./Modules/cCDROMdriveletter -DestinationPath ./Modules/cCDROMdriveletter.zip

So it might be an issue with Compress-Archive instead of Save-Module - Save-Module just downloads the files into a local folder.

@aprice
Copy link

aprice commented Aug 25, 2017

It does seem to be an issue specifically with zips created using Compress-Archive. I have the same problem with zips created using Compress-Archive, but not with zips created using [System.IO.Compression.ZipFile]::CreateFromDirectory. It seems that when windows_zipfile tries to unzip files created with Compress-Archive, the first path separator in the destination path is always a forward slash.

@axelrtgs
Copy link

axelrtgs commented Oct 9, 2018

I think i may have found a work around to this issue, i just encountered it and after much work to try and fix it i have found that changing the section here: https://github.com/chef-cookbooks/windows/blob/master/resources/zipfile.rb#L54 to the following block fixes the issue.

Zip::File.open(cache_file_path) do |zip|
    zip.each do |entry|
    path = ::File.join(new_resource.path, entry.name)

    dirname = path.slice(-1) == '/' || path.slice(-1) == '\\' ? path : ::File.dirname(path)
    FileUtils.mkdir_p(dirname)

    if new_resource.overwrite && ::File.exist?(path) && !::File.directory?(path)
        FileUtils.rm(path)
    end
    zip.extract(entry, path) unless ::File.exist?(path)
    end
end

It would seem that some archives are created with folders that are treated as files and rubyzip tries to extract the directory as a file but the path does not yet exist yet. the mkdir_p doesnt create the right directory since a file path 'c:\something\thing' in these zips gets cut to 'c:\something' by ::File.dirname(path) since by design it strips the last component to get the directory name.

The workaround above should work with either path separator (i added both path separators since archives internally could have either or in the archive and we should handle both) although i have not tested it extensively.

If this solution is acceptable i can submit a PR with the fix.

An example archive for testing which is publicly available and made big a big company (Elastic) is: https://artifacts.elastic.co/downloads/beats/filebeat/filebeat-6.4.2-windows-x86_64.zip

@mxmxx
Copy link

mxmxx commented Nov 16, 2018

Just encountered this attempting to use windows_zipfile with the 6.4.2 Metricbeat package from Elastic. Workaround was to use powershell_script resource instead. This was a head-scratcher b/c interestingly enough, the 6.3.2 package worked fine with windows_zipfile; assuming they changed their compression process between then and 6.4.2.

Windows cookbook: 5.1.5

@adybuxton
Copy link

I've found the same issue with all the latest Elastic beats packages.

@ValkyrieOps
Copy link

Confirming this issue still exists with Elastic beats package on Server 2019/Win10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Doesn't work as expected.
Development

No branches or pull requests

7 participants