Closed jordanlaforge closed 5 years ago
Cool thanks for the feedback.
I removed my changes from the Connect keyword, forgot I left them in.
I added a Set TLS and Set Username and Password keyword that modify a given MQTT client and return it which can be called with SSL Connect later.
I also modified the Set TLS to allow for full customization like tls_set.
I am still working on getting it to build, I thought it would be able to find the .crt file but I guess it builds from a different directory. I will try to link to it remotely.
I added some comments on the individual changes that need to be made. As I mentioned in those, please make sure to add tests to publish/subscribe with the secure connect functionality that you're adding.
Also, please remove all trailing whitespace. If you do a git diff master
(assuming you branched off of master to make your changes), it should show you all the trailing whitespace.
Interesting, running the tests locally works for me. The only issues I had were pub sub which has
=============================================================================
Pubsub
=============================================================================
Publish a non-empty message | PASS
-----------------------------------------------------------------------------
Publish an empty message | PASS
-----------------------------------------------------------------------------
Publish a message with QOS 1 and validate that the message is rece... | PASS
-----------------------------------------------------------------------------
Publish multiple messages and confirm that validation succeeds onl... | FAIL
The expected payload didn't arrive in the topic
-----------------------------------------------------------------------------
Publish an empty message with QOS 1 and validate | PASS
-----------------------------------------------------------------------------
Publish and validate with regular expression | PASS
-----------------------------------------------------------------------------
Subscribe for the first time and validate that no messages are rec... | PASS
-----------------------------------------------------------------------------
Subscribe, publish 1 message and validate it is received | PASS
-----------------------------------------------------------------------------
Subscribe with no limit, publish multiple messages and validate th... | FAIL
Length of '['test message3']' should be 3 but is 1.
-----------------------------------------------------------------------------
Subscribe with limit | FAIL
test message3 != test message1
-----------------------------------------------------------------------------
Unsubscribe and validate no messages are received | PASS
-----------------------------------------------------------------------------
Pubsub | FAIL
11 critical tests, 8 passed, 3 failed
11 tests total, 8 passed, 3 failed
which looked to be an issue with the server. I will keep looking into it
@laforge38 Thanks for fixing the tests. I have added some more feedback to the changes. And in addition, 2 more comments:
I am having trouble getting the subpub tests to pass with a SSL connection to mosquitto. I will update what I have with the other changes and keep looking into the subpub tests. Let me know if you know what is wrong, thanks!
Before I could get to the error in sslpubsub tests, I noticed something in the sslconnect tests. You are using the Publish Single
keyword, but that keyword doesn't make use of the global _mqttc
client. So essentially the Set TLS
keyword doesn't change the behavior of this function and all arguments have to be provided as part of this keyword itself. See: https://www.eclipse.org/paho/clients/python/docs/#single for more info on the underlying function that keyword uses.
So you would use that keyword with TLS like this:
| Publish a single message and disconnect
| | ${tls} | Create Dictionary | ca_certs | ${certFile}
| | ... | tls_version | ${3}
| | ${time} | Get Time | epoch
| | ${message} | Catenate | test message | ${time}
| | Publish Single | topic=${topic} | payload=${message}
| | ... | hostname=${broker.uri} | port=${8883} | tls=${tls}
Notice the _tlsversion that has to be set. This maybe a bug with the paho library that if tls_version is not set, it sets it to None
: http://git.eclipse.org/c/paho/org.eclipse.paho.mqtt.python.git/tree/src/paho/mqtt/publish.py#n159. I'll investigate more on this.
Meanwhile, can you update the 2 publish tests in sslconnect? You can also move them to the 'publish' test file, as they test the same keywords/functionality.
For the failed tests in sslpubsub, I noticed that the messages are received in publish step and if you subscribe without tls options on port 1883, it works. Hopefully that helps in further debugging.
Any plans to sort the issue out ? Pub / Sub using TLS 1.2 on 8883 port is failing
Is this library being supported/ fixed by the developer? Will it be supported by python 3.x ?
This library is supported on python 3.x thanks to contribution in https://github.com/randomsync/robotframework-mqttlibrary/pull/13. This particular issue however hasn't received any updates, but TLS support is available using publish_single
keyword: https://github.com/randomsync/robotframework-mqttlibrary/blob/0450e4c75a5c036e7831a8a50f692e17fdff89a2/src/MQTTLibrary/MQTTKeywords.py#L325
Added a new SSL connection keyword that requires a username, password and cert file