fix(sdk/python): add AGENTFIELD_SKIP_IP_DETECTION opt-out for container IP probing - #921
fix(sdk/python): add AGENTFIELD_SKIP_IP_DETECTION opt-out for container IP probing#921AmirF194 wants to merge 3 commits into
Conversation
…er IP probing _build_callback_candidates() calls _detect_container_ip() unconditionally whenever _is_running_in_container() is true, regardless of whether an explicit callback URL is already known. That function fires blocking requests to cloud-metadata endpoints (169.254.169.254, metadata.google.internal) and falls back to the third-party api.ipify.org, with no way to turn it off. In a Kubernetes NetworkPolicy-restricted environment this floods deny logs and leaks pod egress to a third party for a best-effort callback candidate that is often unused. Add AGENTFIELD_SKIP_IP_DETECTION (default false, same boolean-env-var convention used elsewhere in this module) and gate the probe behind it. Default behavior is unchanged. Fixes Agent-Field#624
|
|
santoshkumarradha
left a comment
There was a problem hiding this comment.
Thanks for tightening this up. The opt-out gate is small, the networking tests passed locally, and the docs note is clear. The only remaining blocker I see is the CLA check. Once that’s signed, this should be good to go.
|
Thanks for the review. Working on getting the CLA signed, will follow up here once it's cleared. |
Performance
✓ No regressions detected |
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
|
Closing this in favour of #969, which lands the same opt-out from a clean-room implementation (written from #624's description, not from this diff) and also skips the probe automatically when a callback URL is already configured — the case pocesar originally reported. The CLA has been pending for ten days and we'd rather not leave #624 open longer; the diagnosis and the env-var approach were yours and #969 credits you for them. If you sign the CLA later we'd be glad to take future contributions. |
|
Thanks for the update, and for crediting the diagnosis. Glad to see #969 merged and covering the callback URL case too. Working on getting the CLA signed, would like to contribute again here. |
Root cause
_build_callback_candidates()insdk/python/agentfield/agent.pycalls_detect_container_ip()unconditionally whenever_is_running_in_container()is true, with no way to opt out._detect_container_ip()sends blocking requests to AWS/Azure/GCP cloud-metadata endpoints (169.254.169.254,metadata.google.internal) and falls back to the third-partyapi.ipify.org, regardless of whether an explicitAGENT_CALLBACK_URL(or constructorcallback_url) is already set. In a Kubernetes deployment with a restrictiveNetworkPolicy, this fires on every agent start, flooding deny logs, and it also leaks pod egress to a third party for a best-effort candidate that most deployments never use.Fix
Add
AGENTFIELD_SKIP_IP_DETECTION(defaultfalse, same boolean-env-var convention already used byAGENTFIELD_LOG_PAYLOADS/AGENTFIELD_LOG_TRACKING/AGENTFIELD_LOG_FIREinlogger.py) and gate the_detect_container_ip()call behind it. Default behavior is unchanged; the Railway-specific hint and the local-network/hostname/localhost fallbacks are untouched. Documented indocs/ENVIRONMENT_VARIABLES.md.On the prior closed attempts
Four earlier PRs on this issue (#698, #740, #770, #834) were closed as placeholder tests dropped into a root
tests/directory that isn't part of this repo's layout, without an actual fix. This PR is different: it changes the real behavior insdk/python/agentfield/agent.pyand adds tests insdk/python/tests/test_agent_networking.py, the file that already covers this exact function, following its existingmonkeypatchstyle.Verification
test_build_callback_candidates_skips_ip_detection_when_disabled,test_build_callback_candidates_runs_ip_detection_by_default,test_ip_detection_disabled_is_case_insensitive,test_ip_detection_disabled_defaults_to_false) fail on unmodifiedmain(AttributeError/ probe still called) and pass on this branch, in a clean Docker container.sdk/pythonsuite (./scripts/run_pytest.sh) green on Python 3.10 and 3.12 (the matrix's low and high ends): 1968 passed, 4 pre-existing skips, no new failures.ruff check .on this branch reports the same 9 pre-existing findings as unmodifiedmain, all inside abuild/lib/pip-build artifact directory unrelated to this diff; nothing in the changed files.NetworkPolicyblocking egress (no cluster in this sandbox). The fix is a control-flow gate on an existing, already-tested function, not a change to its detection logic.