Skip to content

fix: verify datasource access and bound external observability connections - #180

Closed
Atom-oh wants to merge 2 commits into
devfrom
fix/datasource-external-connections
Closed

Atom-oh wants to merge 2 commits into
devfrom
fix/datasource-external-connections

Conversation

@Atom-oh

@Atom-oh Atom-oh commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This hardens the eight datasource HTTP adapters and the administrator-managed custom MCP path. It has two commits.

1. Verify datasource access and disclose incomplete results

  • The Datadog connection test checks metric-query access (API-key validation alone, or a bearer token, no longer passes).
  • Datadog and Dynatrace mark truncated series as collectionStatus: partial.
  • The datasource list reports instance-specific configuration evidence only, never a borrowed kind default or a connectivity claim.

2. Bound and verify external connections

  • Upstream bodies are capped before parsing (4 MiB success / 64 KiB error), and streams close on every exit. Overflow stays an explicit error.
  • A missing explicit Lambda/worker instance ID fails closed. No-ID legacy default resolution still works.
  • Connection tests for Prometheus, Mimir, Loki, Tempo, Jaeger, Dynatrace and ClickHouse run the real bounded query operation instead of a readiness endpoint. The Datadog metric-permission probe is unchanged.
  • Jaeger, Dynatrace, Datadog and Loki expose completion states. Malformed records are reported as unverified, never as absence.
  • Prometheus and Mimir enforce the saved execution ceiling inside the connector, with aligned HTTP deadlines. Other kinds apply HTTP response limits. The form shows the effective limit (55s maximum).
  • Jaeger limits are clamped and responses report requested/returned counts. A saturated limit is marked potentially partial.
  • Tempo, Jaeger, Dynatrace and Datadog receive the selected time window. Unsupported SQL range shapes are rejected.
  • New jaeger_vpc_enabled / dynatrace_vpc_enabled (default false, require agentcore_enabled && integrations_enabled). Mimir's Prometheus API prefix is configurable. Shared default datasources are labelled as shared in the API, UI and docs.
  • Custom MCP connections (agent/pinned_http.py) pin the validated address while preserving Host/SNI and credentials, and reject every redirect. httpx/httpcore are pinned to the versions the SNI extension was tested against.
  • New docs/reference/external-connections.md (indexed in the reference README). The docs-site datasource guides (ko/en/ja/zh) and the existing CHANGELOG entries are amended in place.

Release order

The Lambda producers and the helper must ship before the Web/UI that reads their completion metadata. The AgentCore image with the pinned transport is a separate Deploy AgentCore. The VPC flags stay false until an operator reviews a saved plan. This PR performs no deployment or flag activation.

Tests

  • bash scripts/v2/merge-verify.sh on Python 3.12 (rebased on dev baea6e9): web vitest PASS, Terraform fmt PASS, deployment Node tests PASS. In the Python stage, the only failures are test_review_codec_sandbox.py / test_pr_review_head_images.py, which need a prepared AWSOPS_REVIEW_CODEC_STATE that this host doesn't have. The same failures occur on the dev baseline. agent/test_pinned_http.py needs Python ≥ 3.11 (asyncio.timeout) and passes 13/13 on 3.12.
  • terraform test -filter=tests/runtime_iam.tftest.hcl (mock provider) → 9 passed.

🤖 Generated with Claude Code

Atom-oh and others added 2 commits October 18, 2026 06:22
Datasource adapters cap success and error bodies before parsing,
fail closed on missing explicit instance IDs, probe the real bounded
query operation, and report partial or unverified results instead of
claiming absence. Prometheus/Mimir enforce the saved execution ceiling
inside the connector; other kinds apply HTTP response limits, and the
UI shows the effective limit. Jaeger limits are clamped with
requested/returned counts. Optional Jaeger/Dynatrace VPC attachment
and a configurable Mimir API prefix stay default-off. Custom MCP
connections pin the validated address while preserving Host/SNI, and
reject every redirect.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread agent/test_pinned_http.py
context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
context.load_cert_chain(str(certificate), str(key))
context.set_servername_callback(lambda sock, name, ctx: seen.update(sni=name))
server.socket = context.wrap_socket(server.socket, server_side=True)
@github-actions

Copy link
Copy Markdown

🤖 AI Code Review (two independent reviewers)

Reviewer responses: none

Status: BLOCKED — input coverage incomplete; models were not called

Review input incomplete: diff exceeds 128 KiB; split the change or implement complete bounded review batches. No models were called; no code verdict is available.


Triggered by commit 78588876e116c27fb1a8c76cbd0e84aea3fbae81 · workflow: .github/workflows/pr-review.yml

@Atom-oh

Atom-oh commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Split into three PRs that each fit the AI review input limit (this one was 229 KiB, over the 128 KiB cap). Merge in this order: #183 → #184 → #185. The CodeQL py/insecure-protocol alert is fixed in 184.

@Atom-oh Atom-oh closed this Oct 18, 2026

This branch had an error being deployed

1 failed deployment
ci-review-auto — 78588876 Deployed Oct 18, 2026 by Atom-oh via AI Code Review #706
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants