fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names - #6035

Closed
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e
Closed

fix(serve): private hub deploys attach HubAccessConfig and support aliased content names#6035
e-davidson wants to merge 7 commits into
aws:masterfrom
e-davidson:fix/private-hub-e2e

Conversation

@e-davidson

@e-davidsone-davidson commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Scope update (2026-07-16)

#6036 (merged after this PR opened) independently fixed the model_reference_arn propagation defect with the same change this PR originally carried. This branch has been merged with master: the duplicate propagation block was dropped in favor of the upstream one. This PR now provides the remaining fix plus the test coverage that would have caught the original regression.

Issue

Deploying a JumpStart model from a private hub via ModelBuilder.from_jumpstart_config(hub_name=...) was broken in two ways on v3.16.0:

Defect A (fixed upstream in #6036): HubAccessConfig never attached to CreateModel.
get_init_kwargs resolved model_reference_arn, but the builder never copied it, so _prepare_container_def_base omitted S3DataSource.HubAccessConfig.HubContentArn and the execution role needed s3:GetObject on the public jumpstart-cache-prod-* bucket.

Defect B (fixed here): aliased hub content names unsupported.
CreateHubContentReference accepts a custom HubContentName, but the SDK always looks up hub content by model_id. A reference named differently from the public model id fails with ResourceNotFound.

Changes in this PR

  1. Fix: optional hub_content_name on JumpStartConfig / from_jumpstart_config; when set, hub content is resolved by its actual name.
  2. Unit tests (tests/unit/test_private_hub_artifact_resolution.py, extends the file from fix: ModelBuilder resolves private hub artifacts correctly #5985):
    • model_reference_arn propagation regression test (guards the Fix private hub #6036 fix; Fix private hub #6036's own tests assert a subset)
    • CreateModel payload chain test: drives the real unmocked path _build_for_jumpstart -> _prepare_container_def_base and asserts the exact container definition dict carries HubAccessConfig (present for hub builds, absent for public). This is the assertion that would have caught the regression that shipped in v3.15.1-v3.16.0.
    • hub_content_name lookup + config threading tests
    • Shared setup via module-level _build_jumpstart_builder() helper, matching package conventions
  3. Integration tests (tests/integ/test_private_hub_artifact_resolution.py, slow_test):
    • test_deploy_with_no_s3_execution_role: self-provisions an execution role with zero S3 permissions, deploys from a private hub, asserts the created Model carries HubAccessConfig. Encodes the service-side brokered-access contract that unit tests cannot reach.
    • test_deploy_with_aliased_hub_content_name: deploys a reference named differently from the model_id via hub_content_name.
    • Both reuse the existing self-provisioning hub fixture; full teardown (hub, references, role, endpoints).

Verification (all red -> green)

LayerBaseline v3.16.0With fix
Unit (17 tests)4 discriminating tests FAIL17/17 pass
Integ, live us-east-12 FAIL (missing HubAccessConfig assertion; pydantic rejection of hub_content_name)2 pass (38s), both with the zero-S3 role
Post-merge with #6036n/a17/17 unit tests pass, single propagation block

Live runs used a real private hub + ModelReference and an IAM role with no S3 permissions; endpoints and resources were cleaned up.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…iased content names
Two remaining defects after PR aws#5985:
1. HubAccessConfig missing from CreateModel. _build_for_jumpstart
resolved model_reference_arn via get_init_kwargs but never copied it
onto the builder, so _prepare_container_def_base passed None to
container_def() and the S3DataSource had no HubAccessConfig. The
execution role was forced to need s3:GetObject on the public
jumpstart-cache-prod bucket. Fix: propagate
init_kwargs.model_reference_arn to self.model_reference_arn.
2. Hub content references named differently from the public model_id
could not be resolved (ResourceNotFound). Fix: add hub_content_name
to JumpStartConfig and use it as the hub lookup identifier in the
build path when set.
Verified E2E against a live private hub with an execution role that has
zero S3 permissions: both matching-name and aliased-name deploys now
succeed, with SageMaker brokering artifact access through the hub
content reference.
Pattern follows master-v2, where Model.prepare_container_def passes
model_reference_arn into sagemaker.container_def().
---
X-AI-Prompt: create a new workspace, pull the pysdk, reproduce in our workspace and fix both bugs, the name and that it's not configured correctly. Look at master-v2 to fix this.
X-AI-Tool: meshclaw
…ased hub content names
Add mock-based unit tests covering the two private hub defects fixed in
the previous commit. Verified each discriminating test fails on the
v3.16.0 baseline and passes with the fix:
- test_model_reference_arn_propagated_from_init_kwargs: the ARN resolved
by get_init_kwargs must land on the builder so container_def attaches
S3DataSource.HubAccessConfig.HubContentArn (fails on baseline with
model_reference_arn=None)
- test_hub_content_name_used_as_lookup_model_id: aliased references
resolve by hub_content_name (fails on baseline)
- test_from_jumpstart_config_threads_hub_content_name: config threading
(fails on baseline, JumpStartConfig rejects the field)
Plus negative tests (public catalog path unchanged, no HubAccessConfig
without a model_reference_arn) and pure-function coverage of
container_def HubAccessConfig injection. No AWS calls.
---
X-AI-Prompt: Write the mock unit tests, verify they fail without fix + pass with fix, push to PR
X-AI-Tool: meshclaw
…sConfig
Close the gap between the propagation test and the pure container_def
test: drive the real unmocked path from _build_for_jumpstart through
_prepare_container_def_base and assert the exact container definition
dict sent to the CreateModel API contains
S3DataSource.HubAccessConfig.HubContentArn for private hub builds, and
omits it for public catalog builds.
This is the payload-shape assertion that encodes the customer-visible
contract: with HubAccessConfig present, SageMaker brokers model data
access through the hub content reference and the execution role needs
no s3:GetObject on the public JumpStart cache bucket.
Verified: fails on the v3.16.0 baseline (HubAccessConfig absent),
passes with the fix. No AWS calls.
---
X-AI-Prompt: Did we make sure there are no calls to s3 when using a model reference? what about that we're passing in the intended call to sagemaker create model?
X-AI-Tool: meshclaw
Replace cross-TestCase borrowing (instantiating
TestModelReferenceArnPropagation inside other test classes to reuse its
_build method) with a module-level _build_jumpstart_builder() helper,
matching the package convention for shared builder setup (see
_create_mock_builder in test_model_builder_servers_hf_model_id.py and
_builder in test_bedrock_model_builder.py).
No behavior change. Re-verified both directions: 4 discriminating tests
fail on the v3.16.0 baseline, all 17 pass with the fix.
---
X-AI-Prompt: Refactor to module-level helper, re-verify both directions, push
X-AI-Tool: meshclaw
…ntent name
Add two slow_test integration tests encoding the service-side contract
the unit tests cannot cover:
- test_deploy_with_no_s3_execution_role: creates an execution role with
NO S3 permissions (ECR + CloudWatch Logs + hub read only), deploys
from the private hub, and asserts the created Model resource carries
S3DataSource.HubAccessConfig. On v3.16.0 this fails at CreateModel
with 'Could not access model data at s3://jumpstart-cache-prod-...'.
- test_deploy_with_aliased_hub_content_name: deploys a ModelReference
whose HubContentName differs from the public model_id via the new
JumpStartConfig.hub_content_name. On v3.16.0 this fails with
ResourceNotFound.
Both reuse the existing self-provisioning private_hub fixture (create
hub + reference, tear down after) and clean up endpoints, endpoint
configs, and the IAM role. Mirrors the live E2E verification performed
against this branch (both scenarios deployed successfully with the
zero-S3 role in us-east-1).
---
X-AI-Prompt: Now where's the e2e test?
X-AI-Tool: meshclaw
Live verification of both integ tests in both directions:
- With fix: 2 passed (38s)
- Baseline v3.16.0: 2 failed as intended. no_s3 test failed at the
HubAccessConfig assertion (CreateModel accepted the gated gemma-2b
request without HubAccessConfig; the endpoint then failed
asynchronously). Aliased test failed at JumpStartConfig validation
(hub_content_name field does not exist on baseline).
Corrections from the live run: the no_s3 docstring claimed a synchronous
CreateModel ValidationException, but the deterministic failure signal is
the HubAccessConfig assertion. Cleanup except block now logs failures
instead of silently swallowing them (three Failed-status endpoints
leaked during verification runs and were deleted manually).
---
X-AI-Prompt: Run both directions live now
X-AI-Tool: meshclaw
Upstream aws#6036 independently fixed the model_reference_arn propagation
(Defect A) with the same 6-line fix. Resolution: kept upstream's
propagation block as canonical (removed our duplicate at the earlier
location), kept our hub_content_name lookup fix (Defect B, not covered
upstream), and kept our unit test file as the superset (upstream's 2
tests assert a subset of our TestModelReferenceArnPropagation).
Verified post-merge: 17/17 unit tests pass, single propagation block,
no conflict markers.
@e-davidson

Copy link
Copy Markdown
ContributorAuthor

Superseded by #6039. This PR predated #6036, which independently shipped the same model_reference_arn propagation fix, leaving this branch with a redundant duplicate commit and a merge commit. #6039 is a clean cut from current master carrying only the remaining fix (aliased hub_content_name support) and the full regression test coverage (unit + CreateModel payload chain + live-verified E2E integ tests).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@e-davidson