[jiangpengcheng]

Fixes #24782

Motivation

The python function will try to load crypto_config from crypto_spec.cryptoKeyReaderConfig even it's empty or None, which will show an un-expected error log Failed to load the crypto key reader from spe

Modifications

check the crypto_spec.cryptoKeyReaderConfig before load it

Verifying this change

• Make sure that the change passes the CI checks.

• This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: jiangpengcheng#48

[merlimat]

Hi @jiangpengcheng, thanks for this fix. It's still needed on master: an unset cryptoSpec comes back as an empty default message, not None, so every Python function without crypto calls json.loads("") once for its producer and once per input topic, and logs Failed to load the crypto key reader from spec.

I pushed two commits to your branch to get it ready to merge:

1. Merged master (032ba0c). The only conflict was the blank line before get_crypto_reader, where master has since added new methods. I kept master's side of the file, so this merge drops your original get_crypto_reader change. The next commit replaces it.
2. Narrowed the check (d774784). Instead of building the default reader only when both public_key_path and private_key_path are present, get_crypto_reader now returns None early only when the spec has neither cryptoKeyReaderClassName nor cryptoKeyReaderConfig. Otherwise, a partial config (for example, only a public key) would quietly turn encryption off instead of logging an error. A custom reader class with no config is now created with no arguments instead of failing to parse "". I also added unit tests in test_python_instance.py (TestGetCryptoReader) for an unset spec on the producer and on an input, for a partial config, and for a custom class with no config. Your integration-test check in PulsarFunctionsTest is unchanged.

Let me know if that works for you.