feat(gateway): add create_web_search_target() helper - #656
Conversation
✅ No Breaking Changes DetectedNo public API breaking changes found in this PR. |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
Adds a GatewayClient helper that creates a gateway target exposing Amazon Web Search as an MCP WebSearch tool, so callers do not have to hand-assemble the connector target configuration. - Defaults the target name to "amazon-web-search", because Gateway prefixes every tool with its target name and the agent therefore discovers "amazon-web-search___WebSearch". - Supports target-level include and exclude domain lists, connector version pinning, an agent-facing description, and per-parameter overrides. - Always sends parameterValues, even when empty. The service drops any configuration whose parameterValues is absent before validating, so a configuration carrying nothing but a name leaves nothing to validate and CreateGatewayTarget fails with "Connector configurations must not be empty". - Documents that domain include lists intersect rather than replace, so a caller can narrow the target's list but never relax it, and disjoint lists return no results without raising. - Documents the two IAM actions involved: the caller needs InvokeGateway on the gateway ARN, the gateway service role needs InvokeWebSearch on the connector. Web search takes no API key of its own. Integration tests skip only when the account is not entitled to the web-search connector and re-raise every other error.
f6f2d83 to
cabcba3
Compare
|
Claude Security Review: no high-confidence findings. (run) |
tejaskash
left a comment
There was a problem hiding this comment.
A few comments inline. The main one is the include_domains default version. Also the PR description still describes an earlier revision: it says parameterValues is omitted when there are no exclusions (it is always sent as {}), lists include_domains and connector version as out of scope (both are implemented), says 8 tests (there are 11), and shows the default name as web-search (code uses amazon-web-search). Worth refreshing before merge.
|
|
||
| source: Dict[str, Any] = {"connectorId": "web-search"} | ||
| if connector_version: | ||
| source["version"] = connector_version |
There was a problem hiding this comment.
If include_domains is set and connector_version is not, this sends domainFilter.include against the connector default (1.1.0), which only 1.2.0+ accepts. The service rejects it with a ValidationException. Since the point of the helper is to hide connector details, either default source["version"] to "1.2.0" when include_domains is set, or raise a ValueError here so the caller gets a clear message instead of a server-side error.
| } | ||
| assert config["parameterOverrides"] == [{"path": "$.maxResults", "visible": True}] | ||
|
|
||
| def test_include_domains_only(self): |
There was a problem hiding this comment.
This asserts a request the service will reject (see comment on client.py). Once the version default or validation is added, this test needs to change with it.
| config = call_kwargs["targetConfiguration"]["mcp"]["connector"]["configurations"][0] | ||
| assert config["parameterValues"] == {"domainFilter": {"include": ["docs.aws.amazon.com"]}} | ||
|
|
||
| def test_connector_version_omitted_by_default(self): |
There was a problem hiding this comment.
test_minimal already asserts the full request with exact equality, so this test, test_no_domain_filter_when_no_exclude_domains, test_parameter_values_always_present, and test_default_credential_provider add no new coverage. Suggest dropping them. test_empty_exclude_domains_is_omitted is the only one in this group with a distinct input and should stay.
|
|
||
| @classmethod | ||
| def setup_class(cls): | ||
| cls.region = os.environ.get("BEDROCK_TEST_REGION", "us-east-1") |
There was a problem hiding this comment.
Every other gateway integ test defaults to us-west-2, and the CI workflow sets AWS_REGION=us-west-2 without setting BEDROCK_TEST_REGION. This file defaults to us-east-1, so the gateway gets created in a different region than the rest of the suite. If the account is not entitled in us-east-1, all three tests skip on every CI run. Worth confirming which region CI should actually use here.
| cls.gateway_id = None | ||
| cls.target_ids = [] | ||
|
|
||
| gw = cls.gateway_client.create_gateway_and_wait( |
There was a problem hiding this comment.
When the connector is not available, all three tests skip but setup_class still creates a real gateway and teardown_class deletes it on every run. Consider checking availability here, before create_gateway_and_wait, and skipping the whole class instead.
| def _create_target(self, **kwargs): | ||
| """Create a web search target, skipping the test if the account is not entitled. | ||
|
|
||
| The web-search connector is enabled per account. When it is not, CreateGatewayTarget |
There was a problem hiding this comment.
This says the connector is enabled per account, but the module docstring says it is only offered in three regions. If the skip in CI is really a region mismatch, a reader will look at the wrong thing. Worth reconciling the two explanations.
Description
Adds
GatewayClient.create_web_search_target(), a typed one-call helper for wiring the managedweb-searchconnector to a gateway.GatewayClientalready has helpers for the other two connectors it supports:create_knowledge_base_target()(bedrock-knowledge-bases) andcreate_agentic_retrieve_target()(bedrock-agentic-retrieve). Web search has no equivalent, so callers have to hand-assemble thetargetConfiguration.mcp.connectorblock and the credential provider configuration themselves. This closes that gap and follows the same shape as its two siblings.This is typed convenience, not new capability.
create_gateway_target_and_wait()can already create the same target; the helper removes the need to know the connector ID, the tool name, and the parameter path.Part of #654. That issue also asks for a
WebSearchClientunderbedrock_agentcore.tools; this PR does not include that, since the invoke-side details are still open (see Out of scope).What it produces
sends:
{ "gatewayIdentifier": "gw-123", "name": "web-search", "targetConfiguration": { "mcp": { "connector": { "source": {"connectorId": "web-search"}, "enabled": ["WebSearch"], "configurations": [ { "name": "WebSearch", "parameterValues": {"domainFilter": {"exclude": ["example.com"]}} } ] } } }, "credentialProviderConfigurations": [{"credentialProviderType": "GATEWAY_IAM_ROLE"}] }parameterValuesis omitted entirely whenexclude_domainsis not passed or is empty, so the minimal call sends only{"name": "WebSearch"}as the configuration entry.Where the constants come from
The two values that are not inferable from this repo are the connector ID and the tool name. Both are taken from the CLI, which already ships a web-search path:
connectorId: 'web-search'and the tool nameWebSearch:aws/agentcore-clisrc/cli/operations/connectors/translators.tsparameterValues.domainFilter = { exclude: [...] }: same file,translateWebSearch()The surrounding wire shape (
enabled,configurations,GATEWAY_IAM_ROLE) mirrorscreate_knowledge_base_target()in this file.Note on capitalization: the
ConnectorConfigurationshape in the service model writes its example tool names in lower camel case (retrieve,webSearch), while the shippedcreate_knowledge_base_target()here sendsRetrieveand the CLI sendsWebSearch. This PR follows the shipped code rather than the doc example. This is the one thing the live run could not confirm, because validation stopped at the entitlement check before reaching tool names.Testing
tests/unit/gateway/test_gateway_web_search_targets.py, 8 tests asserting the exact wire shape, the default name,parameterValuesomission,**kwargsoverride behavior, the default credential provider, andwait_configpass-through. Modeled on the existingtest_gateway_kb_targets.py.tests_integ/gateway/test_gateway_web_search_targets.py, 3 opt-in tests marked@pytest.mark.integration, requiringGATEWAY_ROLE_ARN, creating and tearing down a real gateway and targets. Modeled on the existingtests_integ/gateway/test_gateway_kb_targets.py.Full unit suite: 3412 passed, 10 skipped, 4 xpassed.
ruff checkandruff format --checkclean on all three files.What the live run showed
The
Test (gateway)CI job runstests_integ/gatewayagainst a real account inus-west-2, so the first push did exercise this against a liveCreateGatewayTarget. All three tests failed with the same error:The request the SDK built was accepted structurally and rejected at the account entitlement check, with the service naming
web-searchback. The rest of the gateway suite, including the KB target tests, passed in the same run.Two things follow, and the second one is a real limit on this PR:
enabled: ["WebSearch"]or thedomainFilter.excludepath. Those are still only verified against the CLI's implementation.The second push makes the integration tests
pytest.skipon that specific message and still fail on anything else, so the job reports honestly instead of red. To get the remaining verification, the integ test account needs the connector enabled, or someone on an enabled account should run:Out of scope
WebSearchClientinbedrock_agentcore.tools. The other built-in tools have data-plane clients because they have data-plane APIs (StartBrowserSession,StartCodeInterpreterSession). Web search is reached over MCP through a gateway, so an equivalent client needs a decision on how it signs and transports requests. Tracked in [FEATURE] Add a WebSearch client to bedrock_agentcore.tools and a create_web_search_target() helper #654.version.ConnectorSourceaccepts an optionalversion; neither existing connector helper exposes it, so this one does not either. Omitting it means the service picks the latest.domainFilter.excludeis set here, because that is the only parameter with a verified path. Anything else a caller needs can go throughparameter_overridesor by passingtargetConfigurationdirectly, both of which still work.Checklist
web-searchconnector enabledUnrelated CI failures
None of the red jobs on this PR touch gateway code:
Compat (evaluation)errors withModuleNotFoundError: No module named 'deepeval'and'autoevals'in that job's own install step. It fails the same way on chore(deps-dev): update langchain-community requirement from <0.4.2,>=0.3.0 to >=0.3.0,<0.4.3 #653, ci: remove synchronize trigger from Slack PR review caller #626 and ci: add Slack PR review notification caller #625, including the two baselined against v1.21.0.Compat (memory)andTest (memory)fail withServiceQuotaExceededException: Memory count limit exceeded for account ...: Current: 150, Max allowed: 150, plus aDeleteMemorytimeout on a memory stuck inUPDATING. That is integ-account state:Compat (memory)reproduces it while running v1.22.0's own tests, and it also fails on chore(deps-dev): update langchain-community requirement from <0.4.2,>=0.3.0 to >=0.3.0,<0.4.3 #653, ci: remove synchronize trigger from Slack PR review caller #626 and ci: add Slack PR review notification caller #625.Test Python 3.13reports1 failed, 3411 passed. The one failure istests/bedrock_agentcore/payments/integrations/langgraph/test_stage3.py::TestRetryDelay::test_zero_delay_skips_sleep, withExpected 'sleep' to not have been called. Called 762 times.That test patches...langgraph.middleware.time.sleep, which sets the attribute on the sharedtimemodule object rather than a module-local alias, so any othertime.sleepcall in the same interpreter is counted against the mock while the patch is active. The full suite passes locally on this branch (3412 passed, 10 skipped, 4 xpassed), and the same job passes on chore(deps-dev): update langchain-community requirement from <0.4.2,>=0.3.0 to >=0.3.0,<0.4.3 #653, ci: remove synchronize trigger from Slack PR review caller #626 and ci: add Slack PR review notification caller #625.Test Python 3.10has no failing step. Every step, includingUpload coverage to Codecov, is green; the job is red only because of matrix fail-fast, annotatedThe strategy configuration was canceled because "test._3_13" failed.Test (gateway), the job that covers this change, passes.