sdk: a caller's label requirement is met by any one pair, as in sam-node - #514
Merged
Merged
Conversation
sam-node renders X-Sam-Required-Labels and call_remote_tool's required_labels as `check if label(k1, v1) or label(k2, v2)` (api.LabelCheck), so a requirement of several pairs is met by any one of them. The JS and Python SDKs required every pair, so the same call succeeded through a sam-node and was refused by an SDK. The SDKs now apply the any-of rule; the operator's egress floor stays the conjunction and sam-node's alone. TestNativeSDKsMesh runs the labels_gate_test.go matrix through all three implementations against control-plane-attested credentials: each SDK requiring labels of the sam-node over /sam/mcp/1.0.0, and the sam-node requiring labels of each SDK agent through X-Sam-Required-Labels. With the previous predicate the SDK legs fail on the "any-of requirement matches one key" case while the Go leg passes. Reported-by: Hosni Belfeki <https://github.com/HosniBelfeki> Fixes google#508
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements caller-side label requirements for MCP sessions across the JavaScript and Python SDKs. It updates the label matching logic to support an 'any-of' requirement, where a set of label pairs is satisfied if at least one pair matches the provider's credential. The changes include updates to the command interfaces, protocol handling, and the addition of comprehensive integration tests in tests/integration/sdk_mesh_test.go to verify the label matching matrix across different implementations. I have no feedback to provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #508. Reported by @HosniBelfeki, with the proof from
internal/node/labels_gate_test.go.What was wrong
sam-node renders
X-Sam-Required-Labelsandcall_remote_tool'srequired_labelsascheck if label(k1, v1) or label(k2, v2)(api.LabelCheck): a requirement of several pairs is met by any one of them. The JS and Python SDKs required every pair, so the same call succeeded through a sam-node and was refused by an SDK.Change
sdk/js/src/mcp.tsrequireLabelsandsdk/python/src/agent_mesh/mcp_client.pyrequire_labelsapply the any-of rule sam-node applies; the refusal says the provider "carries none of the required labels". Unit tests carry thelabels_gate_test.gomatrix in both languages.egress.require_labels,api.LabelFloorCheck) is the conjunction and stays sam-node's alone; the SDKs have no floor.sdk/README.mdand the Native SDKs guide state the rule.Interoperability test
TestNativeSDKsMeshgained arequired-labelsmatrix run through all three implementations against control-plane-attested credentials. The sam-node and every SDK member attest the same labels (the policy grantsregion=*,team=*); the four cases oflabels_gate_test.goare then run/sam/mcp/1.0.0(toolswithrequired_labels), andX-Sam-Required-Labels, expecting 200 or 403.With the previous predicates the SDK legs fail on "any-of requirement matches one key" while the Go leg passes; with this change all three agree.
The conformance runners take
SAM_SDK_LABELSat enrollment andrequired_labelsontools/callfor this.Validation:
npm test(66),pytest(79),TestNativeSDKsMesh,hack/verify-sdk-generated.sh,go vet.