z-kovacs commented on PR #20542: URL: https://github.com/apache/pulsar/pull/20542#issuecomment-1637927685
@nodece had a suggestion offline how to refactor this: - Remove the `bc-common` library - Create a new `messagecrypto-bcfips` library what would use `messagecrypto-bc` as dependency and implement another `MessageCrypto` implementation purely using BC FIPS inside, in the dependency we would exclude the bc-non-fips libs (shared code would stay in `messagecrypto-bc`) - Clients have to explicitly modify their client code to use the `MessageCryptoBcFips` implementation my argument is this is suboptimal because: - people have to change their code if they want to use bc fips - it might not be possible (3rd party application), also it would complicate their client configuration - if you reference the module `messagecrypto-bc` as dependency from `messagecrypto-bcfips`, it might contain code what was compile-able in `messagecrypto-bc` but not runnable from `messagecrypto-bcfips` and thus would cause runtime issues, this is what happened with the bcfips iplementation (`ClassNotFoundException`) An another idea is of removing the `bc-common` library and moving the crypto api interface and the reflection-based loader into `pulsar-common` this way you don't create a new module (which has other implications), but still you have the isolation of the bcfips and non-fips related implementations, which is giving compile-time and runtime safety. Also this way this is entirely transparent for the application, if you upgrade the pulsar libraries, it will became FIPS capable - depending on how the library dependencies re set up - in-line with the current setup. Please comment if you have preference towards either of the ways, or you are just happy with the way it is currently. -- 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]
