Skip to content

fix: Add SELinux label to local docker volumes #3790

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

Merged
merged 7 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
14 changes: 13 additions & 1 deletion src/sagemaker/local/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@
TRAINING_JOB_NAME_ENV_NAME = "TRAINING_JOB_NAME"
S3_ENDPOINT_URL_ENV_NAME = "S3_ENDPOINT_URL"

# SELinux Enabled
SELINUX_ENABLED = os.environ.get("SAGEMAKER_LOCAL_SELINUX_ENABLED", "False").lower() in [
"1",
"true",
"yes",
]

logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -349,6 +356,7 @@ def retrieve_artifacts(self, compose_data, output_data_config, job_name):
# Gather the artifacts from all nodes into artifacts/model and artifacts/output
for host in self.hosts:
volumes = compose_data["services"][str(host)]["volumes"]
volumes = [v[:-2] if v.endswith(":z") else v for v in volumes]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the :z SELinux label while reading the volume back from compose_data

for volume in volumes:
if re.search(r"^[A-Za-z]:", volume):
unit, host_dir, container_dir = volume.split(":")
Expand Down Expand Up @@ -887,10 +895,14 @@ def __init__(self, host_dir, container_dir=None, channel=None):

self.container_dir = container_dir if container_dir else "/opt/ml/input/data/" + channel
self.host_dir = host_dir
map_format = "{}:{}"
if platform.system() == "Linux" and SELINUX_ENABLED:
# Support mounting shared volumes in SELinux enabled hosts
map_format += ":z"
if platform.system() == "Darwin" and host_dir.startswith("/var"):
self.host_dir = os.path.join("/private", host_dir)

self.map = "{}:{}".format(self.host_dir, self.container_dir)
self.map = map_format.format(self.host_dir, self.container_dir)


def _stream_output(process):
Expand Down
21 changes: 20 additions & 1 deletion tests/unit/sagemaker/local/test_local_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from mock import patch, Mock, MagicMock

import sagemaker
from sagemaker.local.image import _SageMakerContainer, _aws_credentials
from sagemaker.local.image import _SageMakerContainer, _Volume, _aws_credentials

REGION = "us-west-2"
BUCKET_NAME = "mybucket"
Expand Down Expand Up @@ -513,6 +513,7 @@ def test_train_local_code(get_data_source_instance, tmpdir, sagemaker_session):
assert config["services"][h]["image"] == image
assert config["services"][h]["command"] == "train"
volumes = config["services"][h]["volumes"]
volumes = [v[:-2] if v.endswith(":z") else v for v in volumes]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the :z SELinux label while reading the volume back from compose_data

assert "%s:/opt/ml/code" % "/tmp/code" in volumes
assert "%s:/opt/ml/shared" % shared_folder_path in volumes

Expand Down Expand Up @@ -564,9 +565,26 @@ def test_train_local_intermediate_output(get_data_source_instance, tmpdir, sagem
assert config["services"][h]["image"] == image
assert config["services"][h]["command"] == "train"
volumes = config["services"][h]["volumes"]
volumes = [v[:-2] if v.endswith(":z") else v for v in volumes]
assert "%s:/opt/ml/output/intermediate" % intermediate_folder_path in volumes


@patch("platform.system", Mock(return_value="Linux"))
@patch("sagemaker.local.image.SELINUX_ENABLED", Mock(return_value=True))
def test_container_selinux_has_label(tmpdir):
volume = _Volume(str(tmpdir), "/opt/ml/model")

assert volume.map.endswith(":z")


@patch("platform.system", Mock(return_value="Darwin"))
@patch("sagemaker.local.image.SELINUX_ENABLED", Mock(return_value=True))
def test_container_has_selinux_no_label(tmpdir):
volume = _Volume(str(tmpdir), "/opt/ml/model")

assert not volume.map.endswith(":z")


def test_container_has_gpu_support(tmpdir, sagemaker_session):
instance_count = 1
image = "my-image"
Expand Down Expand Up @@ -650,6 +668,7 @@ def test_serve_local_code(tmpdir, sagemaker_session):
assert config["services"][h]["command"] == "serve"

volumes = config["services"][h]["volumes"]
volumes = [v[:-2] if v.endswith(":z") else v for v in volumes]
assert "%s:/opt/ml/code" % "/tmp/code" in volumes
assert (
"SAGEMAKER_SUBMIT_DIRECTORY=/opt/ml/code"
Expand Down