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

[Active Directory] Bind mounts in the joining context manager #1627

Merged
merged 2 commits into from
Mar 31, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions subiquity/server/ad_joiner.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import asyncio
from contextlib import contextmanager
import logging
import os
from socket import gethostname
from subprocess import CalledProcessError
from subiquitycore.utils import arun_command, run_command
Expand All @@ -29,18 +30,28 @@


@contextmanager
def hostname_context(hostname: str):
""" Temporarily adjusts the host name to [hostname] and restores it
back in the end of the caller scope. """
def joining_context(hostname: str, root_dir: str):
""" Temporarily adjusts the host name to [hostname] and bind-mounts
interesting system directories in preparation for running realm
in target's [root_dir], undoing it all on exit. """
hostname_current = gethostname()
hostname_process = run_command(['hostname', hostname])
binds = ("/proc", "/sys", "/dev", "/run")
try:
hostname_process = run_command(['hostname', hostname])
for bind in binds:
bound_dir = os.path.join(root_dir, bind[1:])
if bound_dir != bind:
run_command(["mount", "--bind", bind, bound_dir])
yield hostname_process
finally:
# Restoring the live session hostname.
hostname_process = run_command(['hostname', hostname_current])
if hostname_process.returncode:
log.info("Failed to restore live session hostname")
for bind in binds:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for bind in binds:
for bind in reversed(binds):

I think for the mounts your doing the order won't matter, but it does sometimes. Let's reverse the order in case we add more to the list later.

bound_dir = os.path.join(root_dir, bind[1:])
if bound_dir != bind:
run_command(["umount", "-f", bound_dir])


class AdJoinStrategy():
Expand All @@ -54,14 +65,14 @@ async def do_join(self, info: AdConnectionInfo, hostname: str, context) \
-> AdJoinResult:
""" This method changes the hostname and perform a real AD join, thus
should only run in a live session. """
root_dir = self.app.base_model.target
# Set hostname for AD to determine FQDN (no FQDN option in realm join,
# only adcli, which only understands the live system, but not chroot)
with hostname_context(hostname) as host_process:
with joining_context(hostname, root_dir) as host_process:
if host_process.returncode:
log.info("Failed to set live session hostname for adcli")
return AdJoinResult.JOIN_ERROR

root_dir = self.app.base_model.target
cp = await arun_command([self.realm, "join", "--install", root_dir,
"--user", info.admin_name,
"--computer-name", hostname,
Expand Down