Closed mdaffin closed 5 years ago
@mdaffin looking at the code here and when i just pass that through using the python salt-api as shown below it works:
>>> local.cmd('thecakeisalie', 'state.orchestrate', ['orch.ping'], pillar='{"target": "web-*"}')
{'thecakeisalie': {'outputter': 'highstate', 'data': {'thecakeisalie': {'salt_|-cmd.run_|-cmd.run_|-function': {'comment': 'Function ran successfully. Function cmd.run ran on thecakeisalie.', 'name': 'cmd.run', 'start_time': '16:00:15.792520', 'result': True, 'duration': 152.273, '__run_num__': 0, '__jid__': '20170302160015811048', 'changes': {'ret': {'thecakeisalie': 'test'}, 'out': 'highstate'}, '__id__': 'cmd.run'}}}, 'retcode': 0}}
Can you print out kwargs and args on that command and see how its trying to pass this to localclient?
It might not be working because it looks like this engine is doing it's own name=val parsing and at quick glance it's not clear to me why. LocalClient also performs that work in addition to running args through parse_input
(which is what turns name=value
pairs into a data structure) so I think we could just use that existing behavior....BUT I'm also not clear on why we're relying on the string parsing at all since we can pass in a real data structure from YAML.
I might be missing something, but I think the cmd
argument should preserve backward-compat but be changed to also optionally be a dict and bypass the custom parsing entirely. E.g.:
cmd:
- fun: state.orchestrate
- kwarg:
mods: orch.ping
pillar:
target: 'web-*'
@Ch3LL output as requested:
cmd: state.orchestrate
args: ['orch.ping']
kwargs: {'pillar': '{"target": "web-*"}'}
@whiteinge it does look like that is the case. The above confirms it is just treating the part after the =
as a string rather then decoding it like it does on the command line. I do like your suggestion of allowing full yaml support in the config but it does not solve the problem for non aliased messages from slack, such as !state.orchestrate orch.ping pillar='{"target":"web-*"}'
. Ideally it should parse these in the same way as they are on the command line.
Note that since it is a runner command this is what is passed to the runner.cmd
rather then the local.cmd
Oh! facepalm I see now why this module is using that format. I was thinking SLS files but it makes sense that it's parsing textual messages from Slack. Thanks, that makes sense now.
In that case, I think we can get this all the way working by simply packing all positional args into LocalClient
's arg array. E.g., LocalClient().cmd('*', 'state.sls', arg=['one', 'two=true', 'three={four: Four}'])
. As of #39472 that will also work happily for RunnerClient
too.
I am struggling to understand the decision to can to--what would be the process by which one would pass inline pillar data to an orchestration command via slack?
@nomeelnoj at the moment it looks like only string values can be given as arguments so I don't think it's possible to send Pillar data (which must be a dictionary) from either Slack or as an alias until this change is implemented.
Would it be better if the SlackClient parsed messages for aliases or some other special SlackClient syntax but fell back on using salt.client.caller.cmd() parsing since the slack engine commands seem to set up the expectation this is how it works?
@aphor (Stop me if I'm misreading your question) in my opinion:
Commands from Slack should ideally have exactly the same format as the Salt CLI. E.g., typing salt '*' state.sls pillar='{foo: Foo}'
into Slack can have all the same parsing, behavior, and semantics as the CLI if the positional args are packed into the arg
array (as mentioned above).
The Slack-specific aliases and whitelisting would just be value-adds on top of that native behavior to allow for common shortcuts and some light security.
@whiteinge I think you understood me well. Thanks for the answer.
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.
Description of Issue/Question
Pillar data cannot be passed to the slack engine from slack or the engine config resulting in
Pillar data must be formatted as a dictionary
error message.I expected it to work the same way as the command line. ie you should be able to copy and paste a command from the command line into slack or the
cmd
line in the alias.Setup
/srv/salt/orch/ping.sls
/etc/salt/master.d/slack-engine.conf
Steps to Reproduce Issue
From the salt master running
salt-run state.orchestrate orch.ping pillar='{"target": "web-*"}'
works as expected.In slack running
!state.orchestrate orch.ping pillar='{"target":"web-*"}'
or!ping-web
produces the following error in slack.I have tried a variety of different pillar data, including formatting is as a python dict
'{target: "web-*"}'
.Versions Report
We also saw this in the previous version (2016.11.2).