Salt: archive.extracted ValueError "No path specified" in 2017.7.3

Created on 7 Feb 2018 · 14Comments · Source: saltstack/salt

Description of Issue/Question

archive.extracted seems to be broken in 2017.7.3 w.r.t cached values. Debug log shows the error manifesting in 2017.7.3. This issue is not present in 2016.11.9. Both versions were released on Feb 5th 2018. I've been developing a new salt infrastructure just using "stable" with the bootstrap script and noticed my old working state was no longer working. If previous stable was 2017.7.1 then this would be a regression.

Setup

# /etc/salt/minion
master: master
environment: devenv
log_level: info
state_aggregate:
  - pkg
  - pip

# wkhtmltopdf.sls
wkhtmltopdf-pkgs:
  pkg.installed:
    - names:
      - libxrender1
      - libfontconfig1

wkhtmltopdf-tar:
  file.directory:
    - name: /opt/wkhtmltopdf
  archive.extracted:
    - name: /opt/wkhtmltopdf
    - source: https://github.com/wkhtmltopdf/wkhtmltopdf/releases/download/0.12.4/wkhtmltox-0.12.4_linux-generic-amd64.tar.xz
    - source_hash: https://github.com/wkhtmltopdf/wkhtmltopdf/releases/download/0.12.4/SHA256SUMS                                                              
    - source_hash_name: "*wkhtmltox-0.12.4_linux-generic-amd64.tar.xz"
    - source_hash_update: True
    - require:
      - pkg: wkhtmltopdf-pkgs
      - file: wkhtmltopdf-tar
    - onchanges_in:
      - rsync: wkhtmltopdf-rsync

wkhtmltopdf-rsync:
  rsync.synchronized:
    - delete: false
    - source: /opt/wkhtmltopdf/wkhtmltox/
    - name: /usr/

Steps to Reproduce Issue

Debug log

Versions Report

Salt Version:
           Salt: 2017.7.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (default, Dec  4 2017, 14:50:18)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.4.0-93-generic
         system: Linux
        version: Ubuntu 16.04 xenial

Bug P2 Regression ZRELEASED - 2017.7.4 fixed-pending-your-verification

Source

CrackerJackMack

👍1

Most helpful comment

Ahh, you had keep: False, so it would have deleted the file at the end of the state anyway. But I figured it out.

What's happening here is that 2017.7.3 changed how we managed cached archives to avoid repeated downloads for remote files. So we use a newly-added state called file.cached to ensure the file is present in the minion file cache. Afterward, we use cp.is_cached to get the path to the file in the minion cache. This function returns an empty string if the file is not cached.

The bug here is that when we run cp.is_cached we are not passing a saltenv, so base is assumed. However, when caching the file, we use the effective saltenv. So, any non-base fileserver environment fails this lookup, because cp.is_cached is (incorrectly) looking for the file in the base environment. @CrackerJackMack is using a devenv saltenv, and @jellisii is using a xenial environment, which is why both are affected.

This is a one-line fix: https://github.com/saltstack/salt/pull/45902/commits/6cc5cd9b8a7b573a50bd550e05c5c87fe62b8501

For @jellisii, since you are using a salt:// URL, you can work around this immediately by explicitly adding the saltenv to the source URL:

rkhunter_installer:
    archive.extracted:
        - name: /tmp/
        - source: salt://files/rkhunter-1.4.4.tar.gz?saltenv=xenial
        - if_missing: /usr/local/bin/rkhunter
        - keep: false

This is because cp.is_cached will use the saltenv passed in the the salt:// URL if present.

Another workaround (and this will be the only workaround for @CrackerJackMack because of the http(s) source) would be to download this file (which is the 2017.7.3 version of the archive state module, with the fix applied) and copy it into the salt fileserver under salt://_states/archive.py. Then, use saltutil.sync_states to push the fixed copy of the state module out to the minions. Note that when using this workaround, you'll want to delete that file and re-run the sync_states to un-deploy the hotfix.

terminalmage on 7 Feb 2018

🎉8 👍3

All 14 comments

Can verify, have the same problem.

Setup

I have no custom config on the minion, but the minion is running Ubuntu 16.04, where as the master is running Ubuntu 14.04. I've redacted the rest of this state file because it holds no bearing on the problem at hand.

rkhunter_installer:
    archive.extracted:
        - name: /tmp/
        - source: salt://files/rkhunter-1.4.4.tar.gz
        - if_missing: /usr/local/bin/rkhunter
        - keep: false

Steps to Reproduce Issue

Debug log

Versions Report

Master

