module.run state not passing arguments

[Joe N]

Hi,

I have the following state, which was working OK prior to upgrading to 2016.11:

import-gpg-key:

  module.run:

    - name: gpg.import_key

    - filename: '/etc/salt/gpgkeys/mykey.asc'

    - user: salt

    - require:

      - file: /etc/salt/gpgkeys/mykey.asc

      - pkg: global-pkgs

When I run via state.apply, I get this error:

Module function gpg.import_key threw an exception. Exception: filename or text must be passed.

I copied the module file gpg.py to _modules/ and added a log statement here: https://github.com/saltstack/salt/blob/2016.11/salt/modules/gpg.py#L724

The log statement showed that both filename and text parameters were None

When I run the module from the CLI, the parameters are received and everything works:

sudo salt db1 gpg.import_key filename=/etc/salt/gpgkeys/mykey.asc user=salt

db1:

    ----------

    message:

        Key(s) already exist in keychain.

    res:

        True

My python knowledge is pretty lacking so can anyone point me in the right direction of fixing this?

Salt version:

sudo salt --versions-report

Salt Version:

           Salt: 2016.11.3

Dependency Versions:

           cffi: Not Installed

       cherrypy: 3.8.0

       dateutil: 2.4.2

          gitdb: 0.6.4

      gitpython: 1.0.1

          ioflo: Not Installed

         Jinja2: 2.8

        libgit2: Not Installed

        libnacl: Not Installed

       M2Crypto: 0.21.1

           Mako: Not Installed

   msgpack-pure: Not Installed

 msgpack-python: 0.4.6

   mysql-python: Not Installed

      pycparser: Not Installed

       pycrypto: 2.6.1

         pygit2: Not Installed

         Python: 2.7.12 (default, Nov 19 2016, 06:48:10)

   python-gnupg: 0.3.8

         PyYAML: 3.11

          PyZMQ: 15.2.0

           RAET: Not Installed

          smmap: 0.9.0

        timelib: 0.2.4

        Tornado: 4.4.1

            ZMQ: 4.1.4

System Versions:

           dist: Ubuntu 16.04 xenial

        machine: x86_64

        release: 4.4.0-64-generic

         system: Linux

        version: Ubuntu 16.04 xenial

[Felix Dreissig]

Hi,

On 16. Mar 2017, at 09:17, Joe N <j...@originalmind.com.au> wrote:
> import-gpg-key:
> module.run:
> - name: gpg.import_key
> - filename: '/etc/salt/gpgkeys/mykey.asc'
> - user: salt
> - require:
> - file: /etc/salt/gpgkeys/mykey.asc
> - pkg: global-pkgs
>
> When I run via state.apply, I get this error:
>
> Module function gpg.import_key threw an exception. Exception: filename or text must be passed.

For everyone running into this as well:

The situation regarding `module.run` looks pretty confusing at the moment. Basically, a new syntax was introduced with 2017.7.0 [1], which Joe appears to be using.
The docs [2] could do better at distinguishing between the old and new versions imho and on top of that, docs for previous releases appear to be currently inaccessible (links in the upper right do not work and links in the footer are 403).

What you have to do with older releases (or without the new version explicitly enabled through `use_superseded`), is use the old version of `module.run`, where certain arguments have to be passed in a `kwargs` dictionary.
Joe’s example would look something like this:

import-gpg-key:
module.run:
- name: gpg.import_key

- kwargs:
filename: '/etc/salt/gpgkeys/mykey.asc'

user: salt
- require:
- file: /etc/salt/gpgkeys/mykey.asc
- pkg: global-pkgs

Best regards,
Felix

[1] https://docs.saltstack.com/en/develop/topics/releases/2017.7.0.html#state-module-changes
[2] https://docs.saltstack.com/en/develop/ref/states/all/salt.states.module.html

[Bo Maryniuk]

Guys, blame me for this, since I wrote it. :-)
The old syntax you can get back by setting "use_deprecated:
modules.run" option in your config. But this will stop working two
years later — enough time to migrate. The reasons that was changed,
because it had many magic keywords and we ran into a various clashes
when developing/improving docker support, cloud etc, and that required
mapping/translations of those keywords in hope that nobody will hit it
all over again. But then we thought: why not just stop all that thing
and simply avoid by design? So the new syntax born. And it is actually
much better, because:

1. It is pure Yaml, forget JSON-like inclusions with '{...}' things.
2. It is simpler, no need to use quotes on strings etc.

Additionally, module.run now can perform unlimited calls in one go.
That is, you can run many commands in one set, unlike before.
Basically, please read the manual carefully and change your
'module.run' calls accordingly.
In case you still have questions, go ahead asking them. But I am sure
they are likely because you didn't read manual properly. :-)

> --
> You received this message because you are subscribed to the Google Groups "Salt-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to salt-users+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/salt-users/CFE12CCD-5FE0-4CA3-A77D-8CDAAECCAF00%40f30.me.
> For more options, visit https://groups.google.com/d/optout.

[Felix Dreissig]

Hi,

thanks a lot for your remarks, Bo. However, I am a bit confused:

On 7. Aug 2017, at 10:31, Bo Maryniuk <b...@maryniuk.net> wrote:
> The old syntax you can get back by setting "use_deprecated:
> modules.run" option in your config. But this will stop working two
> years later — enough time to migrate.

According to the 2017.7.0 release notes [1] and the 2017.7 docs [2], the new syntax must be explicitly enabled using `use_superseded` (opt-in vs. opt-out). I would also expect the old syntax to be deprecated at some point, but at the moment, no steps in that direction seem to have been taken.

> But then we thought: why not just stop all that thing
> and simply avoid by design? So the new syntax born. And it is actually
> much better, because:
>
> 1. It is pure Yaml, forget JSON-like inclusions with '{...}' things.
> 2. It is simpler, no need to use quotes on strings etc.

I fully agree that the new syntax is much more intuitive.

That being said, I don’t get the point about YAML vs. JSON: As far as I understand, the curly braces from the docs were just an alternative syntax for a YAML dictionary.
In the example I prepared for Joe, the "nicer" dictionary syntax (without curly braces) is used and that appears to work just fine.

> Basically, please read the manual carefully and change your
> 'module.run' calls accordingly.
> In case you still have questions, go ahead asking them. But I am sure
> they are likely because you didn't read manual properly. :-)

I think "carefully“ is the crucial point now: If you read the docs from top to bottom, you will probably get it.
However, if you expect the examples from the top (or anywhere else for that matter) to be copy/paste-able, you will run into problems since the note about `use_superseded` is buried at the bottom.

Additionally, the examples are kind of mixed with no clear distinction: It starts with some new-style code, then there is some old-style code after "With the previous module.run there are several differences" and after "Another option is to use the new version of module.run" the new version is used again, this time with the not about `use_superseded`.
I think some headings would help greatly here.

That the signature for `module.run` is completely missing [3] is also disadvantageous, but doesn’t make a huge difference.

Best regards,
Felix

[1] https://docs.saltstack.com/en/develop/topics/releases/2017.7.0.html#state-module-changes

[2] https://docs.saltstack.com/en/2017.7/ref/states/all/salt.states.module.html
[3] https://github.com/saltstack/salt/issues/42260