Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIFI-614 Added initial support for new style JMS #222

Closed
wants to merge 1 commit into from

Conversation

olegz
Copy link
Contributor

@olegz olegz commented Feb 14, 2016

NIFI-614 finalized JMSConnectionFactoryProvider ControllerService

NIFI-614 finalized implementation of both Processors and ControllerService

NIFI-614 added initial documentation

NIFI-614 polishing

@olegz olegz force-pushed the NIFI-614 branch 2 times, most recently from 913a744 to 8d95aaa Compare February 17, 2016 15:37
@@ -0,0 +1 @@
/lib/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't ;)

@JPercivall
Copy link
Contributor

Reviewing

@JPercivall
Copy link
Contributor

Following the pattern for Controller Services and their API in nifi-standard-services, JMSConnectionFactoryProviderDefinition should be in it's own package.

@JPercivall
Copy link
Contributor

New commits fail contrib-check for these reasons:

[INFO] --- maven-checkstyle-plugin:2.15:check (check-style) @ nifi-jms-processors ---
[WARNING] src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java63 NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java71 NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/jms/processors/JMSPublisher.java72 NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/jms/processors/MessageBodyToBytesConverter.java35 NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/jms/processors/MessageBodyToBytesConverter.java36 NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/jms/processors/MessageBodyToBytesConverter.java48 NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/jms/processors/MessageBodyToBytesConverter.java49 NonEmptyAtclauseDescription: At-clause should have a non-empty description.

@JPercivall
Copy link
Contributor

What was the motivation for creating an additional details page for the Publish/Consume processors that is just a summary and an overview of the configuration? Both of those could be conveyed using the CapabilityDescription tag and the property descriptions. It also makes it harder to maintain.

@olegz
Copy link
Contributor Author

olegz commented Mar 11, 2016

We'll be adding more docs in there in the future related to specific providers (e.g., IBM, Tibco etc), so treat it as place holder

@JPercivall
Copy link
Contributor

Why are the docs needed as place holders? It is not a breaking change to adding additional documentation pages in the future. Details regarding simple capability description and processor property descriptors should follow the same convention other components follow.

@JPercivall
Copy link
Contributor

Build is still failing contrib check after the latest patches due to:

[INFO] --- maven-checkstyle-plugin:2.15:check (check-style) @ nifi-jms-cf-service ---
[WARNING] src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java58 RegexpSinglelineJava: Line has trailing whitespace.

@olegz olegz force-pushed the NIFI-614 branch 3 times, most recently from b125cd7 to 3aa79c2 Compare March 13, 2016 15:20
@digitalplummer
Copy link

Tested on IBM MQ 7.5 and greater
WebSphere
Tibco

@mosermw
Copy link
Member

mosermw commented Mar 14, 2016

Tested using Tibco with SSL, didn't work.

ERROR o.a.n.jms.processors.PublishJMS org.springframework.jms.JmsSecurityException: Can not initialize SSL client: no trusted certificates are set; nested exception is javax.jms.JMSSecurityException: Can not initialize SSL client: no trusted certificates are set
at org.springframework.jms.support.JmsUtils.convertJmsAccessException(JmsUtils.java:291)
at org.springframework.jms.support.JmsAccessor.convertJmsAccessException(JmsAccessor.java:169)
at org.springframework.jms.core.JmsTemplate.execute(JmsTemplate.java:497)
at org.springframework.jms.core.JmsTemplate.send(JmsTemplate.java:580)
...
at org.apache.nifi.jms.processors.JMSPublisher.publish(JMSPublisher.java:75)

Caused by: javax.jms.JMSSecurityException: Can not initialize SSL client: no trusted certificates are set
at com.tibco.tibjms.TibjmsLinkSSL._initSSL
at com.tibco.tibjms.TibjmsLinkSSL.connect
at com.tibco.tibjms.TibjmsConnection._create
at com.tibco.tibjms.TibjmsConnection.
at com.tibco.tibjms.TibjmsxCFImpl._createImpl
at com.tibco.tibjms.TibjmsxCFImpl._createConnection
at com.tibco.tibjms.TibjmsConnectionFactory.createConnection
at org.springframework.jms.connection.UserCredentialsConnectionFactoryAdapter.doCreateConnection
...
at org.springframework.jms.support.JmsAccessor.createConnection
at org.springframework.jms.core.JmsTemplate.execute

I'm not sure how to pass the keystores from the SSL Context Service property of JMSConnectionFactoryProvider into the instance of the MQ ConnectionFactory Implementation using the configuration options provided.

@JPercivall
Copy link
Contributor

@mosermw shouldn't you just be able to create a typical SSL controller service and set the "SSL Context Service" property to point to it? This should automatically use the properties set in the SSL controller service.

@mosermw
Copy link
Member

mosermw commented Mar 14, 2016

Yep, that's what I did. The TibjmsConnectionFactory must not see the truststore I set in the SSL Context Service. I'm worried that different JMS providers require different SSL setup.

@olegz
Copy link
Contributor Author

olegz commented Mar 14, 2016

I'll look at it. I think I know what's going on.

