Skip to content

SNOW-3194269: snowCD migration: snow connection test --enable-diag per-endpoint diagnostics#3087

Draft
sfc-gh-jbilek wants to merge 5 commits into
mainfrom
snow-3194269-snowcd-migration
Draft

SNOW-3194269: snowCD migration: snow connection test --enable-diag per-endpoint diagnostics#3087
sfc-gh-jbilek wants to merge 5 commits into
mainfrom
snow-3194269-snowcd-migration

Conversation

@sfc-gh-jbilek

Copy link
Copy Markdown
Collaborator

Summary

Brings SnowCD-style connectivity diagnostics into snow connection test --enable-diag. The standalone SnowCD tool is being marked unsupported (snowflake-prod-docs#22135, June 2026) and Snowflake CLI is the documented replacement; today the --enable-diag flag only delegates to snowflake-connector-python's ConnectionDiagnostic, which writes a single text report at /tmp/SnowflakeConnectionTestReport.txt covering ~6 endpoint types. This PR fills the gap.

Linked: SNOW-3194269 (parent epic SNOW-3194158).

What changes

  • New module src/snowflake/cli/_plugins/connection/diagnostic.py — pure Python, no new deps. Public surface: EndpointCheck, DiagnosticReport, NetworkPolicySnapshot, NetworkRule, load_allowlist, check_endpoint, run_diagnostic, collect_network_policy, status_line.
  • Wired into connection test (src/snowflake/cli/_plugins/connection/commands.py) inside the existing if conn_ctx.enable_diag: branch. Additive only — the connector's enable_diag=True and the legacy text report still run as a backstop.
  • SYSTEM$ALLOWLIST() is the primary endpoint source; SYSTEM$ALLOWLIST_PRIVATELINK() is merged in (deduped by (type, host, port)) when available. Falls back to (conn.host, 443) if the public allowlist call fails (low-priv role).
  • Per endpoint: DNS + TCP, plus TLS handshake on port 443 with cert issuer + notAfter extracted; latency measured with time.perf_counter. Wildcards (*.x...) and empty hosts are marked Skipped. Custom CA roots honored via REQUESTS_CA_BUNDLE / SSL_CERT_FILE env vars.
  • Network policy snapshot: CURRENT_IP_ADDRESS() for the live IP, SHOW PARAMETERS LIKE 'NETWORK_POLICY' at account + user scope (user-level wins), DESC NETWORK POLICY for allow/block lists, DESC NETWORK RULE per referenced rule. Privilege errors surface as a Note: row instead of silently showing empty lists.

Backward compatibility

  • --enable-diag and --diag-allowlist-path flags are unchanged.
  • The connector's SnowflakeConnectionTestReport.txt is still produced; the Diag Report Location line in the connection summary is preserved.
  • Existing tests/test_connection.py::test_connection_test_diag_report still passes unchanged.

Tests

  • 29 new unit tests in tests/test_connection_diagnostic.py covering: wildcard skip, DNS / TCP / TLS failures, port-80 TCP-only, latency capture, allowlist file vs. query branches, malformed-entry filtering, fallback when SYSTEM$ALLOWLIST() denies, PrivateLink merge + dedupe + silent failure, CA bundle resolution priority, network-policy snapshot.
  • All 96 existing tests/test_connection.py tests continue to pass.
  • Live-tested against a real Snowflake account (TABLE format, JSON format, --diag-allowlist-path with a fixture file, REQUESTS_CA_BUNDLE env var).

Test plan for reviewers

  • snow connection test -c <conn> --enable-diag — expect streaming Checking ... lines, per-endpoint table, network policy block, summary line.
  • Same with --format json | jq . — confirm Diagnostic key is present and well-formed.
  • --diag-allowlist-path with a JSON fixture — confirm SYSTEM$ALLOWLIST() is not called.
  • Low-priv role — confirm fallback to (conn.host, 443) and Note: row when DESC NETWORK POLICY is denied.
  • REQUESTS_CA_BUNDLE=/path/to/ca.pem — confirm the diagnostic TLS probe uses that bundle.

Out of scope

  • Round-trip latency to Snowflake API endpoints beyond raw TCP connect.
  • A dedicated --cacert flag (env vars cover the use case for v1).

Comment thread src/snowflake/cli/_plugins/connection/diagnostic.py Fixed
Comment thread tests/test_connection_diagnostic.py Dismissed
@sfc-gh-jbilek sfc-gh-jbilek changed the title SNOW-3194269: snowCD migration — snow connection test --enable-diag per-endpoint diagnostics SNOW-3194269: snowCD migration: snow connection test --enable-diag per-endpoint diagnostics Jun 5, 2026
@sfc-gh-jbilek sfc-gh-jbilek force-pushed the snow-3194269-snowcd-migration branch from 2ac7881 to b04eae5 Compare June 5, 2026 08:30
`snow connection test --enable-diag` now runs SnowCD-style live
connectivity checks instead of just printing the path to the
connector's text report. SnowCD itself is being marked unsupported
(snowflake-prod-docs#22135) and Snowflake CLI is the documented
replacement.

What it does:

  - Fetches `SYSTEM$ALLOWLIST()` and merges `SYSTEM$ALLOWLIST_PRIVATELINK()`
    on top, deduped by (type, host, port). Falls back to (conn.host, 443)
    if the public allowlist call fails (low-priv role).
  - Probes every resolvable endpoint: DNS + TCP, plus a TLS handshake
    on port 443 with cert issuer + notAfter extracted. Latency measured
    with time.perf_counter. Wildcards and empty hosts marked Skipped.
  - Honours REQUESTS_CA_BUNDLE and SSL_CERT_FILE env vars for custom
    CA roots, matching what snowflake-connector-python reads.
  - Streams `Checking <TYPE>: <host> ✅ (12.3 ms)` per endpoint, then
    prints a per-endpoint table (url | type | status | latency_ms |
    issuer | cert_expires) and a `Results: X Healthy, Y Unhealthy out
    of Z. N skipped (non-resolvable patterns)` summary line.
  - Network-policy snapshot: CURRENT_IP_ADDRESS() for the live IP,
    SHOW PARAMETERS LIKE 'NETWORK_POLICY' at account + user scope
    (user-level wins), DESC NETWORK POLICY for allow/block lists,
    DESC NETWORK RULE per referenced rule (mode/type/values).
    Privilege errors surface as a `Note:` row instead of silently
    showing empty lists.
  - --format json carries the same data structurally under
    Diagnostic.{checks, healthy, unhealthy, skipped, tested,
    network_policy}.

Backward compatible: the connector's enable_diag=True still runs and
the legacy SnowflakeConnectionTestReport.txt is still produced; the
existing `Diag Report Location` line in the connection summary is
preserved. Existing test_connection_test_diag_report passes unchanged.

Tests: 29 new in tests/test_connection_diagnostic.py covering wildcard
skip, DNS / TCP / TLS failures, port-80 TCP-only, latency capture,
allowlist file vs query branches, malformed-entry filtering, fallback
on permission errors, PrivateLink merge + dedupe + silent failure,
CA bundle resolution priority, and the network-policy snapshot
(user > account precedence, DESC failure mode, query-error survival).
All 96 existing tests/test_connection.py tests continue to pass.
Live-tested against a real account (TABLE format, JSON format,
--diag-allowlist-path, REQUESTS_CA_BUNDLE pickup).
@sfc-gh-jbilek sfc-gh-jbilek force-pushed the snow-3194269-snowcd-migration branch from b04eae5 to 34fe5a0 Compare June 5, 2026 08:34
Apply A Philosophy of Software Design simplifications to the SnowCD
migration code:

diagnostic.py
- Drop pass-through to_dict() on EndpointCheck and NetworkPolicySnapshot;
  use dataclasses.asdict() at call sites.
- Rename NetworkPolicySnapshot.{allowed,blocked}_rule_list to
  {allowed,blocked}_network_rule_list to match the JSON contract directly,
  eliminating the rename layer.
- Unify _query_allowlist (returned []) and _safe_query (returned None)
  into _query_json_list and _query_rows with one consistent return shape.
- Replace exception-based fallback in run_diagnostic with empty-list
  fallback: load_allowlist always returns a list; run_diagnostic falls
  back to (conn.host, 443) when empty + no allowlist file.
- Drop the side-effect-and-return pattern in _describe_network_policy;
  return parsed dict, apply at call site.
- Flatten triple-nested issuer extraction into _issuer_org_name helper.
- Normalise DESC row keys once at the top of _query_rows instead of
  case-insensitive lookups inline at every site.
- Tighten is_resolvable to one expression.

commands.py
- Split _connection_test_with_diag into orchestrator + pure
  _diagnostic_table_results presentation function.
- Replace _stream closure with a one-line conditional lambda.
- Use asdict() for the JSON payload; drop the diagnostic_payload temp.

All 29 new + 96 existing connection tests still pass.
- Route DESC NETWORK POLICY/RULE and SHOW PARAMETERS FOR USER through
  FQN.sql_identifier and identifier_to_show_like_pattern; add _safe_fqn()
  so a malformed server-supplied identifier no longer crashes the run.
- Swap ClickException for CliError per the migration doc; drop the
  now-unused click import.
- Read --diag-allowlist-path through SecurePath with an explicit
  file_size_limit_mb so the read is logged and bounded.
- Sanitize every server-derived string (host, IP, policy/rule names,
  allow/block lists, cert metadata) before display via
  sanitize_for_terminal; add _safe() helper in commands.py.
- Lift the inline OutputFormat / MultipleResults imports to module top.
- Tests: expect CliError on bad JSON, assert IDENTIFIER('...') wrapping
  on the user/policy/rule SQL, and add an ANSI-stripping check on
  status_line for a server-controlled host.
Comment thread tests/test_connection_diagnostic.py Dismissed
`--enable-diag` triggers ConnectionDiagnostic inside snowflake-
connector-python during connect() — a silent multi-second probe of
every endpoint that ran before our own streaming output started. Print
"Running connector connectivity probe (this may take a moment)..."
ahead of the connection so the wait is explained. TABLE output only;
JSON/CSV stay clean.
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