Skip to content

Commit 1b86252

Browse files
Fix/jumpstart network isolation (#5833)
* fix: apply JumpStart model spec enable_network_isolation in ModelBuilder In v2, JumpStart models deployed with EnableNetworkIsolation=True because the JumpStartModel class read inference_enable_network_isolation from the model spec JSON. In v3's ModelBuilder, this field was never extracted from the JumpStart init_kwargs or deploy_kwargs, causing all JumpStart models to deploy without network isolation (defaulting to False). This fix applies enable_network_isolation from the JumpStart model spec in both code paths: 1. _build_for_jumpstart() - regular build path via get_init_kwargs() 2. from_jumpstart_config() - classmethod path via _retrieve_model_deploy_kwargs() The user-provided value (via network= param) takes precedence. * test: add unit tests for JumpStart network isolation fix Tests both code paths: 1. _build_for_jumpstart() - verifies enable_network_isolation is applied from init_kwargs and that user-set values are not overridden 2. from_jumpstart_config() - verifies enable_network_isolation is extracted from deploy_kwargs * test: add unit tests for JumpStart network isolation fix Tests both code paths: 1. _build_for_jumpstart() - verifies enable_network_isolation is applied from init_kwargs and that user-set values are not overridden 2. from_jumpstart_config() - verifies enable_network_isolation is extracted from deploy_kwargs * fix: add _enable_network_isolation to MockModelBuilderServers test fixture The mock directly inherits _ModelBuilderServers without going through ModelBuilder.__post_init__ which initializes this attribute. * test: add integ test for JumpStart network isolation + fix mock fixture Integration test builds a real SageMaker Model (falcon-7b) and verifies EnableNetworkIsolation=True via describe_model API. No endpoint deployed. Also adds _enable_network_isolation to MockModelBuilderServers fixture to fix 7 pre-existing unit test failures. --------- Co-authored-by: Liam Neal Reilly <lnealrei@amazon.com>
1 parent 59ea1ab commit 1b86252

6 files changed

Lines changed: 174 additions & 0 deletions

File tree

‎sagemaker-serve/src/sagemaker/serve/model_builder.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3621,6 +3621,12 @@ def from_jumpstart_config(
36213621
)
36223622
mb_instance.inference_ami_version = deploy_kwargs.get("inference_ami_version")
36233623

3624+
# Apply network isolation from JumpStart model spec if not set by user via network param
3625+
if not mb_instance._enable_network_isolation and deploy_kwargs.get(
3626+
"enable_network_isolation"
3627+
):
3628+
mb_instance._enable_network_isolation = deploy_kwargs["enable_network_isolation"]
3629+
36243630
return mb_instance
36253631

36263632
@_telemetry_emitter(feature=Feature.MODEL_CUSTOMIZATION, func_name="model_builder.transformer")

‎sagemaker-serve/src/sagemaker/serve/model_builder_servers.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,13 @@ def _build_for_jumpstart(self) -> Model:
877877
if hasattr(init_kwargs, "env") and init_kwargs.env:
878878
self.env_vars.update(init_kwargs.env)
879879

880+
# Apply network isolation from JumpStart model spec if not already set by user
881+
if (
882+
not self._enable_network_isolation
883+
and getattr(init_kwargs, "enable_network_isolation", None) is not None
884+
):
885+
self._enable_network_isolation = init_kwargs.enable_network_isolation
886+
880887
# Handle model artifacts for fine-tuned models
881888
if hasattr(init_kwargs, "model_data") and init_kwargs.model_data:
882889
if (
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
#
2+
# Licensed under the Apache License, Version 2.0 (the "License"). You
3+
# may not use this file except in compliance with the License. A copy of
4+
# the License is located at
5+
#
6+
# http://aws.amazon.com/apache2.0/
7+
#
8+
# or in the "license" file accompanying this file. This file is
9+
# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
10+
# ANY KIND, either express or implied. See the License for the specific
11+
# language governing permissions and limitations under the License.
12+
from __future__ import absolute_import
13+
14+
import uuid
15+
import pytest
16+
import logging
17+
18+
from sagemaker.serve.model_builder import ModelBuilder
19+
from sagemaker.core.jumpstart.configs import JumpStartConfig
20+
from sagemaker.train.configs import Compute
21+
22+
logger = logging.getLogger(__name__)
23+
24+
MODEL_ID = "huggingface-llm-falcon-7b-bf16"
25+
INSTANCE_TYPE = "ml.g5.2xlarge"
26+
MODEL_NAME_PREFIX = "js-netiso-test"
27+
28+
29+
@pytest.mark.slow_test
30+
def test_jumpstart_build_enables_network_isolation():
31+
"""Integration test verifying JumpStart models are built with EnableNetworkIsolation.
32+
33+
JumpStart model specs define inference_enable_network_isolation=True for most models.
34+
This test validates that ModelBuilder.build() propagates this setting to the
35+
SageMaker Model resource, matching v2 behavior.
36+
"""
37+
logger.info("Starting JumpStart network isolation integration test...")
38+
39+
compute = Compute(instance_type=INSTANCE_TYPE)
40+
jumpstart_config = JumpStartConfig(model_id=MODEL_ID)
41+
model_builder = ModelBuilder.from_jumpstart_config(
42+
jumpstart_config=jumpstart_config, compute=compute
43+
)
44+
unique_id = str(uuid.uuid4())[:8]
45+
46+
core_model = model_builder.build(model_name=f"{MODEL_NAME_PREFIX}-{unique_id}")
47+
logger.info(f"Model created: {core_model.model_name}")
48+
49+
try:
50+
# Verify ModelBuilder picked up network isolation from spec
51+
assert model_builder._enable_network_isolation, (
52+
f"ModelBuilder._enable_network_isolation should be True for {MODEL_ID}, "
53+
f"got {model_builder._enable_network_isolation}"
54+
)
55+
56+
# Verify the actual SageMaker Model resource has EnableNetworkIsolation=True
57+
sm_client = model_builder.sagemaker_session.sagemaker_client
58+
desc = sm_client.describe_model(ModelName=core_model.model_name)
59+
assert desc.get("EnableNetworkIsolation") is True, (
60+
f"SageMaker Model should have EnableNetworkIsolation=True, "
61+
f"got {desc.get('EnableNetworkIsolation')}"
62+
)
63+
64+
logger.info("✅ Network isolation correctly applied to SageMaker Model")
65+
finally:
66+
core_model.delete()
67+
logger.info("Model deleted.")

‎sagemaker-serve/tests/unit/servers/test_model_builder_servers.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ def __init__(self):
4747
self.framework_version = None
4848
self._is_mlflow_model = False
4949
self.config_name = None
50+
self._enable_network_isolation = False
5051

5152
def _deploy_local_endpoint(self, **kwargs):
5253
return Mock()

‎sagemaker-serve/tests/unit/test_model_builder_coverage_boost.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,34 @@ def test_from_jumpstart_config_basic(self):
381381
self.assertEqual(mb.model, "test-model")
382382
self.assertEqual(mb.model_version, "1.0.0")
383383

384+
@patch("sagemaker.serve.model_builder._retrieve_model_deploy_kwargs")
385+
def test_from_jumpstart_config_applies_network_isolation(self, mock_deploy_kwargs):
386+
"""Test that enable_network_isolation from deploy kwargs is applied."""
387+
from sagemaker.core.jumpstart.configs import JumpStartConfig
388+
from sagemaker.core.training.configs import Compute
389+
390+
mock_deploy_kwargs.return_value = {
391+
"model_data_download_timeout": 600,
392+
"enable_network_isolation": True,
393+
}
394+
395+
js_config = JumpStartConfig(
396+
model_id="test-model",
397+
model_version="1.0.0"
398+
)
399+
400+
mock_session = Mock()
401+
mock_session.boto_region_name = "us-west-2"
402+
403+
mb = ModelBuilder.from_jumpstart_config(
404+
jumpstart_config=js_config,
405+
role_arn="arn:aws:iam::123456789012:role/SageMakerRole",
406+
compute=Compute(instance_type="ml.g5.xlarge"),
407+
sagemaker_session=mock_session,
408+
)
409+
410+
self.assertTrue(mb._enable_network_isolation)
411+
384412

385413
if __name__ == "__main__":
386414
unittest.main()

‎sagemaker-serve/tests/unit/test_model_builder_servers_coverage.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,71 @@ def test_build_for_jumpstart_routes_to_mms(self, mock_prepare, mock_create, mock
501501
mock_create.assert_called_once()
502502

503503

504+
@patch("sagemaker.core.jumpstart.factory.utils.get_init_kwargs")
505+
@patch("sagemaker.serve.model_builder.ModelBuilder._create_model")
506+
@patch("sagemaker.serve.model_builder.ModelBuilder._prepare_for_mode")
507+
def test_build_for_jumpstart_applies_network_isolation_from_spec(
508+
self, mock_prepare, mock_create, mock_get_kwargs
509+
):
510+
"""Test that enable_network_isolation from JumpStart model spec is applied."""
511+
mock_init_kwargs = Mock()
512+
mock_init_kwargs.image_uri = (
513+
"763104351884.dkr.ecr.us-west-2.amazonaws.com/djl-inference:0.21.0-deepspeed0.8.0-cu117"
514+
)
515+
mock_init_kwargs.env = {}
516+
mock_init_kwargs.model_data = "s3://jumpstart-cache/models/model.tar.gz"
517+
mock_init_kwargs.enable_network_isolation = True
518+
mock_get_kwargs.return_value = mock_init_kwargs
519+
520+
mock_model = Mock(spec=Model)
521+
mock_create.return_value = mock_model
522+
523+
builder = ModelBuilder(
524+
model="meta-textgeneration-llama-3-8b",
525+
role_arn=MOCK_ROLE_ARN,
526+
sagemaker_session=self.mock_session,
527+
mode=Mode.SAGEMAKER_ENDPOINT,
528+
)
529+
builder._optimizing = False
530+
531+
builder._build_for_jumpstart()
532+
533+
self.assertTrue(builder._enable_network_isolation)
534+
535+
@patch("sagemaker.core.jumpstart.factory.utils.get_init_kwargs")
536+
@patch("sagemaker.serve.model_builder.ModelBuilder._create_model")
537+
@patch("sagemaker.serve.model_builder.ModelBuilder._prepare_for_mode")
538+
def test_build_for_jumpstart_does_not_override_user_network_isolation(
539+
self, mock_prepare, mock_create, mock_get_kwargs
540+
):
541+
"""Test that user-set network isolation is not overridden by spec."""
542+
mock_init_kwargs = Mock()
543+
mock_init_kwargs.image_uri = (
544+
"763104351884.dkr.ecr.us-west-2.amazonaws.com/djl-inference:0.21.0-deepspeed0.8.0-cu117"
545+
)
546+
mock_init_kwargs.env = {}
547+
mock_init_kwargs.model_data = "s3://jumpstart-cache/models/model.tar.gz"
548+
mock_init_kwargs.enable_network_isolation = False
549+
mock_get_kwargs.return_value = mock_init_kwargs
550+
551+
mock_model = Mock(spec=Model)
552+
mock_create.return_value = mock_model
553+
554+
builder = ModelBuilder(
555+
model="meta-textgeneration-llama-3-8b",
556+
role_arn=MOCK_ROLE_ARN,
557+
sagemaker_session=self.mock_session,
558+
mode=Mode.SAGEMAKER_ENDPOINT,
559+
)
560+
builder._optimizing = False
561+
builder._enable_network_isolation = True # User explicitly set
562+
563+
builder._build_for_jumpstart()
564+
565+
# User's True should not be overridden by spec's False
566+
self.assertTrue(builder._enable_network_isolation)
567+
568+
504569
class TestDeployWrappers(unittest.TestCase):
505570
"""Test deploy wrapper methods."""
506571

0 commit comments

Comments
 (0)