@olegz
Copy link
Contributor Author

olegz commented Mar 15, 2016

So the problem with Tibco and SSL is that Tibco does things differently then other providers. In other words they don't use SSLContext and instead rely on setting individual properties inside their implementation of ConnectionFactory. The good thing is we can extract them from SSLContext, but need ti know exactly what we need. That said, propose to release this new JMS support with understanding that it does not yet support Tibco SSL and add it later as part of the enhancement.

@joewitt
Copy link
Contributor

joewitt commented Mar 15, 2016

i am on the same page of this being a good start oleg provided the JIRA is created to enable ssl context to provide info for providers that want it. We don't have support for tibco ssl in apache now anyway.

@olegz
Copy link
Contributor Author

olegz commented Mar 15, 2016

Agreed and the JIRA is https://issues.apache.org/jira/browse/NIFI-1628

@JPercivall
Copy link
Contributor

I am a +1 on merging this PR in (once commits have been squashed).

I reviewed the code, did a contrib check build and did basic testing with ActiveMQ. With @digitalplummer having done more extensive testing with other JMS brokers, I am satisfied with the PR.

NIFI-614 finalized JMSConnectionFactoryProvider ControllerService

NIFI-614 finalized implementation of both Processors and ControllerService

NIFI-614 added initial documentation

NIFI-614 addressed PR comment with unused import and squashed

NIFI-614 added @OnDisabled method

NIFI-614 changed POMs to 0.6

NIFI-614 removed local .gitignore

NIFI-614 added support for parsing Tibco URL

NIFI-614 removed setting of jms message id

NIFI-614 addressed PR comments, fixed tests

NIFI-614 addressed latest PR comments

NIFI-614 second round of PR comments addressed

NIFI-614 3rd round of PR comments addressed

NIFI-614 finalizing on PR comments

NIFI-614 more PR comments
@jugi92
Copy link
Contributor

jugi92 commented Dec 19, 2017

Was the Tibco issue ever followed up on?
I tried to set sSLTrustedCertificate as dynamic property but it gives the following error:

2017-12-19 17:48:09,788 ERROR [StandardProcessScheduler Thread-7] o.a.n.c.s.StandardControllerServiceNode JMSConnectionFactoryProvider[id=a7cf3b99-c426-18a4-0000-00003dc28240] Failed to invoke @OnEnabled method due to java.lang.IllegalStateException: java.lang.IllegalStateException: Failed to set property sSLTrustedCertificate: {}
java.lang.IllegalStateException: java.lang.IllegalStateException: Failed to set property sSLTrustedCertificate

Tibco Java Doc is available here:
https://docs.tibco.com/pub/enterprise_message_service/6.3.0-february-2012/doc/html/tib_ems_api_reference/api/javadoc/com/tibco/tibjms/TibjmsConnectionFactory.html#setSSLTrustedCertificate(java.lang.String)

@mosermw
Copy link
Member

mosermw commented Dec 19, 2017

Since TIBCO libraries don't fall under a license that is compatible with the Apache License 2.0, Apache NiFi cannot distribute a solution that is specific to TIBCO. But you should be able to build an extension to Apache NiFi that works for you. Here is an example for how you might approach building a NiFi extension for this mosermw/nifi-jms-providers-bundle.

@jugi92
Copy link
Contributor

jugi92 commented Dec 22, 2017

I understand that Apache Nifi can not deliver custom solutions for individual proprietary JMS providers. But as I understood, the dynamic property was build exactly for the purpose to set custom attributes of the Connection Factory as mentioned in the Documentation:
JMSConnectionFactoryProvider

So the question is if this is working for other providers and the implementation on Tibco Site does not allow the dynamic setting of attributes or is it a problem in Nifi that it does not call the set Method of the attribute correctly?

@joewitt
Copy link
Contributor

joewitt commented Dec 22, 2017

@jugi92 you're right that the NIFi support for dynamic/custom attributes to help with Vendor differences. However, even in that case it appears (though I've not personally verified) that this isn't enough in the Tibco case for getting TLS setup propertly for them. For the NiFi method to work the custom attributes need to correspond to bean properties/setters. If their approach requires more awareness then it will require a specific component for them. I am aware of plenty of NiFi/Tibco integrations but they're outside of Apache at this point given what Mike highlighted above.

If you are able to investigate the Tibco TLS setup requirements and see what the gap is perhaps it can be done in a vendor neutral way

@jugi92
Copy link
Contributor

jugi92 commented Dec 22, 2017

I found out that it works if the SSL Context Service is configured correctly and the dynamic property is provided as shown below:

Property Value
SSL Context Service SSLContextServiceForJMS
SSLTrustedCertificate /path/to/TrustedCertificate.cer

So the dynamic attributes are a really valuable thing.
Thanks

@ragnarotech
Copy link

@jugi92 this solution does not work with tibjms.jar v8.x In v8 tibco adds "void setSSLTrustedCertificate(byte[] certificate, java.lang.String trustedEncoding)" to it's TibjmsConnectionFactory class, which is what method the jms util finds first causing a too few arguments exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants