RobertIndie commented on code in PR #24572: URL: https://github.com/apache/pulsar/pull/24572#discussion_r2281296137
########## pip/pip-436.md: ########## @@ -0,0 +1,145 @@ +# PIP-436: Add decryptFailListener to Consumer + +# Background knowledge +Apache Pulsar supports client-side encryption where messages can be encrypted by producers and decrypted by consumers. The current mechanism for handling decryption failures uses ConsumerCryptoFailureAction which provides three options: + +- FAIL: Fail message consumption (default) + +- DISCARD: Silently discard the message + +- CONSUME: Deliver the encrypted message to the application + +With the introduction of ```isEncrypted``` field in ```EncryptionContext``` ([PIP-432](https://github.com/apache/pulsar/pull/24481)), applications can now determine whether a message was successfully decrypted by checking this flag. However, this requires manual inspection of each message. +# Motivation +**This PIP builds upon and extends [PIP-432]((https://github.com/apache/pulsar/pull/24481)) by providing a more elegant way to handle decryption failures. Which was introduced in https://github.com/apache/pulsar/pull/24481#issuecomment-3048640812** + +The proposed decryptFailListener would: + +- Provide a cleaner, more elegant solution for handling decryption failures +- Allow more flexible handling of decrypted vs encrypted messages +- With the ```decryptionFailListener```, users can use listener instead of ```ConsumerCryptoFailureAction```. They can decide in the listener to discard, or decrypt, or anything they want +- Maintain backward compatibility with existing ConsumerCryptoFailureAction behavior + +# Goals + +## In Scope +- Add new DecryptFailListener and ReaderDecryptFailListener interfaces + +- Add corresponding builder methods to ConsumerBuilder and ReaderBuilder + +- Implement the listener callback mechanism in consumer implementations + +- Ensure proper interaction with existing ConsumerCryptoFailureAction settings + +- Maintain backward compatibility with existing applications + +- Enforce mutual exclusivity between decryptFailListener and ConsumerCryptoFailureAction + +## Out of Scope + +- Changes to encryption/decryption algorithms or protocols +- Modifications to the core encryption/decryption process +- Performance optimizations specific to the new listener +- Changes to producer-side encryption logic +# High Level Design + +The solution adds new listener interfaces specifically for decryption failures: +```java +public interface DecryptFailListener<T> { + void received(Consumer<T> consumer, Message<T> message); +} + +public interface ReaderDecryptFailListener<T> { + void received(Reader<T> reader, Message<T> message); +} +``` +### Configuration Validation: +1.Mutual Exclusivity Check: +```java +if (conf.getDecryptFailListener() != null && conf.getCryptoFailureAction() != null) { + throw new InvalidConfigurationException( + "decryptFailListener cannot be used with cryptoFailureAction - choose one approach"); +} +``` +2.Listener Dependency Check: +```java +if (conf.getDecryptFailListener() != null && conf.getMessageListener() == null) { + throw new InvalidConfigurationException( + "decryptFailListener requires messageListener to be configured"); +} +``` +3.Default Behavior: +```java +if (conf.getDecryptFailListener() == null && conf.getCryptoFailureAction() == null) { + conf.setCryptoFailureAction(ConsumerCryptoFailureAction.FAIL); +} +``` +### Runtime Behavior: +1. For successful decryption: + - Always routes to `messageListener` + +2. For failed decryption: + - If `decryptFailListener` is configured: + - Routes to `decryptFailListener` + - Skips `ConsumerCryptoFailureAction` behavior Review Comment: Why don't we just skip the checking for `conf.getCryptoFailureAction()` and document it? Otherwise, the user should explicitly set `CryptoFailureAction` to `null` if they want to use `DecryptFailListener`. Also, setting a null value to an enum config seems confusing to me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
