Skip to content

fix: report TLS trust source in apm doctor - #2602

Open
Aryan Singh K. (aryansk) wants to merge 1 commit into
microsoft:mainfrom
aryansk:fix/2035-doctor-tls-diagnostic
Open

fix: report TLS trust source in apm doctor#2602
Aryan Singh K. (aryansk) wants to merge 1 commit into
microsoft:mainfrom
aryansk:fix/2035-doctor-tls-diagnostic

Conversation

@aryansk

Copy link
Copy Markdown

Closes #2035

Summary

  • report the active TLS trust source and precedence in apm doctor
  • isolate process-global SSL/truststore state between tests with a shared autouse fixture
  • add regression coverage for the doctor TLS row

Validation

  • patch syntax and application context were checked against current main
  • repository tests could not be executed in this chat environment; CI should run the project test suite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new “TLS trust” informational check to apm marketplace doctor and stabilizes test isolation around process-global truststore injection.

Changes:

  • Introduced describe_tls_trust() to generate user-facing TLS trust-source and precedence text.
  • Added a new “TLS trust” informational doctor check and a corresponding unit test assertion.
  • Added an autouse pytest fixture to reset truststore/SSL-related global state between tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
tests/unit/commands/test_marketplace_doctor.py Adds a unit test ensuring the doctor output includes the TLS trust section/source.
tests/conftest.py Adds an autouse fixture to reset truststore injection and clear TLS trust configuration cache between tests.
src/apm_cli/core/tls_trust.py Adds describe_tls_trust() to report TLS trust source and precedence.
src/apm_cli/commands/marketplace/doctor.py Adds a new informational “TLS trust” check to doctor output.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +98 to +103
def describe_tls_trust(env: Mapping[str, str] | None = None) -> tuple[str, str]:
"""Return a user-facing trust source and precedence description."""
if _env_flag(_DISABLE_ENV_VAR, env):
source = f"bundled CA (certifi); {_DISABLE_ENV_VAR}=1 disables OS trust-store injection"
elif has_explicit_ca_override(env):
source = f"explicit CA bundle: {_explicit_ca_path(env)}"
Comment on lines +111 to +114
precedence = (
"APM_DISABLE_TRUSTSTORE > REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE > "
"APM_EXTRA_CA_BUNDLE (when supported) > OS trust store > certifi fallback"
)
)
)

# Check 4: TLS trust source (informational)
)
)

# Check 4: marketplace config presence + parsability
Comment on lines +204 to +205
except Exception as exc:
tls_detail = f"Unable to determine TLS trust source: {str(exc)[:60]}"


def describe_tls_trust(env: Mapping[str, str] | None = None) -> tuple[str, str]:
"""Return a user-facing trust source and precedence description."""
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.

Add apm doctor tls trust-source diagnostic + test-suite ssl-isolation fixture

2 participants