Remove whitespace from string commands

[twangboy]

What does this PR do?

Strips whitespace from string commands.
Adds -NoProfile switch to powershell commands

What issues does this PR fix or reference?

#34054
#16872

Previous Behavior

Commands that had a hard return at the end were producing erroneous quoting. This specifically applies to state files using multiline commands.
PowerShell commands and scripts were loading local powershell profiles. Individualized settings are stored in powershell profiles and can introduce unexpected items into the powershell environment.

New Behavior

Whitespace is removed if the cmd is a string.
Powershell Profiles are not loaded.

Tests written?

No

[terminalmage]

Can you explain why this would be necessary? We use shlex.split() to convert the command to a list to be passed to the timed_subprocess instance, and splitting like this already strips leading and trailing whitespace:

>>> import shlex
>>> shlex.split('   foo bar "baz foo"  ')
['foo', 'bar', 'baz foo']
>>>

Furthermore, subprocess.Popen() doesn't care if there is leading/trailing whitespace:

>>> import subprocess
>>> p = subprocess.Popen('   echo "hello world"  ', shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>>> p.communicate()
('hello world\n', '')
>>>

So, is there a specific reason for this? Something perhaps related to calling cmd.script using Powershell?

[twangboy]

@terminalmage

So, here's the YAML:

Failing-Example:
  cmd.run:
    - name: >
        Clear-Host;
        Write-Host "hello salt"
    - shell: powershell
    - runas: testuser01
    - password: PassWord1!

I don't know if it's a YAML problem or what. It looks like when you do multiline in YAML it adds a newline at the end or something. This is the command after shellex before this fix:

Powershell -NonInteractive "Clear-Host; Write-Host \"hello salt\"
"

And here's the command after this fix:

Powershell -NonInteractive "Clear-Host; Write-Host \"hello salt\""

With runas on Windows it's not using subprocess.Popen() as that doesn't support runas on Windows.

[terminalmage]

But if this is limited to Windows with runas, why put this code at the top of the function where it'll be run on all minions? Why not just put it in this block so it's confined to the cases where it'll actually be needed?

For that matter, I don't understand why you are splitting and joining again here. This seems like needless extra work.

[twangboy]

It wasn't a split... it was a strip...

Anyway, I just pushed a more complicated... solution. Let me know what you think.

[terminalmage]

You didn't follow the actual link that I posted when I referred to the split. Please do that before arguing with me.

[terminalmage]

I don't know why you felt the need to edit all that unrelated code in your latest commit.

[terminalmage]

See #37009 for what I was talking about. Can you please test that code and let me know if it accomplishes what you were trying to accomplish?

[twangboy]

@terminalmage I did follow it, but it didn't actually take me to a specific line number so I wasn't sure what you were talking about. I assume you meant the the shlex.split... If I recall correctly, Mike had me add that for Shell Injection checking or something...

Anyways, yours works too. And you don't need the cmd.strip() in the call to win_runas. It must have been the shlex.split() that was causing the problem... So, if you don't need the shlex for shell injection checking, then by all means merge yours.

[terminalmage]

I'll double-check on the shell injection testing, if it's true then we can leave it in and then document it with a comment so that someone else doesn't see it and get the same idea I did and try to remove it.

[twangboy]

@terminalmage I moved the whitespace strip to the powershell block. It looks like this is where it was getting that extra hard return that was causing the problems. Let me know what you think.

[terminalmage]

OK, this looks better, we can address whether or not we still need to split/rejoin separately.