Salt Version:
           Salt: 2017.7.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: 0.5.4
      gitpython: 0.3.2 RC1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 0.9.1
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.2.3
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.6 (default, Nov 23 2017, 15:49:48)
   python-gnupg: Not Installed
         PyYAML: 3.10
          PyZMQ: 14.0.1
           RAET: Not Installed
          smmap: 0.8.2
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: Ubuntu 14.04 trusty
         locale: UTF-8
        machine: x86_64
        release: 4.14.12-x86_64
         system: Linux
        version: Ubuntu 14.04 trusty

Minion

Salt Version:
           Salt: 2017.7.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: 1.3.7
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (default, Dec  4 2017, 14:50:18)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.14.14-x86_64
         system: Linux
        version: Ubuntu 16.04 xenial

jellisii on 7 Feb 2018

yes, I can confirm this bug for Debian Stretch.

"You made my day" - every time you publish an update ...
I tested extensive Saltstack changes / justify of packages/states for some weeks to upgrade from Jessie to Stretch with all needed external repos/packages and was finally succesfull setup my instances several time in one run only.
Then today my CTO was checking my setup so we can next weeks rollout the changes and nothing works... (like last 2 years 3 times before same way; this is not funny)

After finding out that

this bug has no clear reason (for my tested 2017.7.2 setup) and
that also a gem install of "compass" needs now additional Debian package "libffi-dev" setup

it was my first check if a new saltstack version was published ... and strike ... :(

Reiner030 on 7 Feb 2018

Unfortunately I am unable to replicate either of the examples that have been provided :/

https://gist.github.com/gtmanfred/95697af8f217153dba92da1fa7eb71e3

Can you provide us with any other information on what to run to replicate this issue?

@terminalmage can you take a look at this?

Thanks,
Daniel

gtmanfred on 7 Feb 2018

@gtmanfred I don't know the significance here, but I was testing states with salt-call state.apply on the minion (not a masterless setup FYI)

CrackerJackMack on 7 Feb 2018

I tested with salt-call as well, and got the same result.

also tested ubuntu 14.04 and 16.04, i did not test debian, because no state was provided for that comment.

gtmanfred on 7 Feb 2018

@CrackerJackMack Can you run a cp.is_cached https://github.com/wkhtmltopdf/wkhtmltopdf/releases/download/0.12.4/wkhtmltox-0.12.4_linux-generic-amd64.tar.xz and see if it returns the cached path?

@jellisii Can you run a cp.is_cached salt://files/rkhunter-1.4.4.tar.gz and see if it returns the cached path?

terminalmage on 7 Feb 2018

[MYBOX] root@mybox:~# salt-call cp.is_cached salt://files/rkhunter-1.4.4.tar.gz
local:

jellisii on 7 Feb 2018

Ahh, you had keep: False, so it would have deleted the file at the end of the state anyway. But I figured it out.

This is a one-line fix: https://github.com/saltstack/salt/pull/45902/commits/6cc5cd9b8a7b573a50bd550e05c5c87fe62b8501

For @jellisii, since you are using a salt:// URL, you can work around this immediately by explicitly adding the saltenv to the source URL:

rkhunter_installer:
    archive.extracted:
        - name: /tmp/
        - source: salt://files/rkhunter-1.4.4.tar.gz?saltenv=xenial
        - if_missing: /usr/local/bin/rkhunter
        - keep: false

This is because cp.is_cached will use the saltenv passed in the the salt:// URL if present.

terminalmage on 7 Feb 2018

🎉8 👍3

@terminalmage thanks for the super quick turn around! I can confirm the fix works with the instructions provided. I don't know if this warrants a hotfix release or what the policy is related to break fixes but super easy to back out as you state.

CrackerJackMack on 7 Feb 2018

@CrackerJackMack Thanks for the confirmation. Sorry about this, I always try to take care to code for edge cases and this one was frustrating to have missed. I am in the process of adding an automated test though to ensure this doesn't regress again in the future.

terminalmage on 7 Feb 2018

Workaround works as expected. Thanks for the heads up @terminalmage and the quick response @gtmanfred

jellisii on 7 Feb 2018

Regression test added to the pull-request: https://github.com/saltstack/salt/pull/45902

terminalmage on 7 Feb 2018

I get the same problem，I modify the archive.py，It work. Thanks @terminalmage

richardweib on 8 Feb 2018

👍1

@terminalmage, @richardweib and all this workaround did not work for me. Can someone please provide step by step directions?
I don't have an _states folder in my root directory so I created one and added the archive.py there.
I noticed I have an archive.py in this location usr/lib/python2.7/site-packages/salt/states/archive.py and I updated it there as well.
Then I did salt_sync.util on the servers.
What am I doing wrong before I attempt to upgrade to 7.4 where presumably this bug is fixed.
Thanks!