Commit 9cb3058
authored
fix: survive a broken pyOpenSSL install (FLYTE-SDK-7T) (#1488)
Sentry: [FLYTE-SDK-7T](https://unionai.sentry.io/issues/7693268977/) —
`AttributeError: module 'lib' has no attribute 'GEN_EMAIL'`, 2 events,
release 2.5.6.
## What happens
`flyte deploy` dies during client initialisation, before it does any
work:
```
flyte/cli/_deploy.py invoke
flyte/cli/_common.py initialize_config -> init
flyte/_initialize.py init -> _initialize_client
flyte/remote/_client/controlplane.py <module>
flyte/remote/_client/auth/_session.py <module> <- from OpenSSL import SSL, crypto
OpenSSL/SSL.py <module>
OpenSSL/crypto.py X509Extension
AttributeError: module 'lib' has no attribute 'GEN_EMAIL'
```
`_session.py` imported pyOpenSSL at module scope, and `controlplane.py`
imports `_session`, so importing pyOpenSSL was on the path of **every
command that builds a client**.
That import is unusually fragile. pyOpenSSL binds names out of the
`cryptography` C bindings *at class-body time* — `X509Extension` reads
`_lib.GEN_EMAIL` while the module is still executing — so an
incompatible pyOpenSSL/cryptography pair fails during **import**, with
an `AttributeError`, not the `ImportError` an optional-dependency guard
would normally catch. The reporting host is an Ubuntu box running out of
`/usr/local/lib/python3.10/dist-packages`, the classic shape for this: a
distro-packaged pyOpenSSL sitting alongside a newer pip-installed
`cryptography`.
Two things went wrong: the user's `flyte deploy` failed with a message
naming a module they never imported and giving no hint what to
reconcile, and the SDK reported its own crash to Sentry, where a broken
third-party install in someone's environment is not an SDK bug.
## The fix
pyOpenSSL is only *used* by `_bootstrap_ssl_from_server`, i.e. only when
`insecure_skip_verify` is set. Nothing else in the module touches it. So
the import failure is held at module scope rather than propagated, and
converted at the one call site that needs the library:
```python
_PYOPENSSL_IMPORT_ERROR: BaseException | None = None
try:
from OpenSSL import SSL, crypto
except (ImportError, AttributeError) as _e:
SSL = None
crypto = None
_PYOPENSSL_IMPORT_ERROR = _e
```
`_bootstrap_ssl_from_server` then raises an `InitializationError` naming
both packages and the command that reconciles them, chained to the
original `AttributeError`. `InitializationError` is already on
`_is_user_error`'s allow-list, so this class of report stops reaching
Sentry — same treatment `EndpointUnreachable` got a few lines below in
#1387.
`AttributeError` is deliberately in the `except` tuple alongside
`ImportError`; catching only `ImportError` would not have caught this
crash at all.
Note the module-level `SSL` / `crypto` names are kept rather than moved
into a function-local import, so the eight existing tests that
`patch(f"{_SESSION_MOD}.SSL")` keep working untouched.
## Verification
The three new tests execute the real module body into a fresh module
object with `import OpenSSL` raising the production `AttributeError`, so
they exercise the actual module-level guard without reloading — and
therefore without disturbing the live `_session` other modules imported
from.
On clean `origin/main` all three fail, reproducing the Sentry signature
exactly:
```
src/flyte/remote/_client/auth/_session.py:12: in <module>
from OpenSSL import SSL, crypto
E AttributeError: module 'lib' has no attribute 'GEN_EMAIL'
```
On this branch all three pass. They cover: the module imports despite
the broken install; the cold path converts it to a user-kind
`InitializationError` with the cause chained and both package names in
the message, raised *before* any socket connect is attempted; and
`_sentry.capture_exception` drops it.
`tests/flyte/remote` + `tests/flyte/test_sentry.py`: 657 passed. Full
`tests/flyte`: 3631 passed, 21 failed — the same 21 that fail on clean
`origin/main` in this environment (verified by running both and diffing;
no test fails only on this branch). ruff, mypy and `check-docstrings`
clean.
## Left alone deliberately
I enumerated every third-party package that importing `controlplane`
pulls in, to see whether 7T had siblings. pyOpenSSL is the only one that
is imported but not needed by the failing command — everything else on
that list (`pyqwest`, `obstore`, `pydantic_core`, `cryptography` itself)
is genuinely used, so guarding its import would only move the failure
rather than remove it.
Worth flagging separately, though: that same import pulls in **pandas
and pyarrow**, which `flyte deploy` has no use for. That is an
import-cost issue rather than a crash, with no Sentry evidence behind
it, so it is not touched here.
fixes FLYTE-SDK-7T
---------
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>1 parent 34f44f9 commit 9cb3058
2 files changed
Lines changed: 114 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | 12 | | |
14 | 13 | | |
15 | 14 | | |
| |||
21 | 20 | | |
22 | 21 | | |
23 | 22 | | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
24 | 40 | | |
25 | 41 | | |
26 | 42 | | |
| |||
88 | 104 | | |
89 | 105 | | |
90 | 106 | | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
91 | 120 | | |
92 | 121 | | |
93 | 122 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
409 | 409 | | |
410 | 410 | | |
411 | 411 | | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
412 | 496 | | |
413 | 497 | | |
414 | 498 | | |
| |||
0 commit comments