Skip to content

Commit d0e94f7

Browse files
authored
Merge branch 'main' into feat/logging-config
2 parents e2c2d0f + 7f88fcb commit d0e94f7

2 files changed

Lines changed: 58 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414

1515
### **Changed**
1616

17+
- fixed `sagemaker-templates` Model Deploy seed code S3 permission race condition where `grant_read_write()` on an imported bucket created a `DefaultPolicy` with no CloudFormation dependency from the SageMaker Model, causing intermittent `s3:GetObject` access denied errors
1718
- consolidated redundant `DevStage`/`PreProdStage`/`ProdStage` classes into a single `DeployStage` in `sagemaker-templates` model deploy seed code, fixing redundant CF stack names (e.g. `dev-dev-endpoint``dev-{project}-endpoint`) and adding project uniqueness to prevent cross-project collisions
1819

1920
## v3.2.3

modules/sagemaker/sagemaker-templates/templates/model_deploy/seed_code/deploy_app/deploy_app/deploy_endpoint_stack.py

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from aws_cdk import aws_ec2 as ec2
1212
from aws_cdk import aws_iam as iam
1313
from aws_cdk import aws_kms as kms
14-
from aws_cdk import aws_s3 as s3
1514
from aws_cdk import aws_sagemaker as sagemaker
1615
from config.config_mux import StageYamlDataClassConfig
1716
from config.constants import (
@@ -135,7 +134,46 @@ def __init__(
135134
),
136135
)
137136

138-
model_bucket = s3.Bucket.from_bucket_arn(self, "ModelBucket", MODEL_BUCKET_ARN)
137+
# Add S3 read/write permissions directly to the managed policy document.
138+
#
139+
# We intentionally avoid using bucket.grant_read_write() here because
140+
# the model bucket would be imported via from_bucket_arn (S3 ARNs contain no account),
141+
# and this stack deploys inside a Stage with a concrete account ID. CDK's
142+
# Grant.addToPrincipalOrResource compares the bucket's unresolved ${AWS::AccountId}
143+
# token against the concrete account string, gets ONE_UNRESOLVED, and falls through
144+
# to the resource-policy path. This causes two problems:
145+
#
146+
# 1. With a ManagedPolicy grantee (the original code), CDK 2.174.0+ rejects the
147+
# ManagedPolicy as an invalid Principal when constructing the resource policy
148+
# statement (aws/aws-cdk#32795).
149+
#
150+
# 2. With a Role grantee (PR #396's fix), CDK creates a separate DefaultPolicy
151+
# inline resource with no CloudFormation dependency from the CfnModel, causing
152+
# an IAM propagation race where SageMaker's CreateModel API runs before the S3
153+
# permissions are attached to the role.
154+
#
155+
# Adding statements directly to the ManagedPolicy avoids both issues: permissions
156+
# are embedded in the policy document at creation time, and the existing dependency
157+
# chain (ManagedPolicy -> Role -> CfnModel) guarantees they are effective before
158+
# the model is created.
159+
model_execution_policy.add_statements(
160+
iam.PolicyStatement(
161+
actions=[
162+
"s3:GetObject*",
163+
"s3:GetBucket*",
164+
"s3:List*",
165+
"s3:DeleteObject*",
166+
"s3:PutObject",
167+
"s3:PutObjectLegalHold",
168+
"s3:PutObjectRetention",
169+
"s3:PutObjectTagging",
170+
"s3:PutObjectVersionTagging",
171+
"s3:Abort*",
172+
],
173+
effect=iam.Effect.ALLOW,
174+
resources=[MODEL_BUCKET_ARN, f"{MODEL_BUCKET_ARN}/*"],
175+
),
176+
)
139177

140178
if ECR_REPO_ARN:
141179
model_execution_policy.add_statements(
@@ -153,9 +191,6 @@ def __init__(
153191
managed_policies=[model_execution_policy],
154192
)
155193

156-
# Grant S3 read/write permissions to the role (not the ManagedPolicy)
157-
model_bucket.grant_read_write(model_execution_role)
158-
159194
# setup timestamp to be used to trigger the custom resource update event to retrieve
160195
# latest approved model and to be used with model and endpoint config resources' names
161196
now = datetime.now().replace(tzinfo=timezone.utc)
@@ -242,8 +277,23 @@ def __init__(
242277
kms_key_id=kms_key.key_id,
243278
)
244279

245-
# Grant write permissions for data capture
246-
model_bucket.grant_write(model_execution_role, "endpoint-data-capture/*")
280+
# Grant write permissions for data capture (same pattern as above —
281+
# add directly to the managed policy to avoid the imported-bucket grant issue)
282+
model_execution_policy.add_statements(
283+
iam.PolicyStatement(
284+
actions=[
285+
"s3:DeleteObject*",
286+
"s3:PutObject",
287+
"s3:PutObjectLegalHold",
288+
"s3:PutObjectRetention",
289+
"s3:PutObjectTagging",
290+
"s3:PutObjectVersionTagging",
291+
"s3:Abort*",
292+
],
293+
effect=iam.Effect.ALLOW,
294+
resources=[f"{MODEL_BUCKET_ARN}/endpoint-data-capture/*"],
295+
),
296+
)
247297
else:
248298
data_capture_config = None
249299

0 commit comments

Comments
 (0)