(MODULES-6652) Fix download 7z behind proxy #94

helge000 · 2018-02-16T10:47:27Z

Adds a local file '7za.exe' witch is copied to $::system32 in case unzip
method is '7zip'.
7za - source: https://chocolatey.org/7za.exe, witch was originally used
in the template.

templates/InstallChocolatey.ps1.erb

ferventcoder · 2018-02-16T11:40:48Z

A few things:

Find a temp location, don't use system32.
Make sure this is 32-bit 7za.exe.
Ensure it is at least 18.1 to get past all current CVEs.

ferventcoder · 2018-02-16T11:43:45Z

We are pushing the latest 7za up to https://chocolatey.org, so if you want to use that as a source, hold until we have it up there.

helge000 · 2018-02-16T12:12:06Z

A few things:

Find a temp location, don't use system32.

Do you have something specific in mind here?

I did not use a temp location on purpose to avoid recreating 7za.exe unnecessarily. However, it might be ok since we only call install on installs and updates? If I use a temp location, I only see C:\Windows\temp; I would need to access it quite ugly via $::system32\\..\\temp.

Make sure this is 32-bit 7za.exe.

Should be the case already since I used the one from chocolatey.org

Ensure it is at least 18.1 to get past all current CVEs.

I could only find 18.0.1; so I'll just wait until you tell me to redownload.

templates/InstallChocolatey.ps1.erb

ferventcoder · 2018-02-19T19:50:46Z

@helge000 https://chocolatey.org/7za.exe is now 18.1. (it shows 18.0.1 as product version).

helge000 · 2018-02-20T10:12:44Z

manifests/install.pp

