Skip to content

Commit 734a74b

Browse files
author
Eli Davidson
committed
refactor(serve): inline additional-source EULA fold in _build_for_jumpstart
Scope the EULA handling down to mirror the primary model exactly: strip the JumpStart-internal HostingEulaKey (not part of the CreateModel API shape) and fold accept_eula into each source's ModelAccessConfig when set. No client-side gatedness check or raise: the control plane determines gatedness from the artifact's bucket and rejects CreateModel with its own EULA validation error, for the primary model and additional sources alike. This reverts the sagemaker-core half of the previous commit: session_helper.py returns to upstream and the whole change lives in the JumpStart builder that produces the spec-derived sources. Tests assert the new contract on the CreateModel request: acceptance folded when accept_eula is set (True or False), no ModelAccessConfig when unset.
1 parent 06e06d7 commit 734a74b

3 files changed

Lines changed: 88 additions & 91 deletions

File tree

‎sagemaker-core/src/sagemaker/core/helper/session_helper.py‎

Lines changed: 1 addition & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3072,47 +3072,6 @@ def _optimization_job_status(sagemaker_client, job_name):
30723072
return desc
30733073

30743074

3075-
def _apply_accept_eula_to_additional_model_data_sources(additional_model_data_sources, accept_eula):
3076-
"""Apply EULA acceptance to additional model data sources.
3077-
3078-
This is the additional-source counterpart of the primary model handling in
3079-
``container_def`` below, driven by the same ``accept_eula`` flag. Sources
3080-
resolved from JumpStart specs may carry a ``HostingEulaKey``: a truthy value
3081-
marks a gated source that requires explicit license acceptance. Acceptance
3082-
is folded into the source's ``S3DataSource.ModelAccessConfig`` and a gated
3083-
source without acceptance raises before any request is made. The
3084-
``HostingEulaKey`` itself is JumpStart-internal metadata the CreateModel
3085-
API rejects, so it is always removed.
3086-
3087-
Args:
3088-
additional_model_data_sources (list[dict]): AdditionalModelDataSources
3089-
entries, possibly carrying JumpStart ``HostingEulaKey`` metadata.
3090-
accept_eula (bool): Whether model terms of use have been accepted.
3091-
3092-
Returns:
3093-
list[dict]: The sources in CreateModel API shape.
3094-
3095-
Raises:
3096-
ValueError: If a gated source is present and ``accept_eula`` is not True.
3097-
"""
3098-
applied_sources = []
3099-
for source in additional_model_data_sources:
3100-
applied_source = dict(source)
3101-
if applied_source.pop("HostingEulaKey", None):
3102-
if not accept_eula:
3103-
raise ValueError(
3104-
"accept_eula must be set to True to deploy this model. "
3105-
"Please set accept_eula=True on the ModelBuilder instance "
3106-
"to confirm you have read and accepted the end-user license "
3107-
"agreement for this model."
3108-
)
3109-
s3_data_source = dict(applied_source.get("S3DataSource", {}))
3110-
s3_data_source["ModelAccessConfig"] = {"AcceptEula": True}
3111-
applied_source["S3DataSource"] = s3_data_source
3112-
applied_sources.append(applied_source)
3113-
return applied_sources
3114-
3115-
31163075
def container_def(
31173076
image_uri,
31183077
model_data_url=None,
@@ -3156,13 +3115,7 @@ def container_def(
31563115
c_def = {"Image": image_uri, "Environment": env}
31573116

31583117
if additional_model_data_sources:
3159-
c_def["AdditionalModelDataSources"] = (
3160-
_apply_accept_eula_to_additional_model_data_sources(
3161-
additional_model_data_sources, accept_eula
3162-
)
3163-
if isinstance(additional_model_data_sources, list)
3164-
else additional_model_data_sources
3165-
)
3118+
c_def["AdditionalModelDataSources"] = additional_model_data_sources
31663119

31673120
if isinstance(model_data_url, str) and (
31683121
not (model_data_url.startswith("s3://") and model_data_url.endswith("tar.gz"))

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

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,18 +1017,35 @@ def _build_for_jumpstart(self) -> Model:
10171017
# Propagate additional model data sources resolved from the JumpStart
10181018
# spec (e.g. speculative decoding draft models like EAGLE). The JumpStart
10191019
# factory (_add_additional_model_data_sources_to_kwargs) already returns
1020-
# these in CreateModel API shape via camel_case_to_pascal_case(...);
1021-
# EULA handling (enforcing accept_eula on gated sources and stripping
1022-
# the JumpStart-internal HostingEulaKey) happens in ``container_def``,
1023-
# the same place the primary model's accept_eula is applied. Without
1024-
# this propagation, sources declared in the spec are dropped from the
1025-
# CreateModel call and the container fails to find the referenced
1026-
# artifacts at runtime.
1020+
# these in CreateModel API shape via camel_case_to_pascal_case(...),
1021+
# except for two adjustments applied here, mirroring how ``container_def``
1022+
# treats the primary model's ``ModelDataSource``:
1023+
# - the JumpStart-internal ``HostingEulaKey`` is removed (it is not part
1024+
# of the CreateModel API shape, request validation rejects it), and
1025+
# - when ``accept_eula`` is set, it is folded into each source's
1026+
# ``S3DataSource.ModelAccessConfig``.
1027+
# Gated-source EULA enforcement is server-side, same as for the primary
1028+
# model: the control plane determines gatedness from the artifact's
1029+
# bucket and rejects CreateModel with an EULA validation error when a
1030+
# gated source lacks ``ModelAccessConfig`` with ``AcceptEula`` true.
1031+
# Without this propagation, sources declared in the spec are dropped
1032+
# from the CreateModel call and the container fails to find the
1033+
# referenced artifacts at runtime.
10271034
additional_model_data_sources = getattr(
10281035
init_kwargs, "additional_model_data_sources", None
10291036
)
10301037
if isinstance(additional_model_data_sources, list) and additional_model_data_sources:
1031-
self.additional_model_data_sources = additional_model_data_sources
1038+
accept_eula = getattr(self, "accept_eula", None)
1039+
prepared_sources = []
1040+
for source in additional_model_data_sources:
1041+
prepared_source = dict(source)
1042+
prepared_source.pop("HostingEulaKey", None)
1043+
if accept_eula is not None:
1044+
s3_data_source = dict(prepared_source.get("S3DataSource", {}))
1045+
s3_data_source["ModelAccessConfig"] = {"AcceptEula": accept_eula}
1046+
prepared_source["S3DataSource"] = s3_data_source
1047+
prepared_sources.append(prepared_source)
1048+
self.additional_model_data_sources = prepared_sources
10321049

10331050
# Handle model artifacts for fine-tuned models
10341051
if hasattr(init_kwargs, "model_data") and init_kwargs.model_data:

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

Lines changed: 62 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,8 +1050,9 @@ class TestJumpStartAdditionalModelDataSourcesUserFlow(unittest.TestCase):
10501050
Everything in between runs for real: JumpStart model-id detection,
10511051
JumpStartModelSpecs parsing, inference-config resolution, the
10521052
get_init_kwargs factory (image URI resolution, content-bucket injection,
1053-
PascalCase shaping), _build_for_jumpstart's EULA translation, and
1054-
_prepare_container_def_base assembling the CreateModel request.
1053+
PascalCase shaping), the propagation in _build_for_jumpstart, the
1054+
accept_eula application in container_def (shared with the primary model),
1055+
and _prepare_container_def_base assembling the CreateModel request.
10551056
"""
10561057

10571058
SPEC_DIR = Path(__file__).parent / "data" / "jumpstart_specs"
@@ -1078,7 +1079,7 @@ def _spec_additional_source(cls, model_id, config_name="lmi-optimized"):
10781079
return sources[0]
10791080

10801081
@classmethod
1081-
def _expected_create_model_source(cls, model_id, bucket, accept_eula=False):
1082+
def _expected_create_model_source(cls, model_id, bucket, accept_eula=None):
10821083
"""The shape the CreateModel request must carry for the fixture's
10831084
source. Data values (channel name, compression, data type, key prefix)
10841085
come from the spec fixture so it stays the single source of truth; the
@@ -1088,8 +1089,8 @@ def _expected_create_model_source(cls, model_id, bucket, accept_eula=False):
10881089
- snake_case spec keys become the PascalCase API keys
10891090
- the bucket-less key prefix resolves into the given content bucket
10901091
- HostingEulaKey never appears (the API rejects it)
1091-
- EULA acceptance, when given, is folded into
1092-
S3DataSource.ModelAccessConfig
1092+
- accept_eula, when set, is folded into S3DataSource.ModelAccessConfig
1093+
exactly like the primary model's ModelDataSource
10931094
"""
10941095
raw = cls._spec_additional_source(model_id)
10951096
expected = {
@@ -1100,8 +1101,8 @@ def _expected_create_model_source(cls, model_id, bucket, accept_eula=False):
11001101
"S3Uri": f"s3://{bucket}/{raw['s3_data_source']['s3_uri']}",
11011102
},
11021103
}
1103-
if accept_eula:
1104-
expected["S3DataSource"]["ModelAccessConfig"] = {"AcceptEula": True}
1104+
if accept_eula is not None:
1105+
expected["S3DataSource"]["ModelAccessConfig"] = {"AcceptEula": accept_eula}
11051106
return expected
11061107

11071108
def setUp(self):
@@ -1196,9 +1197,12 @@ def test_ungated_additional_source_reaches_create_model_without_eula(self):
11961197
],
11971198
)
11981199

1199-
def test_ungated_additional_source_gets_no_model_access_config(self):
1200-
"""accept_eula=True must not add a ModelAccessConfig to an ungated
1201-
additional source (it applies to the primary model only)."""
1200+
def test_accept_eula_applies_to_every_model_data_source_uniformly(self):
1201+
"""The single accept_eula knob folds the same ModelAccessConfig into
1202+
the primary model AND each additional source, exactly like the primary
1203+
model's ModelDataSource handling. Gatedness is not the SDK's call: the
1204+
service determines it from the artifact's bucket and ignores the config
1205+
on ungated sources."""
12021206
self._build(
12031207
"openai-reasoning-gpt-oss-20b", accept_eula=True, instance_type="ml.g7e.2xlarge"
12041208
)
@@ -1208,11 +1212,12 @@ def test_ungated_additional_source_gets_no_model_access_config(self):
12081212
container_def["AdditionalModelDataSources"],
12091213
[
12101214
self._expected_create_model_source(
1211-
"openai-reasoning-gpt-oss-20b", bucket=self.PUBLIC_CONTENT_BUCKET
1215+
"openai-reasoning-gpt-oss-20b",
1216+
bucket=self.PUBLIC_CONTENT_BUCKET,
1217+
accept_eula=True,
12121218
)
12131219
],
12141220
)
1215-
# The single accept_eula knob still covers the primary (base) model.
12161221
self.assertEqual(
12171222
container_def["ModelDataSource"]["S3DataSource"]["ModelAccessConfig"],
12181223
{"AcceptEula": True},
@@ -1244,31 +1249,53 @@ def test_gated_additional_source_with_accepted_eula_sends_model_access_config(se
12441249
{"AcceptEula": True},
12451250
)
12461251

1247-
def test_gated_additional_source_without_eula_raises_and_never_calls_create_model(self):
1248-
"""No EULA acceptance: build() fails with the actionable error and no
1249-
CreateModel call is ever made."""
1250-
with self.assertRaises(ValueError) as ctx:
1251-
self._build(
1252-
"meta-textgeneration-llama-3-1-70b",
1253-
inference_config_name="lmi-optimized",
1254-
instance_type="ml.p4d.24xlarge",
1255-
)
1252+
def test_gated_additional_source_without_eula_sends_no_model_access_config(self):
1253+
"""No EULA decision: the request carries the gated source without any
1254+
ModelAccessConfig. Enforcement is the service's: the SageMaker control
1255+
plane resolves the private-bucket URI as gated and rejects CreateModel
1256+
with an EULA validation error, the same as for a gated primary model."""
1257+
self._build(
1258+
"meta-textgeneration-llama-3-1-70b",
1259+
inference_config_name="lmi-optimized",
1260+
instance_type="ml.p4d.24xlarge",
1261+
)
12561262

1257-
self.assertIn("accept_eula must be set to True", str(ctx.exception))
1258-
self.session.create_model.assert_not_called()
1263+
container_def = self._create_model_container_def()
1264+
self.assertEqual(
1265+
container_def["AdditionalModelDataSources"],
1266+
[
1267+
self._expected_create_model_source(
1268+
"meta-textgeneration-llama-3-1-70b", bucket=self.PRIVATE_CONTENT_BUCKET
1269+
)
1270+
],
1271+
)
12591272

1260-
def test_gated_additional_source_with_rejected_eula_raises_and_never_calls_create_model(self):
1261-
"""Explicit accept_eula=False fails the same way."""
1262-
with self.assertRaises(ValueError) as ctx:
1263-
self._build(
1264-
"meta-textgeneration-llama-3-1-70b",
1265-
accept_eula=False,
1266-
inference_config_name="lmi-optimized",
1267-
instance_type="ml.p4d.24xlarge",
1268-
)
1269-
1270-
self.assertIn("accept_eula must be set to True", str(ctx.exception))
1271-
self.session.create_model.assert_not_called()
1273+
def test_gated_additional_source_with_rejected_eula_sends_acceptance_false(self):
1274+
"""Explicit accept_eula=False is transmitted faithfully as
1275+
ModelAccessConfig={"AcceptEula": False} on every source, exactly like
1276+
the primary model. The service rejects the gated sources."""
1277+
self._build(
1278+
"meta-textgeneration-llama-3-1-70b",
1279+
accept_eula=False,
1280+
inference_config_name="lmi-optimized",
1281+
instance_type="ml.p4d.24xlarge",
1282+
)
1283+
1284+
container_def = self._create_model_container_def()
1285+
self.assertEqual(
1286+
container_def["AdditionalModelDataSources"],
1287+
[
1288+
self._expected_create_model_source(
1289+
"meta-textgeneration-llama-3-1-70b",
1290+
bucket=self.PRIVATE_CONTENT_BUCKET,
1291+
accept_eula=False,
1292+
)
1293+
],
1294+
)
1295+
self.assertEqual(
1296+
container_def["ModelDataSource"]["S3DataSource"]["ModelAccessConfig"],
1297+
{"AcceptEula": False},
1298+
)
12721299

12731300
def test_unselected_config_sources_do_not_leak_into_create_model(self):
12741301
"""Config resolution gates which sources apply: the same gated model on

0 commit comments

Comments
 (0)