Salt: pkgrepo.managed ppa creates sources list with invalid filenames

[azylman]

Description of Issue/Question

pkgrepo.managed ppa creates sources list with invalid filenames

Setup

I had a salt state that resembled:

pkgrepo.managed:
  - ppa: avsm/ocaml42+opam12

It created a .list file in /etc/apt/sources.list.d/ named after the PPA, including the plus sign. However, plus signs aren't valid in names of apt data source files, which resulted in apt ignoring the file. From the man page:

The /etc/apt/sources.list.d directory provides a way to add sources.list entries in separate files. The format is the same as for the regular sources.list file. File names need to end with .list and may only contain letters (a-z and A-Z), digits (0-9), underscore (_), hyphen (-) and period (.) characters. Otherwise APT will print a notice that it has ignored a file

I was able to work around it by explicitly providing a file name in the state, but it would be nice if it worked by default.

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

Versions Report

$ salt --versions-report
Salt Version:
           Salt: 2015.8.3

Dependency Versions:
         Jinja2: 2.8
       M2Crypto: 0.21.1
           Mako: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.1
         Python: 2.7.10 (default, Oct 14 2015, 16:09:02)
           RAET: Not Installed
        Tornado: 4.3
            ZMQ: 4.0.5
           cffi: 1.1.2
       cherrypy: Not Installed
       dateutil: 2.5.2
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
         pygit2: Not Installed
   python-gnupg: Not Installed
          smmap: Not Installed
        timelib: Not Installed

System Versions:
           dist: Ubuntu 15.10 wily
        machine: x86_64
        release: 4.2.0-35-generic
         system: Ubuntu 15.10 wily

[jfindlay]

@azylman, thanks for reporting. You should be able to work around this by using the file argument to the pkgrepo.managed state function.

[abednarik]

HI

Looks like is already fixed in 2015.8

root@ubuntu:/etc/apt/sources.list.d# cat /srv/salt/source.sls
base:
  pkgrepo.managed:
    - ppa: avsm/ocaml42+opam12
root@ubuntu:/etc/apt/sources.list.d# ls -l  /etc/apt/sources.list.d/
total 12
-rw-r--r-- 1 root root 142 Apr 15 13:16 avsm-ocaml42_opam12-trusty.list
-rw-r--r-- 1 root root  65 Apr 15 13:16 platform-repository.list
-rw-r--r-- 1 root root  65 Apr 15 13:15 platform-repository.list.save

[azylman]

2015.8.what? I'm using 2015.8.3.

[azylman]

I'm trying now with 2015.8.8 and I'll report back

[azylman]

I just tried this with 2015.8.8 and saw the same behavior:

$ salt --versions-report && ls -l /etc/apt/sources.list.d/
Salt Version:
           Salt: 2015.8.8

Dependency Versions:
         Jinja2: 2.8
       M2Crypto: 0.21.1
           Mako: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.1
         Python: 2.7.10 (default, Oct 14 2015, 16:09:02)
           RAET: Not Installed
        Tornado: 4.3
            ZMQ: 4.0.5
           cffi: 1.1.2
       cherrypy: Not Installed
       dateutil: 2.5.2
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
        libgit2: Not Installed
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
         pygit2: Not Installed
   python-gnupg: Not Installed
          smmap: Not Installed
        timelib: Not Installed

System Versions:
           dist: Ubuntu 15.10 wily
        machine: x86_64
        release: 4.2.0-34-generic
         system: Ubuntu 15.10 wily

total 12
-rw-r--r-- 1 root root 66 Apr 15 19:40 avsm-ocaml42+opam12-wily.list
-rw-r--r-- 1 root root 69 Apr 15 19:40 docker.list
-rw-r--r-- 1 root root 41 Apr 15 19:40 heroku.list

[notpeter]

Similarly, if you don't specify a file, but do specify clean_file you get a traceback:

nginx_repo:
  pkgrepo.managed:
    - ppa: nginx/stable
    - clean_file: True

Traceback:

----------
          ID: nginx_repo
    Function: pkgrepo.managed
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/dist-packages/salt/state.py", line 1624, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1492, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/dist-packages/salt/states/pkgrepo.py", line 386, in managed
                  salt.utils.fopen(kwargs['file'], 'w').close()
              KeyError: 'file'
     Started: 17:46:43.244013
    Duration: 1255.888 ms
     Changes:   

This worked just fine with previous salt versions but is now broken under 2015.8.10.

It's fine if you want to make file a required keyword, but the pkgrepo docs should be updated to reflect that. Specifically all the ppa examples.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.