NIFI-10675 Correct Neo4J 4.X+ SSL support

[MikeThomsen]

Summary

NIFI-00000

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using mvn clean install -P contrib-check
  • JDK 8
  • JDK 11
  • JDK 17

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[exceptionfactory]

Thanks for addressing the issues with TLS support in Neo4j @MikeThomsen, the addition of test containers for integration is also useful.

At a high level, there is a concern with introducing static certificates to version control. These certificates will eventually expire, resulting in test failures. One option is to generate the certificates programmatically in the integration test, which could be done through a combination of the TemporaryKeyStoreBuilder and then writing out the generated values using the Bouncy Castle PEMWriter. On the other hand, since integration tests are not part of automated execution, I'm wondering whether it makes sense to introduce an integration test for TLS.

[MikeThomsen]

@exceptionfactory we can take that on as a follow on ticket, but FYI the reason Testcontainers is not used in the SSL integration test is that it appears that the Neo4J Testcontainers module doesn't include support for SSL in its API for setting the neo4j user password. Could have just been me TBH, but the container hung doing that for me when I configured it to use all of the appropriate env and volume configurations.

[exceptionfactory]

Thanks for the additional details @MikeThomsen, that is helpful.

[exceptionfactory]

@MikeThomsen Reviewing these changes in a little more detail, I think it looks good aside from the embedded certificate files.

If it would be helpful, I should be able to implement automatic certificate generation in the integration test, which would avoid the need for checking the certificates into the repository. Let me know, and I could look at pushing an update to this branch.

[exceptionfactory]

This property could make use of the newer Resource Reference feature, which would identify the trusted certificates to be specified a File path.

[MikeThomsen]

@exceptionfactory I wasn't able to get Testcontainers to work with SSL, so you'd have to work around that in your proposed workflow.

[exceptionfactory]

Thanks @MikeThomsen, I will follow up with what I can find in the course of testing.

[exceptionfactory]

@MikeThomsen I refactored the SSL integration test to use Testcontainers. The test leverages the TemporaryKeyStoreBuilder to generate a key pair and certificate, and then encodes the private key and certificate for use with the container. The initialization method sets the required environment variables to enable TLS and then starts the container. This refactoring allowed for the removal of the certificate and key files from the repository.

I pushed the commit to the pull request branch, so let me know what you think and I can proceed to merge if it looks good.

[MikeThomsen]

@exceptionfactory Verified your changes run to completion. If you're otherwise +1 on merge, we can close this out.

[exceptionfactory]

Thanks for verifying the integration test updates @MikeThomsen! +1 merging