Issues/24 and other fixes

[jantman]

This rolls together a number of fixes I'm working on:

• fixes Add logging #20 by adding a bunch of logging
• fixes Caller id is not in the expected format #22 for me
• fixes TypeError: string indices must be integers, not str #24 by making sure that messages are properly base64 decoded when received
• fixes another issue by converting PEM server certificates to the required DER format
• fixes an issue where Config.getboolean doesn't recognize 'yes' as a True value
• if the Listener thread gets an exception decoding the message, catch and log it. Otherwise, the python process locks up and needs to be manually killed (issue with uncaught exceptions in threads)

With all of this work, the 'simple_action.py' example now works in my environment.

[jantman]

@rafaduran I was wondering if you could take a look at this please?

[rafaduran]

@jantman I'm sorry but I haven't got any notification on this and I've just saw it, I wouldn't had time to look at it anyway... However, next week I will be plenty of time, so I will look on this then; hopefully sooner if I can.

[jantman]

@rafaduran Ok thanks. Next week is fine... I'm pretty busy for the next week or so, and we're coming up on the US holiday season, so I probably won't be doing a lot of work with it until the beginning of the new year. I just wanted to bring your attention to it... I'm going to be resuming work with this in a few weeks.

[rafaduran]

After not working on this for a while I've got a few issues with me test environment I needed to fix before looking at this, so I couldn't start looking at this until now.

[rafaduran]

I'm still getting issues with integration tests, specially with SSL security provider; I couldn't find the problem yet...

[jantman]

@rafaduran are you referring to the Travis job (linked here) or something else? I might be able to spot it if I look through the output...

[rafaduran]

@jantman I'm having two different kind of issues:

• tavis-ci on my branch jantman-issues/24 (based on this pull request, with some fixes, mostly unit tests) seems to be failing in a non deterministic way
• Running integration tests in my development environment always get stuck at tests/integration/base.py::TestWithSSLMCo20x::test_ping_call

Latter issue I think may be related to the use threading.Condition in the listener, leading in some way to a deadlock. In ntteurope/kombu-stomp I've also had to solve a similar problem, using a generator in that case, so I considering changing how the listener works (I think the generator is a much better solution than threading.Condition). However, I'm just guessing... travis-ci will be fine (hopefully) when this is solved.

[rafaduran]

@jantman could you please check my branch jantman-issues/24 caab478? My development environment it's fine with it and travis-ci too (thought I still see some timeout errors).

[jantman]

@rafaduran this branch seems to work fine for me, except for the ruby sym ":"s - on my branch I see result dicts returned with keys "body", "hash", "senderagent", etc. but on yours I see ":body", ":hash", ":senderagent", etc. - that's a bit unclean, but it works, and I can work with it, so I'd say it's good for me.

I haven't been using the integration tests, as they don't seem to accurately reflect my environment; I've been running directly against my Puppet-based mco instance.

[rafaduran]

@jantman you see the keys with leading colon ( :body, :hash, ...) because of my last change. This is required because depending on Puppet and/or Ruby versions (not really sure which one), Ruby symbols can be serialized as ":symbol" or "!ruby/sym symbol" and since PyYAML treats the former as an string, the latter must be translated into string too for compatibility reasons.

I think this is ready now, so I will check do some final checks (test coverage, documentation,..) and I will merge and release as soon as I can.

[jantman]

@rafaduran ok, thanks. Personally I think it would be nice if pymco stripped that out before returning results to the user (i.e. both ":symbol" and "!ruby/sym symbol" are returned as "symbol") but I supposed that could get confusing for the user to debug...

[rafaduran]

@jantman I did one last change (hopefully) before merging and it would be great if you can re-test it, could you please pull from my branch and test it again? It's mostly minor changes but I'm also using Python ssl PEM to DER functionality; if it works fine, I'd rather use it, thought I have no integration tests for that yet.

[jantman]

@rafaduran Ok, I'm running that at 5078dd9 ...

I don't know why I didn't remember this before, but... I tried this before as well, and spent quite a while on it, before I decided to use PyCrypto. This does not work for me:

[ERROR listener.py:105 -           on_message() ] RSA key format is not supported
Traceback (most recent call last):
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/src/python-mcollective/pymco/listener.py", line 98, in on_message
    decoded = self.security.decode(body, b64=useb64)
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/src/python-mcollective/pymco/security/__init__.py", line 77, in decode
    return self.verify(deserialized)
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/src/python-mcollective/pymco/security/ssl.py", line 55, in verify
    verifier = PKCS1_v1_5.new(self.server_public_key)
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/src/python-mcollective/pymco/security/ssl.py", line 110, in server_public_key
    cache=self._server_public_key)
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/src/python-mcollective/pymco/security/ssl.py", line 102, in _load_rsa_key
    cache = self._server_public_key = utils.load_rsa_key(self.config[key])
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/src/python-mcollective/pymco/utils.py", line 70, in load_rsa_key
    k = RSA.importKey(content)
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/lib/python2.7/site-packages/Crypto/PublicKey/RSA.py", line 680, in importKey
    return self._importKeyDER(externKey)
  File "/home/jantman/tickets/PUPPET-193/botutladm1_venv/lib/python2.7/site-packages/Crypto/PublicKey/RSA.py", line 588, in _importKeyDER
    raise ValueError("RSA key format is not supported")
ValueError: RSA key format is not supported

I'd tried this exact same thing in jantman@40135e9 and found that, unfortunately, ssl.PEM_cert_to_DER_cert() doesn't support X.509 certificates, like the PEM files that a Puppet-based mco setup will use. Hence my use of PyCrypto to convert. For reference, see that commit and http://stackoverflow.com/a/12921889

There's also one other remaining issue from my point of view, the requirements - pycrypto and PyYAML aren't installed by pip, so scripts fail and then I have to install them manually. Ideally I think they should just be included in the requirements. I think I remember you having some concerns about that... maybe this is a good place to use setuptools extras? I'd be happy to cut another PR with an example of how to do that if you're not familiar with it.

[rafaduran]

@jantman ok, I will remove last commit and I will use your implementation for PEM to DER conversion. About pycrypto and PyYAML in requirements, pycrypto is already in ssl extras and I will add PyYAML. Thank you for your help, I really appreciate your time spent here!

[rafaduran]

This is now merged.