-  $unzip_type   = $::chocolatey::use_7zip ? {
-    true      => '7zip',
-    default   => 'windows'
+  $sevenzip_exe = "${::system32}\\chocolatey_temp_7za.exe"


If you don't mind code style, we could use something like

$sevenzip_exe = inline_template('"#{ENV['TEMP']}\\7za.exe"')

Witch is basically the current behavior.

That could work, but I wonder what the Puppet style guide says about that.

jpogran · 2018-02-20T16:38:07Z

I am not OK with vendoring in a binary in our module, or writing it to system32

ferventcoder · 2018-02-21T02:18:35Z

I am not OK with vendoring in a binary in our module, or writing it to system32

@jpogran I replied at https://tickets.puppetlabs.com/browse/MODULES-6652. Let me know if you want to keep the discussion here or if that location is more appropriate?

Iristyle · 2018-02-22T05:38:58Z

I can understand the appeal of vendoring 7za.exe inside the module to solve this problem. There are a few downsides from a maintainers perspective however that we have to consider:

Licensing for 7-zip is GNU LGPL per http://www.7-zip.org/license.txt. IANAL, but I'm not sure that redistributing 7-zip would be compatible with our Apache 2 license (also see https://www.apache.org/licenses/GPL-compatibility.html). In any event, it's a complicated subject.
Including other projects that are not Puppet maintained incurs an additional ongoing cost from a security perspective. To even add 7-zip requires approval by the release team, which means evaluating a number of criteria, including the disclosure practices of 7-zip. If approved, active security triage is then required going forward, which could result in unscheduled releases.
While the 7-zip source code is available as a 7-zip package, it's not available in a public repository like GitHub. This does make it a bit more difficult if there's ever a need to build custom binaries, for instance in response to a critical vulnerability that doesn't yet have a release.

Given the above complexity, is there another way we could address the hardcoded URI? For instance, would it be enough to make it configurable, so that users can stage their own copy (for instance by placing it in a Code Manager control repo), and using a file:// style URI?

helge000 · 2018-02-22T10:44:59Z

@Iristyle, thanks for elaborating; makes much more sense to me now.

helge000 · 2018-02-22T10:46:48Z

manifests/init.pp

@@ -62,6 +66,7 @@
 class chocolatey (
  $choco_install_location         = $::chocolatey::params::install_location,
  $use_7zip                       = $::chocolatey::params::use_7zip,
+  $binary_7zip                    = $::chocolatey::params::binary_7zip,


@ferventcoder, are you ok with this new param name?

That doesn't self document what it is. Needs something like 7zip_download_url to make it consistent with the other params.

Certainly, though I cannot start a var name with a number. What about:
$download_url_7zip

Ouch. How about seven_zip_download_url? Although both would probably work, just weird workaround that folks may not immediately understand.

helge000 · 2018-02-22T10:50:16Z

@ferventcoder, I refactored to use a new parameter. I also tried to find the relevant docs. I feel I should also add a spec for the new file?

ferventcoder · 2018-02-22T12:57:48Z

@Iristyle way ahead of you on that (see JIRA) - but thank you for elaborating on the reasoning.

manifests/init.pp

glennsarti · 2019-03-07T03:12:28Z

README.md

@@ -793,6 +793,10 @@ If you override the default installation directory you need to set appropriate p

 Specifies whether to use the built-in shell or allow the installer to download 7zip to extract `chocolatey.nupkg` during installation. Valid options: `true`, `false`. Default: `false`.

+##### `seven_zip_download_url`
+
+Specifies the source file for `7za.exe`. Supports all sources supported by Puppet's `file {}` resource. You should use a 32bit binary for compatibility.  Defaults to `https://chocolatey.org/7za.exe`


nit: perhaps instead of file {} use [file](<url to file resource>)

spec/unit/puppet_x/chocolatey/chocolatey_install_spec.rb

michaeltlombardi · 2019-03-11T16:39:00Z

spec/unit/facter/choco_temp_dir.rb

+    Facter.clear_messages
+  end
+
+  context "on Windows", :if => Puppet::Util::Platform.windows? do


Should maybe replace with skips in the It blocks? CC @ThoughtCrhyme

michaeltlombardi · 2019-03-11T17:29:33Z

Passed Adhoc at: https://jenkins-master-prod-1.delivery.puppetlabs.net/view/modules/view/windows/view/_adhoc/view/chocolatey/job/forge-windows_puppetlabs-chocolatey_init-manual-parameters_adhoc/30/

Prior to this commit the 7zip executable, a requirement for the installation of Chocolatey, was hardcoded for download from a single internet URL. This caused failures when pointed through a proxy. This commit adds a new parameter, `$seven_zip_download_url` to the Chocolatey class. This is used to specify the path to the `7za.exe` binary, which can live anywhere that the Puppet File resource can retrieve it for the target machine. It uses such a file declaration in the `install.pp` manifest to handle cases where the use of 7zip is required. This parameter defaults to the previously hardcoded URL, `https://chocolatey.org/7za.exe`. This commit also adds a new fact, `choco_temp_dir`, to discover the appropriate temporary folder on disk in which to place the 7zip binary. It follows the pattern for using/testing facts as the `choco_install_path` fact already used in this module, by adding the logic for the fact in helper code. This commit therefore also includes new tests to the module for the helper code, the fact, the parameter, and the expected change in behavior. It also includes updated documentation per the new parameter.

helge000 commented Feb 16, 2018

View reviewed changes

templates/InstallChocolatey.ps1.erb Show resolved Hide resolved

helge000 force-pushed the MODULES-6652 branch from 387e394 to 9aeda4e Compare February 16, 2018 10:50

helge000 changed the title ~~Fixes MODULES-6652~~ (MODULES-6652) Fix download 7z behind proxy Feb 16, 2018

helge000 force-pushed the MODULES-6652 branch 4 times, most recently from 6c414ed to 50f1b38 Compare February 16, 2018 11:12

ferventcoder mentioned this pull request Feb 16, 2018

Hard coded 7z url defeats proxies chocolatey-archive/puppet-chocolatey#162

Closed

helge000 force-pushed the MODULES-6652 branch from 50f1b38 to 2c23510 Compare February 19, 2018 10:41

helge000 commented Feb 19, 2018

View reviewed changes

templates/InstallChocolatey.ps1.erb Show resolved Hide resolved

helge000 commented Feb 20, 2018

View reviewed changes

helge000 force-pushed the MODULES-6652 branch 2 times, most recently from 9491105 to de1d97f Compare February 22, 2018 10:41

helge000 commented Feb 22, 2018

View reviewed changes

helge000 force-pushed the MODULES-6652 branch from de1d97f to 823f103 Compare February 22, 2018 15:47

helge000 commented Feb 22, 2018

View reviewed changes

manifests/init.pp Show resolved Hide resolved

helge000 force-pushed the MODULES-6652 branch from 823f103 to d3f89d4 Compare February 22, 2018 17:46

michaeltlombardi force-pushed the MODULES-6652 branch from d3f89d4 to ee8806f Compare March 7, 2019 02:55

glennsarti reviewed Mar 7, 2019

View reviewed changes

michaeltlombardi force-pushed the MODULES-6652 branch from ee8806f to 8f1584f Compare March 7, 2019 15:54

michaeltlombardi force-pushed the MODULES-6652 branch 7 times, most recently from c37317e to d6db7cd Compare March 8, 2019 22:02

ThoughtCrhyme previously approved these changes Mar 8, 2019

View reviewed changes

spec/unit/puppet_x/chocolatey/chocolatey_install_spec.rb Outdated Show resolved Hide resolved

michaeltlombardi dismissed ThoughtCrhyme’s stale review via 0041283 March 8, 2019 23:28

michaeltlombardi force-pushed the MODULES-6652 branch 2 times, most recently from 0041283 to 7e6db24 Compare March 8, 2019 23:37

michaeltlombardi reviewed Mar 11, 2019

View reviewed changes

michaeltlombardi force-pushed the MODULES-6652 branch from 7e6db24 to f50315d Compare March 11, 2019 17:57

ThoughtCrhyme approved these changes Mar 11, 2019

View reviewed changes

ThoughtCrhyme merged commit e743906 into puppetlabs:master Mar 11, 2019

LukasAud added the feature label May 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(MODULES-6652) Fix download 7z behind proxy #94

(MODULES-6652) Fix download 7z behind proxy #94

helge000 commented Feb 16, 2018

ferventcoder commented Feb 16, 2018

ferventcoder commented Feb 16, 2018

helge000 commented Feb 16, 2018

ferventcoder commented Feb 19, 2018

helge000 Feb 20, 2018

ferventcoder Feb 20, 2018

jpogran commented Feb 20, 2018

ferventcoder commented Feb 21, 2018

Iristyle commented Feb 22, 2018

helge000 commented Feb 22, 2018

helge000 Feb 22, 2018

ferventcoder Feb 22, 2018

helge000 Feb 22, 2018 •

edited

Loading

ferventcoder Feb 22, 2018

helge000 commented Feb 22, 2018

ferventcoder commented Feb 22, 2018

glennsarti Mar 7, 2019

michaeltlombardi Mar 11, 2019

michaeltlombardi commented Mar 11, 2019

(MODULES-6652) Fix download 7z behind proxy #94

(MODULES-6652) Fix download 7z behind proxy #94

Conversation

helge000 commented Feb 16, 2018

ferventcoder commented Feb 16, 2018

ferventcoder commented Feb 16, 2018

helge000 commented Feb 16, 2018

ferventcoder commented Feb 19, 2018

helge000 Feb 20, 2018

Choose a reason for hiding this comment

ferventcoder Feb 20, 2018

Choose a reason for hiding this comment

jpogran commented Feb 20, 2018

ferventcoder commented Feb 21, 2018

Iristyle commented Feb 22, 2018

helge000 commented Feb 22, 2018

helge000 Feb 22, 2018

Choose a reason for hiding this comment

ferventcoder Feb 22, 2018

Choose a reason for hiding this comment

helge000 Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

ferventcoder Feb 22, 2018

Choose a reason for hiding this comment

helge000 commented Feb 22, 2018

ferventcoder commented Feb 22, 2018

glennsarti Mar 7, 2019

Choose a reason for hiding this comment

michaeltlombardi Mar 11, 2019

Choose a reason for hiding this comment

michaeltlombardi commented Mar 11, 2019

helge000 Feb 22, 2018 •

edited

Loading