From 8813f8155d96f2bcab18f4eb09a0fafe60af9656 Mon Sep 17 00:00:00 2001 From: Mohamed Zeidan Date: Wed, 26 Aug 2026 19:13:53 -0700 Subject: [PATCH 1/2] evaluator_model was checked against a hardcoded _ALLOWED_EVALUATOR_MODELS dict (model -> regions) in sagemaker/train/constants.py. That list is triplicated across clients and goes stale: when a judge model reaches end of life it still passes client-side validation, so the eval job spins up and only fails deep inside the in-container Bedrock CreateEvaluationJob call, wasting compute and surfacing a poor error. Replace it with two-step validation against authoritative sources: - Construction: fetch the service-maintained supported-judge-models list at s3://jumpstart-cache-prod-/fmhMetadata/supported-llmaj-judge-models.json and fail fast if evaluator_model is not a supported judge model. - evaluate(): call bedrock:GetFoundationModel and fail fast if the model is unavailable in the region or past its endOfLifeTime. The lookup is gated on the caller's IAM permission via a new non-raising caller_can_perform() helper that mirrors the existing SimulatePrincipalPolicy caller-check pattern (verify_evaluation_caller_permissions). Both steps degrade gracefully instead of blocking: if a source can't be read (missing bedrock:GetFoundationModel permission, unreadable list, or a transient error) the SDK logs an actionable warning with a link to the supported models and continues. - Remove _ALLOWED_EVALUATOR_MODELS from sagemaker/train/constants.py - Add caller_can_perform() to sagemaker/core/helper/iam_role_resolver.py - Add unit tests for both validation steps and caller_can_perform --- .../core/helper/iam_role_resolver.py | 53 +++ .../unit/helper/test_iam_role_resolver.py | 59 +++ .../src/sagemaker/train/constants.py | 27 +- .../train/evaluate/llm_as_judge_evaluator.py | 243 +++++++++- .../evaluate/test_llm_as_judge_evaluator.py | 435 ++++++++++++++++-- 5 files changed, 727 insertions(+), 90 deletions(-) diff --git a/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py b/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py index eee9b0eed5..a254264373 100644 --- a/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py +++ b/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py @@ -758,6 +758,59 @@ def verify_evaluation_caller_permissions( return True +def caller_can_perform( + actions: List[str], sagemaker_session=None +) -> Optional[bool]: + """Return whether the caller identity is allowed to perform ALL of ``actions``. + + Read-only, non-raising sibling of :func:`verify_evaluation_caller_permissions`: + it resolves the caller's backing IAM role and simulates ``actions`` against it + with ``iam:SimulatePrincipalPolicy``. Use it to gate an optional client-side + AWS call on whether the caller is actually permitted to make it, degrading + gracefully (rather than raising) when that cannot be determined. + + ``actions`` should be account-level / wildcard-resource actions — they are + simulated without ``ResourceArns``, so resource-scoped actions can come back + ``implicitDeny`` even for a caller who holds them. + + Args: + actions: IAM action names to check (e.g. ``["bedrock:GetFoundationModel"]``). + sagemaker_session: SageMaker session (used to get the boto session). + + Returns: + True — every action is allowed. + False — at least one action is denied. + None — could not be determined (caller is not a role, cannot call + ``sts:GetCallerIdentity``, or lacks ``iam:SimulatePrincipalPolicy``). + """ + boto_session = _get_boto_session(sagemaker_session) + sts_client = boto_session.client("sts") + iam_client = boto_session.client("iam") + + try: + caller_identity = sts_client.get_caller_identity() + except ClientError: + return None + + caller_arn = caller_identity["Arn"] + account_id = caller_identity["Account"] + partition = _partition_from_arn(caller_arn) + + caller_role_arn = _resolve_caller_role_arn(iam_client, caller_arn, account_id, partition) + if not caller_role_arn: + return None + + try: + denied = _simulate_denied_actions(iam_client, caller_role_arn, list(actions)) + except ClientError as e: + error_code = e.response.get("Error", {}).get("Code", "") + if error_code in ("AccessDenied", "AccessDeniedException"): + return None + raise + + return not denied + + # --------------------------------------------------------------------------- # Opt-in IAM execution-role creation. # diff --git a/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py b/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py index 48fcdf3be0..0c6ad7689e 100644 --- a/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py +++ b/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py @@ -11,6 +11,7 @@ RoleValidationError, resolve_and_validate_role, verify_hyperpod_connect_permissions, + caller_can_perform, HYPERPOD_CLI_CONNECT_ACTIONS, _load_policy_config, _get_required_actions, @@ -1081,3 +1082,61 @@ class TestBackwardCompatibleExceptions: def test_role_auto_creation_error_importable(self): assert issubclass(RoleAutoCreationError, Exception) + + +class TestCallerCanPerform: + """caller_can_perform() — non-raising caller-permission probe.""" + + _ASSUMED_ROLE = "arn:aws:sts::123456789012:assumed-role/MyRole/session" + + def _paginator_denying(self, allowed, denied): + paginator = MagicMock() + results = [{"EvalActionName": a, "EvalDecision": "allowed"} for a in allowed] + results += [{"EvalActionName": a, "EvalDecision": "implicitDeny"} for a in denied] + paginator.paginate.return_value = [{"EvaluationResults": results}] + return paginator + + def test_all_actions_allowed_returns_true(self): + mock_session, mock_iam, _ = _make_session(self._ASSUMED_ROLE) + mock_iam.get_role.return_value = { + "Role": {"Arn": "arn:aws:iam::123456789012:role/MyRole"} + } + mock_iam.get_paginator.return_value = _paginator_allowing( + ["bedrock:GetFoundationModel"] + ) + + assert ( + caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is True + ) + + def test_denied_action_returns_false(self): + mock_session, mock_iam, _ = _make_session(self._ASSUMED_ROLE) + mock_iam.get_role.return_value = { + "Role": {"Arn": "arn:aws:iam::123456789012:role/MyRole"} + } + mock_iam.get_paginator.return_value = self._paginator_denying( + allowed=[], denied=["bedrock:GetFoundationModel"] + ) + + assert ( + caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is False + ) + + def test_caller_not_a_role_returns_none(self): + # An IAM user (not an assumed role) has no backing role to simulate. + mock_session, _, _ = _make_session("arn:aws:iam::123456789012:user/alice") + assert caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is None + + def test_cannot_simulate_returns_none(self): + mock_session, mock_iam, _ = _make_session(self._ASSUMED_ROLE) + mock_iam.get_role.return_value = { + "Role": {"Arn": "arn:aws:iam::123456789012:role/MyRole"} + } + paginator = MagicMock() + paginator.paginate.side_effect = ClientError( + {"Error": {"Code": "AccessDenied", "Message": "no simulate"}}, + "SimulatePrincipalPolicy", + ) + mock_iam.get_paginator.return_value = paginator + + assert caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is None diff --git a/sagemaker-train/src/sagemaker/train/constants.py b/sagemaker-train/src/sagemaker/train/constants.py index ee3034e236..b0767697cd 100644 --- a/sagemaker-train/src/sagemaker/train/constants.py +++ b/sagemaker-train/src/sagemaker/train/constants.py @@ -58,25 +58,14 @@ def get_sagemaker_hub_name() -> str: "qwen.qwen3-235b-a22b-2507-v1:0": ["us-west-2", "ap-northeast-1"] } -# Allowed evaluator models for LLM as Judge evaluator with region restrictions. -# -# Source of truth: the Bedrock Console judge-model regional -# allowlist.cross-checked against -# https://docs.aws.amazon.com/bedrock/latest/userguide/evaluation-judge.html#evaluation-judge-supported -_ALLOWED_EVALUATOR_MODELS = { - "mistral.mistral-large-2402-v1:0": ["us-west-2", "us-east-1", "eu-west-1"], - "meta.llama3-1-70b-instruct-v1:0": ["us-west-2", "us-east-1"], - "anthropic.claude-3-haiku-20240307-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "anthropic.claude-haiku-4-5-20251001-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "anthropic.claude-sonnet-4-5-20250929-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "anthropic.claude-opus-4-5-20251101-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "amazon.nova-pro-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "amazon.nova-2-lite-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "amazon.nova-micro-v1:0": ["us-west-2", "us-east-1", "ap-northeast-1", "eu-west-1"], - "amazon.nova-premier-v1:0": ["us-west-2", "us-east-1"], - "anthropic.claude-3-5-sonnet-20240620-v1:0": ["ap-northeast-1"], - "anthropic.claude-3-5-sonnet-20241022-v2:0": ["ap-northeast-1"], -} +# NOTE: The former hardcoded ``_ALLOWED_EVALUATOR_MODELS`` allowlist for the +# LLM-as-Judge evaluator has been removed. evaluator_model is now validated in two +# steps (see ``sagemaker.train.evaluate.llm_as_judge_evaluator``): at construction +# against the service-maintained supported-judge-models list at +# ``s3://jumpstart-cache-prod-/fmhMetadata/supported-llmaj-judge-models.json`` +# (is it a judge-capable model), and at evaluate() time against Bedrock +# ``GetFoundationModel`` (is it still in service / not past end of life). So the SDK +# no longer needs a hand-maintained model→region map. SM_RECIPE = "recipe" SM_RECIPE_YAML = "recipe.yaml" diff --git a/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py b/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py index c83a0934ae..d7ab26bcf9 100644 --- a/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py +++ b/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py @@ -7,7 +7,7 @@ import json import logging import uuid -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Set, Union from pydantic import root_validator, validator @@ -25,11 +25,75 @@ from sagemaker.train.common_utils.data_utils import validate_data_path_exists from sagemaker.train.common_utils.model_aliases import NOVA_BEDROCK_MODEL_IDS from sagemaker.train.common_utils.recipe_utils import _is_nova_model -from sagemaker.train.constants import _ALLOWED_EVALUATOR_MODELS from sagemaker.train.defaults import TrainDefaults _logger = logging.getLogger(__name__) +# Documentation listing the Bedrock foundation models supported as LLM-as-Judge +# evaluators. Surfaced to users when evaluator_model validation cannot run or fails. +_EVALUATOR_JUDGE_DOCS_URL = ( + "https://docs.aws.amazon.com/bedrock/latest/userguide/" + "evaluation-judge.html#evaluation-judge-supported" +) + +# S3 key of the service-maintained supported-judge-models list, mirrored per region +# under the JumpStart cache bucket. This file is the source of truth for which +# Bedrock models are supported as LLM-as-Judge evaluators (kept current by the +# Bedrock evaluation control plane), so the SDK reads it instead of hardcoding a list. +_SUPPORTED_JUDGE_MODELS_S3_KEY = "fmhMetadata/supported-llmaj-judge-models.json" + + +def _supported_judge_models_s3_uri(region: str) -> str: + """Return the S3 URI of the supported-judge-models list for ``region``.""" + return f"s3://jumpstart-cache-prod-{region}/{_SUPPORTED_JUDGE_MODELS_S3_KEY}" + + +def _fetch_supported_judge_model_ids(session: Any, region: str) -> Optional[Set[str]]: + """Fetch the set of supported LLM-as-Judge model IDs for ``region``. + + Reads ``s3://jumpstart-cache-prod-/fmhMetadata/supported-llmaj-judge-models.json``, + the per-region mirror of the Bedrock evaluation control plane's supported-judge + allowlist. Because that file is service-maintained and drops models that reach + end of life, membership answers both "is this a supported judge model" and + "is it still current". + + This never raises: if the list cannot be fetched or parsed (missing object, + denied access, network error, or unexpected shape) it returns ``None`` so the + caller can fall back to the non-blocking degradation path. + + Args: + session: SageMaker session used to read the object. + region: AWS region whose JumpStart cache bucket holds the list. + + Returns: + A non-empty set of model ID strings, or ``None`` if unavailable. + """ + from sagemaker.core.s3.client import S3Downloader + + s3_uri = _supported_judge_models_s3_uri(region) + try: + raw = S3Downloader.read_file(s3_uri=s3_uri, sagemaker_session=session) + doc = json.loads(raw) + except Exception: # noqa: BLE001 - degrade gracefully on any fetch/parse error + return None + + if not isinstance(doc, dict): + return None + entries = doc.get("supported_judge_models") + if not isinstance(entries, list): + return None + + model_ids: Set[str] = set() + for entry in entries: + if isinstance(entry, dict): + model_id = entry.get("model_id") + if isinstance(model_id, str): + model_ids.add(model_id) + + # An empty set means the file parsed but yielded nothing usable — treat as + # "cannot verify" and degrade rather than reject every model. + return model_ids or None + def _resolve_bedrock_model_id(base_model_name: str, region: str) -> Optional[str]: """Derive Bedrock inference profile ID from JumpStart model name + region. @@ -202,27 +266,157 @@ def _validate_model_compatibility(cls, values): @validator('evaluator_model') def _validate_evaluator_model(cls, v, values): - """Validate evaluator_model is allowed and check region compatibility.""" - - if v not in _ALLOWED_EVALUATOR_MODELS: + """Validate that evaluator_model is a supported judge model (construction step 1). + + Fetches the service-maintained supported-judge-models list for the + session's region (see :func:`_fetch_supported_judge_model_ids`) and fails + fast at construction if ``v`` is not in it. This list is the catalog of + judge-*capable* models; it can still contain models that have reached end + of life, so it only answers "is this a valid judge model" — whether the + model is still in service is checked separately, at evaluate() time, by + :meth:`_check_evaluator_model_lifecycle`. + + Degradation route: if the list cannot be retrieved (no session/region, or + the file cannot be read/parsed), emit a warning and continue without + blocking — the evaluation job may still succeed. + """ + session = values.get('sagemaker_session') + region = None + if session is not None and hasattr(session, 'boto_region_name'): + region = session.boto_region_name + if not region: + region = values.get('region') + + supported_model_ids = None + if session is not None and region: + supported_model_ids = _fetch_supported_judge_model_ids(session, region) + + if supported_model_ids is None: + _logger.warning( + "The SDK couldn't retrieve the list of supported judge models, so it " + "can't confirm '%s' is a valid judge model. The evaluation will still " + "run, but it may fail if the model isn't supported. See the list of " + "supported judge models: %s", + v, + _EVALUATOR_JUDGE_DOCS_URL, + ) + return v + + if v not in supported_model_ids: raise ValueError( - f"Invalid evaluator_model '{v}'. " - f"Allowed models are: {list(_ALLOWED_EVALUATOR_MODELS.keys())}" + f"evaluator_model '{v}' is not a supported LLM-as-Judge model in " + f"region '{region}'. Choose one of the supported judge models. " + f"See {_EVALUATOR_JUDGE_DOCS_URL}" ) - - # Get current region from session - session = values.get('sagemaker_session') - if session and hasattr(session, 'boto_region_name'): - current_region = session.boto_region_name - allowed_regions = _ALLOWED_EVALUATOR_MODELS[v] - - if current_region not in allowed_regions: - raise ValueError( - f"Evaluator model '{v}' is not available in region '{current_region}'. " - f"Available regions for this model: {allowed_regions}" - ) - + return v + + def _check_evaluator_model_lifecycle(self, region: str) -> None: + """Fail fast if evaluator_model is retired (past end of life) in ``region``. + + Evaluate() step 2, complementing the construction-time supported-model + check. The supported-judge-models list is a superset that can still list + models past end of life, so this queries Bedrock for the model's live + lifecycle and raises before the job is submitted when the model is no + longer usable. + + The Bedrock lookup is gated on the caller's IAM permissions, mirroring the + repo's caller-permission pattern (:func:`caller_can_perform` / + :func:`verify_evaluation_caller_permissions`): + + * Caller allowed to call ``bedrock:GetFoundationModel`` → look up the + model and raise if it is absent in the region or past ``endOfLifeTime``. + * Caller not allowed, or it cannot be determined (e.g. no + ``iam:SimulatePrincipalPolicy``) → emit a user-friendly warning that the + system cannot verify whether the model is still in service, and continue. + + Args: + region: AWS region resolved for the evaluation. + """ + from datetime import datetime, timezone + + from botocore.exceptions import ClientError + + from sagemaker.core.helper.iam_role_resolver import ( + _get_boto_session, + caller_can_perform, + ) + + allowed = caller_can_perform( + ["bedrock:GetFoundationModel"], self.sagemaker_session + ) + if allowed is False: + # The caller's role definitively lacks the permission. + _logger.warning( + "Your IAM role does not include the bedrock:GetFoundationModel " + "permission, so the SDK can't check whether the evaluator model '%s' " + "is still in service or has reached end of life. The evaluation will " + "still run, but it may fail if this model has been retired. Add " + "bedrock:GetFoundationModel to your role to enable this check. See the " + "list of supported judge models: %s", + self.evaluator_model, + _EVALUATOR_JUDGE_DOCS_URL, + ) + return + if allowed is None: + # We could not confirm the permission (identity isn't a role, or + # iam:SimulatePrincipalPolicy is unavailable) — don't assert it's missing. + _logger.warning( + "The SDK couldn't confirm your IAM role includes the " + "bedrock:GetFoundationModel permission, so it can't check whether the " + "evaluator model '%s' is still in service or has reached end of life. " + "The evaluation will still run, but it may fail if this model has been " + "retired. See the list of supported judge models: %s", + self.evaluator_model, + _EVALUATOR_JUDGE_DOCS_URL, + ) + return + + boto_session = _get_boto_session(self.sagemaker_session) + try: + client = boto_session.client("bedrock", region_name=region) + response = client.get_foundation_model(modelIdentifier=self.evaluator_model) + except ClientError as e: + error_code = e.response.get("Error", {}).get("Code", "") + if error_code in ("ResourceNotFoundException", "ValidationException"): + raise ValueError( + f"evaluator_model '{self.evaluator_model}' is not available in " + f"region '{region}'. It may be unsupported in this region or have " + f"reached end of life. Choose a judge model that is in service in " + f"this region. See {_EVALUATOR_JUDGE_DOCS_URL}" + ) from e + # Any other error (throttling, service issue): don't block the user. + _logger.warning( + "The SDK couldn't verify whether the evaluator model '%s' is still in " + "service right now (a temporary error occurred). The evaluation will " + "still run, but it may fail if this model has been retired. See the " + "list of supported judge models: %s", + self.evaluator_model, + _EVALUATOR_JUDGE_DOCS_URL, + ) + return + except Exception: # noqa: BLE001 - degrade gracefully on any client error + _logger.warning( + "The SDK couldn't verify whether the evaluator model '%s' is still in " + "service right now (a temporary error occurred). The evaluation will " + "still run, but it may fail if this model has been retired. See the " + "list of supported judge models: %s", + self.evaluator_model, + _EVALUATOR_JUDGE_DOCS_URL, + ) + return + + details = response.get("modelDetails", {}) if isinstance(response, dict) else {} + lifecycle = details.get("modelLifecycle", {}) if isinstance(details, dict) else {} + end_of_life = lifecycle.get("endOfLifeTime") if isinstance(lifecycle, dict) else None + + if isinstance(end_of_life, datetime) and end_of_life <= datetime.now(timezone.utc): + raise ValueError( + f"evaluator_model '{self.evaluator_model}' has reached end of life in " + f"region '{region}' (end-of-life {end_of_life.isoformat()}) and can no " + f"longer be used as a judge. Choose a judge model that is in service. " + f"See {_EVALUATOR_JUDGE_DOCS_URL}" + ) def _should_use_inspectai_path(self) -> bool: """Determine if the InspectAI path should be used for Phase 1 inference. @@ -826,7 +1020,14 @@ def evaluate(self, dry_run: bool = False): aws_context = self._get_aws_execution_context(role_type="model_eval") region = aws_context['region'] role_arn = aws_context['role_arn'] - + + # Step 2 of evaluator_model validation: fail fast (before submitting the job) + # if the judge model has reached end of life. The construction-time check + # only confirmed the model is judge-capable; this confirms it is still in + # service. Gated on caller permissions — warns and continues if it can't be + # verified. + self._check_evaluator_model_lifecycle(region) + # Resolve model artifacts artifacts = self._resolve_model_artifacts(region) diff --git a/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py b/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py index cc84d94f6e..462d7af705 100644 --- a/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py +++ b/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py @@ -14,13 +14,68 @@ from __future__ import absolute_import import json +from datetime import datetime, timedelta, timezone + import pytest from unittest.mock import patch, Mock +from botocore.exceptions import ClientError from pydantic import ValidationError from sagemaker.train.evaluate.llm_as_judge_evaluator import LLMAsJudgeEvaluator from sagemaker.train.evaluate.constants import EvalType +# Where the evaluator reads the supported-judge-models list from. +_S3_READ_FILE_PATH = "sagemaker.core.s3.client.S3Downloader.read_file" +# The caller-permission helper the lifecycle check gates on. +_CALLER_CAN_PERFORM_PATH = ( + "sagemaker.core.helper.iam_role_resolver.caller_can_perform" +) + + +def _configure_bedrock_get_model(mock_session, lifecycle=None, side_effect=None): + """Wire mock_session.boto_session.client('bedrock').get_foundation_model. + + Args: + mock_session: Mock session whose boto_session is a Mock. + lifecycle: dict placed at modelDetails.modelLifecycle in the response. + side_effect: if set, raised by get_foundation_model instead of returning. + """ + bedrock_client = Mock() + if side_effect is not None: + bedrock_client.get_foundation_model.side_effect = side_effect + else: + bedrock_client.get_foundation_model.return_value = { + "modelDetails": {"modelLifecycle": lifecycle or {"status": "ACTIVE"}} + } + mock_session.boto_session.client.return_value = bedrock_client + return bedrock_client + + +def _supported_models_doc(model_ids): + """Build a supported-llmaj-judge-models.json body listing ``model_ids``.""" + return json.dumps( + { + "schema_version": "1.0", + "supported_judge_models": [{"model_id": mid} for mid in model_ids], + } + ) + + +def _patch_supported_models(model_ids=None, side_effect=None): + """Patch S3Downloader.read_file to serve a supported-judge-models list. + + Args: + model_ids: iterable of model_ids the list should contain. + side_effect: if provided, set as read_file's side_effect (e.g. an error) + instead of returning a document body. + """ + if side_effect is not None: + return patch(_S3_READ_FILE_PATH, side_effect=side_effect) + return patch( + _S3_READ_FILE_PATH, return_value=_supported_models_doc(model_ids or []) + ) + + # Test constants DEFAULT_REGION = "us-west-2" DEFAULT_ROLE = "arn:aws:iam::123456789012:role/test-role" @@ -877,89 +932,102 @@ def test_llm_as_judge_evaluator_valid_evaluator_models(mock_artifact, mock_resol mock_session.boto_region_name = "us-west-2" # Region where all models including nova-pro are available mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE - - for model in valid_models: - evaluator = LLMAsJudgeEvaluator( - model=DEFAULT_MODEL, - evaluator_model=model, - dataset=DEFAULT_DATASET, - builtin_metrics=["Correctness"], - s3_output_path=DEFAULT_S3_OUTPUT, - mlflow_resource_arn=DEFAULT_MLFLOW_ARN, - model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, - sagemaker_session=mock_session, - ) - assert evaluator.evaluator_model == model + + # The supported-judge-models list reports every model under test as supported. + with _patch_supported_models(model_ids=valid_models): + for model in valid_models: + evaluator = LLMAsJudgeEvaluator( + model=DEFAULT_MODEL, + evaluator_model=model, + dataset=DEFAULT_DATASET, + builtin_metrics=["Correctness"], + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + assert evaluator.evaluator_model == model @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @patch('sagemaker.core.resources.Artifact') def test_llm_as_judge_evaluator_invalid_evaluator_model(mock_artifact, mock_resolve): - """Test LLMAsJudgeEvaluator raises error for invalid evaluator model.""" + """Test LLMAsJudgeEvaluator fails fast when the model is not in the supported list. + + Covers both never-supported models and EOL models: neither appears in the + service-maintained supported-judge-models list, so construction raises. + """ mock_info = Mock() mock_info.base_model_name = DEFAULT_MODEL mock_info.base_model_arn = DEFAULT_BASE_MODEL_ARN mock_info.source_model_package_arn = None mock_resolve.return_value = mock_info - + mock_artifact.get_all.return_value = iter([]) mock_artifact_instance = Mock() mock_artifact_instance.artifact_arn = DEFAULT_ARTIFACT_ARN mock_artifact.create.return_value = mock_artifact_instance - + mock_session = Mock() mock_session.boto_region_name = DEFAULT_REGION mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE - - with pytest.raises(ValidationError) as exc_info: - LLMAsJudgeEvaluator( - model=DEFAULT_MODEL, - evaluator_model="invalid-model", - dataset=DEFAULT_DATASET, - builtin_metrics=["Correctness"], - s3_output_path=DEFAULT_S3_OUTPUT, - mlflow_resource_arn=DEFAULT_MLFLOW_ARN, - model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, - sagemaker_session=mock_session, - ) - assert "Invalid evaluator_model 'invalid-model'" in str(exc_info.value) + + # The supported list contains real models, but not "invalid-model". + supported = [DEFAULT_EVALUATOR_MODEL, "anthropic.claude-3-haiku-20240307-v1:0"] + with _patch_supported_models(model_ids=supported): + with pytest.raises(ValidationError) as exc_info: + LLMAsJudgeEvaluator( + model=DEFAULT_MODEL, + evaluator_model="invalid-model", + dataset=DEFAULT_DATASET, + builtin_metrics=["Correctness"], + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + assert "is not a supported LLM-as-Judge model" in str(exc_info.value) + assert "invalid-model" in str(exc_info.value) @patch('sagemaker.train.defaults.TrainDefaults.get_sagemaker_session') @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @patch('sagemaker.core.resources.Artifact') def test_llm_as_judge_evaluator_region_restriction(mock_artifact, mock_resolve, mock_get_session): - """Test LLMAsJudgeEvaluator raises error for model not available in region.""" + """Test LLMAsJudgeEvaluator raises when the model is absent from a region's list.""" mock_info = Mock() mock_info.base_model_name = DEFAULT_MODEL mock_info.base_model_arn = DEFAULT_BASE_MODEL_ARN mock_info.source_model_package_arn = None mock_resolve.return_value = mock_info - + mock_artifact.get_all.return_value = iter([]) mock_artifact_instance = Mock() mock_artifact_instance.artifact_arn = DEFAULT_ARTIFACT_ARN mock_artifact.create.return_value = mock_artifact_instance - + mock_session = Mock() mock_session.boto_region_name = "eu-central-1" # Region not supported for nova-pro mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE mock_get_session.return_value = mock_session - - with pytest.raises(ValidationError) as exc_info: - LLMAsJudgeEvaluator( - model=DEFAULT_MODEL, - evaluator_model="amazon.nova-pro-v1:0", - dataset=DEFAULT_DATASET, - builtin_metrics=["Correctness"], - s3_output_path=DEFAULT_S3_OUTPUT, - mlflow_resource_arn=DEFAULT_MLFLOW_ARN, - model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, - sagemaker_session=mock_session, - ) - assert "not available in region" in str(exc_info.value) + + # The eu-central-1 supported-judge-models list does not include nova-pro. + with _patch_supported_models(model_ids=["anthropic.claude-3-haiku-20240307-v1:0"]): + with pytest.raises(ValidationError) as exc_info: + LLMAsJudgeEvaluator( + model=DEFAULT_MODEL, + evaluator_model="amazon.nova-pro-v1:0", + dataset=DEFAULT_DATASET, + builtin_metrics=["Correctness"], + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + assert "is not a supported LLM-as-Judge model" in str(exc_info.value) + assert "eu-central-1" in str(exc_info.value) @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @@ -1033,11 +1101,11 @@ def test_non_nova_jumpstart_model_uses_existing_path(mock_artifact, mock_resolve @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @patch('sagemaker.core.resources.Artifact') def test_nova_model_rejected_in_unsupported_region(mock_artifact, mock_resolve): - """Test that Nova model in unsupported region fails validation. + """Test that a Nova base model in an unsupported region fails validation. - In practice, the evaluator_model region validator fires first when both - the evaluator model and the Bedrock prefix are unsupported in a region. - This test verifies that construction fails with a region-related error. + evaluator_model validation degrades gracefully here (no Bedrock list is + available from the bare mock), so the Nova cross-region-inference + compatibility root-validator is what blocks construction. """ mock_info = Mock() mock_info.base_model_name = "nova-textgeneration-lite" @@ -1055,7 +1123,7 @@ def test_nova_model_rejected_in_unsupported_region(mock_artifact, mock_resolve): mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE - with pytest.raises(ValueError, match="not available in region"): + with pytest.raises(ValueError, match="not supported for"): LLMAsJudgeEvaluator( evaluator_model=DEFAULT_EVALUATOR_MODEL, dataset=DEFAULT_DATASET, @@ -1065,3 +1133,270 @@ def test_nova_model_rejected_in_unsupported_region(mock_artifact, mock_resolve): model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, sagemaker_session=mock_session, ) + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_evaluator_model_validation_degrades_when_list_unreadable(mock_artifact, mock_resolve): + """If the supported-judge-models list can't be read, construction must NOT block. + + This is the degradation route (e.g. denied access or a missing object): we + warn that the model can't be verified but continue rather than block the user. + """ + mock_info = Mock() + mock_info.base_model_name = DEFAULT_MODEL + mock_info.base_model_arn = DEFAULT_BASE_MODEL_ARN + mock_info.source_model_package_arn = None + mock_resolve.return_value = mock_info + + mock_artifact.get_all.return_value = iter([]) + mock_artifact_instance = Mock() + mock_artifact_instance.artifact_arn = DEFAULT_ARTIFACT_ARN + mock_artifact.create.return_value = mock_artifact_instance + + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + + # Reading the list fails (denied access / missing object / network error). + with _patch_supported_models(side_effect=Exception("access denied")): + evaluator = LLMAsJudgeEvaluator( + evaluator_model=DEFAULT_EVALUATOR_MODEL, + dataset=DEFAULT_DATASET, + model=DEFAULT_MODEL, + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + assert evaluator.evaluator_model == DEFAULT_EVALUATOR_MODEL + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_evaluator_model_validation_degrades_on_malformed_list(mock_artifact, mock_resolve): + """A malformed/unexpected list document must NOT block construction.""" + mock_info = Mock() + mock_info.base_model_name = DEFAULT_MODEL + mock_info.base_model_arn = DEFAULT_BASE_MODEL_ARN + mock_info.source_model_package_arn = None + mock_resolve.return_value = mock_info + + mock_artifact.get_all.return_value = iter([]) + mock_artifact_instance = Mock() + mock_artifact_instance.artifact_arn = DEFAULT_ARTIFACT_ARN + mock_artifact.create.return_value = mock_artifact_instance + + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + + # File is present but not the expected shape (no supported_judge_models array). + with patch(_S3_READ_FILE_PATH, return_value=json.dumps({"unexpected": True})): + evaluator = LLMAsJudgeEvaluator( + evaluator_model=DEFAULT_EVALUATOR_MODEL, + dataset=DEFAULT_DATASET, + model=DEFAULT_MODEL, + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + assert evaluator.evaluator_model == DEFAULT_EVALUATOR_MODEL + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_evaluator_model_validation_degrades_without_region(mock_artifact, mock_resolve): + """No resolvable region means validation is skipped (non-blocking) with a warning.""" + mock_info = Mock() + mock_info.base_model_name = DEFAULT_MODEL + mock_info.base_model_arn = DEFAULT_BASE_MODEL_ARN + mock_info.source_model_package_arn = None + mock_resolve.return_value = mock_info + + mock_artifact.get_all.return_value = iter([]) + mock_artifact_instance = Mock() + mock_artifact_instance.artifact_arn = DEFAULT_ARTIFACT_ARN + mock_artifact.create.return_value = mock_artifact_instance + + mock_session = Mock() + mock_session.boto_region_name = None # No region resolvable from session + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + + with patch(_S3_READ_FILE_PATH) as mock_read_file: + evaluator = LLMAsJudgeEvaluator( + evaluator_model=DEFAULT_EVALUATOR_MODEL, + dataset=DEFAULT_DATASET, + model=DEFAULT_MODEL, + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + assert evaluator.evaluator_model == DEFAULT_EVALUATOR_MODEL + # The list should not be fetched when no region is available. + mock_read_file.assert_not_called() + + +# --------------------------------------------------------------------------- +# _check_evaluator_model_lifecycle (evaluate()-time end-of-life check) +# --------------------------------------------------------------------------- +def _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session): + """Construct an evaluator (supported-model check stubbed out) for lifecycle tests.""" + mock_info = Mock() + mock_info.base_model_name = DEFAULT_MODEL + mock_info.base_model_arn = DEFAULT_BASE_MODEL_ARN + mock_info.source_model_package_arn = None + mock_resolve.return_value = mock_info + + mock_artifact.get_all.return_value = iter([]) + mock_artifact_instance = Mock() + mock_artifact_instance.artifact_arn = DEFAULT_ARTIFACT_ARN + mock_artifact.create.return_value = mock_artifact_instance + + with _patch_supported_models(model_ids=[DEFAULT_EVALUATOR_MODEL]): + return LLMAsJudgeEvaluator( + evaluator_model=DEFAULT_EVALUATOR_MODEL, + dataset=DEFAULT_DATASET, + model=DEFAULT_MODEL, + s3_output_path=DEFAULT_S3_OUTPUT, + mlflow_resource_arn=DEFAULT_MLFLOW_ARN, + model_package_group=DEFAULT_MODEL_PACKAGE_GROUP_ARN, + sagemaker_session=mock_session, + ) + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_active_model_passes(mock_artifact, mock_resolve): + """An in-service (ACTIVE) judge model passes the lifecycle check.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + bedrock_client = _configure_bedrock_get_model( + mock_session, lifecycle={"status": "ACTIVE"} + ) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + + bedrock_client.get_foundation_model.assert_called_once_with( + modelIdentifier=DEFAULT_EVALUATOR_MODEL + ) + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_future_eol_passes(mock_artifact, mock_resolve): + """A LEGACY model whose end-of-life is still in the future is still usable.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + future = datetime.now(timezone.utc) + timedelta(days=30) + _configure_bedrock_get_model( + mock_session, lifecycle={"status": "LEGACY", "endOfLifeTime": future} + ) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_past_eol_raises(mock_artifact, mock_resolve): + """A model past its end-of-life fails fast before the job is submitted.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + past = datetime.now(timezone.utc) - timedelta(days=1) + _configure_bedrock_get_model( + mock_session, lifecycle={"status": "LEGACY", "endOfLifeTime": past} + ) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): + with pytest.raises(ValueError, match="reached end of life"): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_model_not_found_raises(mock_artifact, mock_resolve): + """A model absent from the region (ResourceNotFound) fails fast.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + not_found = ClientError( + {"Error": {"Code": "ResourceNotFoundException", "Message": "no such model"}}, + "GetFoundationModel", + ) + _configure_bedrock_get_model(mock_session, side_effect=not_found) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): + with pytest.raises(ValueError, match="not available in region"): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_no_permission_warns_and_continues(mock_artifact, mock_resolve): + """Without bedrock:GetFoundationModel permission, we warn and do NOT block.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + bedrock_client = _configure_bedrock_get_model(mock_session) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=False): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + + # Permission denied → the model is never looked up. + bedrock_client.get_foundation_model.assert_not_called() + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_permission_undetermined_warns_and_continues(mock_artifact, mock_resolve): + """When permission can't be determined (None), we warn and do NOT block.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + bedrock_client = _configure_bedrock_get_model(mock_session) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=None): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + + bedrock_client.get_foundation_model.assert_not_called() + + +@patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') +@patch('sagemaker.core.resources.Artifact') +def test_lifecycle_transient_bedrock_error_does_not_block(mock_artifact, mock_resolve): + """A transient Bedrock error (e.g. throttling) must NOT block construction/submit.""" + mock_session = Mock() + mock_session.boto_region_name = DEFAULT_REGION + mock_session.boto_session = Mock() + mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) + + throttling = ClientError( + {"Error": {"Code": "ThrottlingException", "Message": "slow down"}}, + "GetFoundationModel", + ) + _configure_bedrock_get_model(mock_session, side_effect=throttling) + with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise From 0a874754e4506aba8127941d4efd9d9156ef9049 Mon Sep 17 00:00:00 2001 From: Mohamed Zeidan Date: Thu, 27 Aug 2026 09:18:17 -0700 Subject: [PATCH 2/2] additions --- .../core/helper/iam_role_resolver.py | 53 ------- .../unit/helper/test_iam_role_resolver.py | 59 -------- .../train/evaluate/llm_as_judge_evaluator.py | 129 ++++++++---------- .../evaluate/test_llm_as_judge_evaluator.py | 83 +++++------ 4 files changed, 104 insertions(+), 220 deletions(-) diff --git a/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py b/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py index a254264373..eee9b0eed5 100644 --- a/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py +++ b/sagemaker-core/src/sagemaker/core/helper/iam_role_resolver.py @@ -758,59 +758,6 @@ def verify_evaluation_caller_permissions( return True -def caller_can_perform( - actions: List[str], sagemaker_session=None -) -> Optional[bool]: - """Return whether the caller identity is allowed to perform ALL of ``actions``. - - Read-only, non-raising sibling of :func:`verify_evaluation_caller_permissions`: - it resolves the caller's backing IAM role and simulates ``actions`` against it - with ``iam:SimulatePrincipalPolicy``. Use it to gate an optional client-side - AWS call on whether the caller is actually permitted to make it, degrading - gracefully (rather than raising) when that cannot be determined. - - ``actions`` should be account-level / wildcard-resource actions — they are - simulated without ``ResourceArns``, so resource-scoped actions can come back - ``implicitDeny`` even for a caller who holds them. - - Args: - actions: IAM action names to check (e.g. ``["bedrock:GetFoundationModel"]``). - sagemaker_session: SageMaker session (used to get the boto session). - - Returns: - True — every action is allowed. - False — at least one action is denied. - None — could not be determined (caller is not a role, cannot call - ``sts:GetCallerIdentity``, or lacks ``iam:SimulatePrincipalPolicy``). - """ - boto_session = _get_boto_session(sagemaker_session) - sts_client = boto_session.client("sts") - iam_client = boto_session.client("iam") - - try: - caller_identity = sts_client.get_caller_identity() - except ClientError: - return None - - caller_arn = caller_identity["Arn"] - account_id = caller_identity["Account"] - partition = _partition_from_arn(caller_arn) - - caller_role_arn = _resolve_caller_role_arn(iam_client, caller_arn, account_id, partition) - if not caller_role_arn: - return None - - try: - denied = _simulate_denied_actions(iam_client, caller_role_arn, list(actions)) - except ClientError as e: - error_code = e.response.get("Error", {}).get("Code", "") - if error_code in ("AccessDenied", "AccessDeniedException"): - return None - raise - - return not denied - - # --------------------------------------------------------------------------- # Opt-in IAM execution-role creation. # diff --git a/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py b/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py index 0c6ad7689e..48fcdf3be0 100644 --- a/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py +++ b/sagemaker-core/tests/unit/helper/test_iam_role_resolver.py @@ -11,7 +11,6 @@ RoleValidationError, resolve_and_validate_role, verify_hyperpod_connect_permissions, - caller_can_perform, HYPERPOD_CLI_CONNECT_ACTIONS, _load_policy_config, _get_required_actions, @@ -1082,61 +1081,3 @@ class TestBackwardCompatibleExceptions: def test_role_auto_creation_error_importable(self): assert issubclass(RoleAutoCreationError, Exception) - - -class TestCallerCanPerform: - """caller_can_perform() — non-raising caller-permission probe.""" - - _ASSUMED_ROLE = "arn:aws:sts::123456789012:assumed-role/MyRole/session" - - def _paginator_denying(self, allowed, denied): - paginator = MagicMock() - results = [{"EvalActionName": a, "EvalDecision": "allowed"} for a in allowed] - results += [{"EvalActionName": a, "EvalDecision": "implicitDeny"} for a in denied] - paginator.paginate.return_value = [{"EvaluationResults": results}] - return paginator - - def test_all_actions_allowed_returns_true(self): - mock_session, mock_iam, _ = _make_session(self._ASSUMED_ROLE) - mock_iam.get_role.return_value = { - "Role": {"Arn": "arn:aws:iam::123456789012:role/MyRole"} - } - mock_iam.get_paginator.return_value = _paginator_allowing( - ["bedrock:GetFoundationModel"] - ) - - assert ( - caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is True - ) - - def test_denied_action_returns_false(self): - mock_session, mock_iam, _ = _make_session(self._ASSUMED_ROLE) - mock_iam.get_role.return_value = { - "Role": {"Arn": "arn:aws:iam::123456789012:role/MyRole"} - } - mock_iam.get_paginator.return_value = self._paginator_denying( - allowed=[], denied=["bedrock:GetFoundationModel"] - ) - - assert ( - caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is False - ) - - def test_caller_not_a_role_returns_none(self): - # An IAM user (not an assumed role) has no backing role to simulate. - mock_session, _, _ = _make_session("arn:aws:iam::123456789012:user/alice") - assert caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is None - - def test_cannot_simulate_returns_none(self): - mock_session, mock_iam, _ = _make_session(self._ASSUMED_ROLE) - mock_iam.get_role.return_value = { - "Role": {"Arn": "arn:aws:iam::123456789012:role/MyRole"} - } - paginator = MagicMock() - paginator.paginate.side_effect = ClientError( - {"Error": {"Code": "AccessDenied", "Message": "no simulate"}}, - "SimulatePrincipalPolicy", - ) - mock_iam.get_paginator.return_value = paginator - - assert caller_can_perform(["bedrock:GetFoundationModel"], mock_session) is None diff --git a/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py b/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py index d7ab26bcf9..d12b9f705d 100644 --- a/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py +++ b/sagemaker-train/src/sagemaker/train/evaluate/llm_as_judge_evaluator.py @@ -53,13 +53,15 @@ def _fetch_supported_judge_model_ids(session: Any, region: str) -> Optional[Set[ Reads ``s3://jumpstart-cache-prod-/fmhMetadata/supported-llmaj-judge-models.json``, the per-region mirror of the Bedrock evaluation control plane's supported-judge - allowlist. Because that file is service-maintained and drops models that reach - end of life, membership answers both "is this a supported judge model" and - "is it still current". + allowlist. Membership answers only "is this a judge-capable model" — the list + is a superset that can still include models past end of life, so whether the + model is currently in service is checked separately by + :meth:`LLMAsJudgeEvaluator._check_evaluator_model_lifecycle`. This never raises: if the list cannot be fetched or parsed (missing object, denied access, network error, or unexpected shape) it returns ``None`` so the - caller can fall back to the non-blocking degradation path. + caller can fall back to the non-blocking degradation path. The two failure + modes are logged at ``debug`` to aid diagnosis without adding user-facing noise. Args: session: SageMaker session used to read the object. @@ -74,13 +76,17 @@ def _fetch_supported_judge_model_ids(session: Any, region: str) -> Optional[Set[ try: raw = S3Downloader.read_file(s3_uri=s3_uri, sagemaker_session=session) doc = json.loads(raw) - except Exception: # noqa: BLE001 - degrade gracefully on any fetch/parse error + except Exception as e: # noqa: BLE001 - degrade gracefully on any fetch/parse error + _logger.debug("Could not read supported-judge-models list from %s: %s", s3_uri, e) return None - if not isinstance(doc, dict): - return None - entries = doc.get("supported_judge_models") + entries = doc.get("supported_judge_models") if isinstance(doc, dict) else None if not isinstance(entries, list): + _logger.debug( + "Supported-judge-models list at %s has an unexpected shape (missing a " + "'supported_judge_models' array); skipping validation.", + s3_uri, + ) return None model_ids: Set[str] = set() @@ -90,9 +96,16 @@ def _fetch_supported_judge_model_ids(session: Any, region: str) -> Optional[Set[ if isinstance(model_id, str): model_ids.add(model_id) - # An empty set means the file parsed but yielded nothing usable — treat as - # "cannot verify" and degrade rather than reject every model. - return model_ids or None + if not model_ids: + # Parsed, but no usable model IDs — treat as "cannot verify" and degrade + # rather than reject every model. + _logger.debug( + "Supported-judge-models list at %s parsed but contained no model IDs; " + "skipping validation.", + s3_uri, + ) + return None + return model_ids def _resolve_bedrock_model_id(base_model_name: str, region: str) -> Optional[str]: @@ -316,19 +329,22 @@ def _check_evaluator_model_lifecycle(self, region: str) -> None: Evaluate() step 2, complementing the construction-time supported-model check. The supported-judge-models list is a superset that can still list - models past end of life, so this queries Bedrock for the model's live - lifecycle and raises before the job is submitted when the model is no - longer usable. - - The Bedrock lookup is gated on the caller's IAM permissions, mirroring the - repo's caller-permission pattern (:func:`caller_can_perform` / - :func:`verify_evaluation_caller_permissions`): - - * Caller allowed to call ``bedrock:GetFoundationModel`` → look up the - model and raise if it is absent in the region or past ``endOfLifeTime``. - * Caller not allowed, or it cannot be determined (e.g. no - ``iam:SimulatePrincipalPolicy``) → emit a user-friendly warning that the - system cannot verify whether the model is still in service, and continue. + models past end of life, so this queries Bedrock ``GetFoundationModel`` + for the model's live lifecycle and raises before the job is submitted when + the model is no longer usable. + + The permission needed for the lookup (``bedrock:GetFoundationModel``) is a + resource-scoped action, so we do NOT pre-check it with + ``iam:SimulatePrincipalPolicy`` — simulating a resource-scoped action + without ``ResourceArns`` yields false ``implicitDeny`` verdicts for callers + who scope their grants, which would silently skip this very check. Instead + we call ``GetFoundationModel`` directly and interpret the result: + + * ``ResourceNotFoundException`` / ``ValidationException`` → the model is + not available in the region (unsupported or fully retired) → raise. + * ``endOfLifeTime`` in the past → the model has reached end of life → raise. + * ``AccessDenied`` → the caller lacks the permission → warn and continue. + * any other error (throttling, service issue) → warn and continue. Args: region: AWS region resolved for the evaluation. @@ -337,47 +353,18 @@ def _check_evaluator_model_lifecycle(self, region: str) -> None: from botocore.exceptions import ClientError - from sagemaker.core.helper.iam_role_resolver import ( - _get_boto_session, - caller_can_perform, - ) - - allowed = caller_can_perform( - ["bedrock:GetFoundationModel"], self.sagemaker_session - ) - if allowed is False: - # The caller's role definitively lacks the permission. - _logger.warning( - "Your IAM role does not include the bedrock:GetFoundationModel " - "permission, so the SDK can't check whether the evaluator model '%s' " - "is still in service or has reached end of life. The evaluation will " - "still run, but it may fail if this model has been retired. Add " - "bedrock:GetFoundationModel to your role to enable this check. See the " - "list of supported judge models: %s", - self.evaluator_model, - _EVALUATOR_JUDGE_DOCS_URL, - ) - return - if allowed is None: - # We could not confirm the permission (identity isn't a role, or - # iam:SimulatePrincipalPolicy is unavailable) — don't assert it's missing. - _logger.warning( - "The SDK couldn't confirm your IAM role includes the " - "bedrock:GetFoundationModel permission, so it can't check whether the " - "evaluator model '%s' is still in service or has reached end of life. " - "The evaluation will still run, but it may fail if this model has been " - "retired. See the list of supported judge models: %s", - self.evaluator_model, - _EVALUATOR_JUDGE_DOCS_URL, - ) - return + from sagemaker.core.helper.iam_role_resolver import _get_boto_session boto_session = _get_boto_session(self.sagemaker_session) try: client = boto_session.client("bedrock", region_name=region) response = client.get_foundation_model(modelIdentifier=self.evaluator_model) - except ClientError as e: - error_code = e.response.get("Error", {}).get("Code", "") + except Exception as e: # noqa: BLE001 - map Bedrock errors, degrade on the rest + error_code = ( + e.response.get("Error", {}).get("Code", "") + if isinstance(e, ClientError) + else "" + ) if error_code in ("ResourceNotFoundException", "ValidationException"): raise ValueError( f"evaluator_model '{self.evaluator_model}' is not available in " @@ -385,6 +372,18 @@ def _check_evaluator_model_lifecycle(self, region: str) -> None: f"reached end of life. Choose a judge model that is in service in " f"this region. See {_EVALUATOR_JUDGE_DOCS_URL}" ) from e + if error_code in ("AccessDeniedException", "AccessDenied", "UnauthorizedOperation"): + _logger.warning( + "Your IAM role does not include the bedrock:GetFoundationModel " + "permission, so the SDK can't check whether the evaluator model " + "'%s' is still in service or has reached end of life. The " + "evaluation will still run, but it may fail if this model has been " + "retired. Add bedrock:GetFoundationModel to your role to enable " + "this check. See the list of supported judge models: %s", + self.evaluator_model, + _EVALUATOR_JUDGE_DOCS_URL, + ) + return # Any other error (throttling, service issue): don't block the user. _logger.warning( "The SDK couldn't verify whether the evaluator model '%s' is still in " @@ -395,16 +394,6 @@ def _check_evaluator_model_lifecycle(self, region: str) -> None: _EVALUATOR_JUDGE_DOCS_URL, ) return - except Exception: # noqa: BLE001 - degrade gracefully on any client error - _logger.warning( - "The SDK couldn't verify whether the evaluator model '%s' is still in " - "service right now (a temporary error occurred). The evaluation will " - "still run, but it may fail if this model has been retired. See the " - "list of supported judge models: %s", - self.evaluator_model, - _EVALUATOR_JUDGE_DOCS_URL, - ) - return details = response.get("modelDetails", {}) if isinstance(response, dict) else {} lifecycle = details.get("modelLifecycle", {}) if isinstance(details, dict) else {} diff --git a/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py b/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py index 462d7af705..b5a3f8516e 100644 --- a/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py +++ b/sagemaker-train/tests/unit/train/evaluate/test_llm_as_judge_evaluator.py @@ -26,10 +26,6 @@ # Where the evaluator reads the supported-judge-models list from. _S3_READ_FILE_PATH = "sagemaker.core.s3.client.S3Downloader.read_file" -# The caller-permission helper the lifecycle check gates on. -_CALLER_CAN_PERFORM_PATH = ( - "sagemaker.core.helper.iam_role_resolver.caller_can_perform" -) def _configure_bedrock_get_model(mock_session, lifecycle=None, side_effect=None): @@ -1283,8 +1279,7 @@ def test_lifecycle_active_model_passes(mock_artifact, mock_resolve): bedrock_client = _configure_bedrock_get_model( mock_session, lifecycle={"status": "ACTIVE"} ) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise bedrock_client.get_foundation_model.assert_called_once_with( modelIdentifier=DEFAULT_EVALUATOR_MODEL @@ -1305,8 +1300,7 @@ def test_lifecycle_future_eol_passes(mock_artifact, mock_resolve): _configure_bedrock_get_model( mock_session, lifecycle={"status": "LEGACY", "endOfLifeTime": future} ) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @@ -1323,9 +1317,8 @@ def test_lifecycle_past_eol_raises(mock_artifact, mock_resolve): _configure_bedrock_get_model( mock_session, lifecycle={"status": "LEGACY", "endOfLifeTime": past} ) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): - with pytest.raises(ValueError, match="reached end of life"): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) + with pytest.raises(ValueError, match="reached end of life"): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @@ -1343,60 +1336,74 @@ def test_lifecycle_model_not_found_raises(mock_artifact, mock_resolve): "GetFoundationModel", ) _configure_bedrock_get_model(mock_session, side_effect=not_found) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): - with pytest.raises(ValueError, match="not available in region"): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) + with pytest.raises(ValueError, match="not available in region"): + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @patch('sagemaker.core.resources.Artifact') -def test_lifecycle_no_permission_warns_and_continues(mock_artifact, mock_resolve): - """Without bedrock:GetFoundationModel permission, we warn and do NOT block.""" +def test_lifecycle_access_denied_warns_and_continues(mock_artifact, mock_resolve): + """AccessDenied from GetFoundationModel → warn about the permission, don't block. + + We call Bedrock directly (no SimulatePrincipalPolicy pre-gate), so a missing — + including a scoped — permission surfaces here as AccessDenied and degrades. + """ mock_session = Mock() mock_session.boto_region_name = DEFAULT_REGION mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) - bedrock_client = _configure_bedrock_get_model(mock_session) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=False): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise - - # Permission denied → the model is never looked up. - bedrock_client.get_foundation_model.assert_not_called() + denied = ClientError( + {"Error": {"Code": "AccessDeniedException", "Message": "not authorized"}}, + "GetFoundationModel", + ) + _configure_bedrock_get_model(mock_session, side_effect=denied) + # Should NOT raise — degrades with a permission-specific warning. + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @patch('sagemaker.core.resources.Artifact') -def test_lifecycle_permission_undetermined_warns_and_continues(mock_artifact, mock_resolve): - """When permission can't be determined (None), we warn and do NOT block.""" +def test_lifecycle_transient_bedrock_error_does_not_block(mock_artifact, mock_resolve): + """A transient Bedrock error (e.g. throttling) must NOT block construction/submit.""" mock_session = Mock() mock_session.boto_region_name = DEFAULT_REGION mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) - bedrock_client = _configure_bedrock_get_model(mock_session) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=None): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise - - bedrock_client.get_foundation_model.assert_not_called() + throttling = ClientError( + {"Error": {"Code": "ThrottlingException", "Message": "slow down"}}, + "GetFoundationModel", + ) + _configure_bedrock_get_model(mock_session, side_effect=throttling) + evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise @patch('sagemaker.train.common_utils.model_resolution._resolve_base_model') @patch('sagemaker.core.resources.Artifact') -def test_lifecycle_transient_bedrock_error_does_not_block(mock_artifact, mock_resolve): - """A transient Bedrock error (e.g. throttling) must NOT block construction/submit.""" +def test_evaluate_invokes_lifecycle_check(mock_artifact, mock_resolve): + """evaluate() must call _check_evaluator_model_lifecycle with the resolved region.""" mock_session = Mock() mock_session.boto_region_name = DEFAULT_REGION mock_session.boto_session = Mock() mock_session.get_caller_identity_arn.return_value = DEFAULT_ROLE + mock_session.sagemaker_config = None # let the telemetry decorator resolve cleanly evaluator = _build_lifecycle_evaluator(mock_artifact, mock_resolve, mock_session) - throttling = ClientError( - {"Error": {"Code": "ThrottlingException", "Message": "slow down"}}, - "GetFoundationModel", - ) - _configure_bedrock_get_model(mock_session, side_effect=throttling) - with patch(_CALLER_CAN_PERFORM_PATH, return_value=True): - evaluator._check_evaluator_model_lifecycle(DEFAULT_REGION) # no raise + sentinel = RuntimeError("lifecycle-check-invoked") + aws_context = { + "role_arn": DEFAULT_ROLE, + "region": DEFAULT_REGION, + "account_id": "123456789012", + } + with patch.object(evaluator, "_get_resolved_model_info", return_value=None), \ + patch.object(evaluator, "_get_aws_execution_context", return_value=aws_context), \ + patch.object( + evaluator, "_check_evaluator_model_lifecycle", side_effect=sentinel + ) as mock_lifecycle: + with pytest.raises(RuntimeError, match="lifecycle-check-invoked"): + evaluator.evaluate() + + mock_lifecycle.assert_called_once_with(DEFAULT_REGION)