voxpupuli / puppet-vmwaretools

Puppet module to manage VMware Operating System Specific Packages for VMware tools installation.
http://forge.puppetlabs.com/razorsedge/vmwaretools
Apache License 2.0
34 stars 44 forks source link

Incorrect repo url for Suse 11.3 #57

Open akomakom opened 7 years ago

akomakom commented 7 years ago

manifests/repo.pp constructs the following (invalid) url on SLES11.3:

http://packages.vmware.com/tools/esx/latest/sles11.3/x86_64/ It uses $::operatingsystemrelease in the string which is "11.3" on this version.

The correct URL would be: https://packages.vmware.com/tools/esx/latest/sles11sp3/x86_64/ (note "11sp3" instead of "11.3")

If I get this working properly I may make a PR

akomakom commented 7 years ago

Just changed line 216 of params.pp to:

$distrelease = "${::os[release][major]}sp${::os[release][minor]}"

However, there is another problem. The vmware repo URL now redirects to https://, and on SuSE 11.3 the rpm binary can't handle SSL. In other words this doesn't work:

      exec { 'vmware-import-gpgkey':
        path        => '/bin:/usr/bin:/sbin:/usr/sbin',
        command     => "rpm --import ${gpgkey}",
        refreshonly => true,
      }

The safe way to make this work is to wget the key first and then import a local file. @razorsedge, what would you recommend for a pull request: adding a wget requirement or using some download_file type module, or some other option?

razorsedge commented 7 years ago

@akomakom There should be plenty of modules that implement a file download. No need to re-invent one.

The ::os[release][major] is that a Puppet 4 facter construct?

Does changing line 216 also work for other SuSE versions?

akomakom commented 7 years ago

@razorsedge I certainly have no plans to roll my own download module, I was referring to puppet/download_file as an example, but upon closer inspection that turns out to be a windows-only module. There are plenty out there.

Your other questions bring up a good point: hash facts (ie $os[release][major]) are supported in puppet 3 (possibly even in later 2.x), but their use requires puppet.conf setting of stringify_facts = false, which may be an unreasonable expectation for all clients. It becomes the default behavior in puppet 4. Unfortunately, I didn't see the value "3" (from SuSE 11.3) in any other fact. Maybe it's safer to string-replace the fact we have (11.3 -> 11sp3).

The URL structure seems valid for all 11.x and 9.x suse versions: https://packages.vmware.com/tools/esx/latest/index.html

For 12.x this module doesn't apply anyway.

akomakom commented 7 years ago

I just confirmed that my original change also works for SLES 11.4 (but again, it relies on stringify_facts = false so may not work in some Puppet 3.x deployments).

The change, for clarity, was:

Index: manifests/params.pp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- manifests/params.pp (revision 3868)
+++ manifests/params.pp (revision 3953)
@@ -255,7 +255,7 @@
         'SLES', 'SLED': {
           # TODO: tools 3.5 and 4.x use either sles11 or sles11sp1 while tools >=5 use sles11.1
           if ($majdistrelease == '9') or  ($majdistrelease == '11') {
-            $distrelease = $::operatingsystemrelease
+            $distrelease = "${::os[release][major]}sp${::os[release][minor]}"
           } else {
             $distrelease = $majdistrelease
           }

The original problem still remains - you have to perform rpm --import VMWARE-PACKAGING-GPG-RSA-KEY.pub yourself because (at least on some SLES 11.4 installs I have) rpm can't handle being redirected to https.