RobertIndie commented on code in PR #24572: URL: https://github.com/apache/pulsar/pull/24572#discussion_r2268369469
########## 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) { Review Comment: The default value for `CryptoFailureAction` is `FAIL`. This means that users should explicitly set `CryptoFailureAction` to `null` if they are using `DecryptFailListener`. Would it be a good idea to automatically disable the `CryptoFailureAction` feature when `DecryptFailListener` is set? ########## 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) { Review Comment: Why do we need this check? ########## 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: We already skip the `ConsumerCryptoFailureAction` behavior here. So seems like we don't need the check `if (conf.getDecryptFailListener() != null && conf.getCryptoFailureAction() != null)` ########## 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 + - Else: + - Follows configured `ConsumerCryptoFailureAction` + + +## Public-facing Changes +### Public API +#### New interfaces: +```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); +} +``` +#### New ConsumerBuilder methods: +```java +ConsumerBuilder<T> decryptFailListener(DecryptFailListener<T> decryptFailListener); +``` +#### New ReaderBuilder methods: +```java +ReaderBuilder<T> readerDecryptFailListener(ReaderDecryptFailListener<T> readerDecryptFailListener); Review Comment: ```suggestion ReaderBuilder<T> decryptFailListener(ReaderDecryptFailListener<T> readerDecryptFailListener); ``` Why don't we use the same config name with the consumer? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
