Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows Container support #96

Closed
wants to merge 8 commits into from
2 changes: 2 additions & 0 deletions changelogs/fragments/96-windows-support.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- docker connection plugin - added support for Windows containers (https://github.com/ansible-collections/community.docker/pull/96).
51 changes: 47 additions & 4 deletions plugins/connection/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ def __init__(self, play_context, new_stdin, *args, **kwargs):

# Windows uses Powershell modules
if getattr(self._shell, "_IS_WINDOWS", False):
self.has_native_async = True
self.module_implementation_preferences = ('.ps1', '.exe', '')
self.allow_executable = False

if 'docker_command' in kwargs:
self.docker_cmd = kwargs['docker_command']
Expand Down Expand Up @@ -214,7 +216,11 @@ def exec_command(self, cmd, in_data=None, sudoable=False):
""" Run a command on the docker host """
super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable)

local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd])
if getattr(self._shell, "_IS_WINDOWS", False):
sudoable = False
local_cmd = self._build_exec_cmd(["cmd", "/c", cmd])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong but I believe using cmd /c here will mean any raw task with the connection plugin will be using cmd as the shell rather than PowerShell. You may want to change this based on whether the ansible_shell_type is either cmd or powershell to allow a user to change this as they desire.

Copy link
Author

@tmeckel tmeckel May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jborean93 yes this is true. Bit when I change the code as follows I receive a file not found. So the interpreter (Powershell) cannot be found. So I suspect I didn't grasp what you meant by saying I should utilize the ansible_shell_type. Any guidance?

image

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what the default shell is for a Windows container, i.e. what runs when you do docker exec .... Is it cmd or is it powershell.exe, or maybe no shell at all?

Basically right now if someone was to do the following

- raw: echo "hi"

Is that going to be running under cmd or powershell? With the current changes here that will be running as cmd which might not be ideal if someone has set ansible_shell_type=powershell. Maybe the desire is to have raw with this connection plugin independent to the shell plugin used, if that's the case then the PR as it is makes sense. If you wish for this to be dependent on the shell plugin then you will have to hardcode cmd.exe /c when using the cmd shell and powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -Command for the powershell shell.

This might be something that @felixfontein may want to comment on.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I can say something here. I've never used Docker with Windows containers, so I have zero experience what these contain.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly mean more in general, do you expect that raw should match the ansible_shell_type (powershell or cmd) or should it just be the same thing on the connection plugin?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matching ansible_shell_type would be nice I guess, though I'm not even sure whether Linux containers fully support that (I've never really looked at shell plugins). I also have no idea how complicated it would be to 'properly' implement this.

else:
local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd])

display.vvv(u"EXEC {0}".format(to_text(local_cmd)), host=self._play_context.remote_addr)
display.debug("opening command with Popen()")
Expand Down Expand Up @@ -268,6 +274,12 @@ def exec_command(self, cmd, in_data=None, sudoable=False):
display.debug("done communicating")

display.debug("done with docker.exec_command()")

# When running on Windows, stderr may contain CLIXML encoded output
if getattr(self._shell, "_IS_WINDOWS", False) and stderr.startswith(b"#< CLIXML"):
from ansible.plugins.shell.powershell import _parse_clixml
tmeckel marked this conversation as resolved.
Show resolved Hide resolved
stderr = _parse_clixml(stderr)

return (p.returncode, stdout, stderr)

def _prefix_login_path(self, remote_path):
Expand All @@ -288,6 +300,16 @@ def _prefix_login_path(self, remote_path):
remote_path = os.path.join(os.path.sep, remote_path)
return os.path.normpath(remote_path)

def _escape_win_path(self, path):
""" converts a Windows path to one that's supported by SFTP and SCP """
# If using a root path then we need to start with /
prefix = ""
if re.match(r'^\w{1}:', path):
prefix = "/"

# Convert all '\' to '/'
return "%s%s" % (prefix, path.replace("\\", "/"))

def put_file(self, in_path, out_path):
""" Transfer a file from local to docker container """
super(Connection, self).put_file(in_path, out_path)
Expand All @@ -308,11 +330,32 @@ def put_file(self, in_path, out_path):
count = ' count=0'
else:
count = ''
args = self._build_exec_cmd([self._play_context.executable, "-c", "dd of=%s bs=%s%s" % (out_path, BUFSIZE, count)])
if getattr(self._shell, "_IS_WINDOWS", False):
cmd = '''& { $strm = [Console]::OpenStandardInput(1024)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use _encode_script as per below you can skip the & { ... } entirely and just run the command as is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jborean93 changed the code as advised, but I receive an invalid payload

image

image

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is happening here, the error you've shared is something that is thrown by the bootstrap_wrapper.ps1 which is used when executing a module. The code you have here is for copying a file to the docker container and shouldn't be calling the bootstrap_wrapper code.

$data = [System.Array]::CreateInstance([byte],1024)
$fstrm = [IO.File]::OpenWrite(%s)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to have more logic to handle here. Right now at a quote glance I could have a path like:

’+ (&[ScriptBlock]::Create([[Text.Encoding]::UTF8.GetString([Convert]::FromBase64String("my code to inject")) +’

Note the use of unicode smart quotes

PowerShell (un)helpfully treats a bunch of smart quotes as the same as single quotes so even if you ran shlex_quote on your path it won't escape what is needed. As soon as you can escape outside of a quote you can embed whatever command you want to run here.

There are 2 choices I can see you moving to:

  • Use a the same (or similar regex) to the ansible.windows quote filter for powershell to escape the smart quotes and give you a literal value
  • Base64 encode the path and decode that in your script - base64 means you no longer have to deal with quotes or unicode characters
cmd = '''
...
$outPath = [Text.Encoding]::UTF8.GetString([Convert]::FromBase64String('%s'))
...
''' % (to_text(base64.b64encode(to_bytes(out_path, encoding='utf-8'))))

Both options remove the need for `shlex_quote` but I still recommend you normalize the path as you have done today.

do {
$read = $strm.Read($data, 0, $data.Length)
if ($read -gt 0) {
$fstrm.Write($data, 0, $read)
}
} while ($read -gt 0)
$fstrm.Flush()
$fstrm.Dispose()
$strm.Dispose() }''' % (out_path)
args = self._build_exec_cmd(["cmd", "/c", self._shell._encode_script(cmd, as_list=False, strict_mode=False, preserve_rc=False)])
tmeckel marked this conversation as resolved.
Show resolved Hide resolved
display.debug(u"Command to execute: {0}".format(args))
else:
args = self._build_exec_cmd([self._play_context.executable, "-c", "dd of=%s bs=%s%s" % (out_path, BUFSIZE, count)])

args = [to_bytes(i, errors='surrogate_or_strict') for i in args]
try:
p = subprocess.Popen(args, stdin=in_file,
stdout=subprocess.PIPE, stderr=subprocess.PIPE)
p = subprocess.Popen(
args,
stdin=in_file,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE
)
except OSError:
raise AnsibleError("docker connection requires dd command in the container to put files")
stdout, stderr = p.communicate()
Expand Down
Loading