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

gracefully handle when salt-minion cannot decrypt key #22578

Merged
merged 1 commit into from
Apr 20, 2015

Conversation

hvnsweeting
Copy link
Contributor

or salt-minion will die. Returns None to deligate
this job to compile_pillar function handle it,
as it is the only caller of this function for now.

@hvnsweeting
Copy link
Contributor Author

Actually I got this problem on 2014.1.10, but as the version
is not actively maintained, I port this fix to 2014.7.

Stacktrace:

TypeError: string indices must be integers, not str
Traceback (most recent call last):
  File "/usr/bin/salt-minion", line 14, in <module>
    salt_minion()
  File "/usr/lib/pymodules/python2.7/salt/scripts.py", line 35, in salt_minion
    minion.start()
  File "/usr/lib/pymodules/python2.7/salt/__init__.py", line 221, in start
    self.prepare()
  File "/usr/lib/pymodules/python2.7/salt/__init__.py", line 209, in prepare
    self.minion = salt.minion.Minion(self.config)
  File "/usr/lib/pymodules/python2.7/salt/minion.py", line 553, in __init__
    opts['environment'],
  File "/usr/lib/pymodules/python2.7/salt/pillar/__init__.py", line 73, in compile_pillar
    ret_pillar = self.sreq.crypted_transfer_decode_dictentry(load, dictkey='pillar', tries=3, timeout=7200)
  File "/usr/lib/pymodules/python2.7/salt/transport/__init__.py", line 61, in crypted_transfer_decode_dictentry
    aes = key.private_decrypt(ret['key'], 4)
TypeError: string indices must be integers, not str
  1. I didn't test on 2014.7 and with RAET as I don't use it. I just changed the code according to my little understanding of those modules.
  2. let's discuss to have a good MR here as I'm only sure for my only case (I tested with 2014.1.10).

@thatch45
Copy link
Contributor

Go Go Jenkins!

@thatch45
Copy link
Contributor

Hmm, I thought we fixed this further up the pipe in 2014.7.4 or 2015.2, I am re-running the tests as they seem to have died

@jfindlay
Copy link
Contributor

@hvnsweeting, is it possible for you to rebase your branch? If not it's fine, we'll figure it out. Doing this might resolve the testing issues. Something like:

git fetch upstream
git checkout 2014-7-fix-compile-pillar
git rebase upstream/2014.7
git push origin 2014-7-fix-compile-pillar

@hvnsweeting
Copy link
Contributor Author

I rebased but test seems failed/dead, is there a way to know the reason? I found nothing useful in https://jenkins.saltstack.com/job/salt-pr-dsl/4345/

@jfindlay
Copy link
Contributor

@rallytime may know better how to interpret what's going on here than I.

@rallytime
Copy link
Contributor

There is a stack trace in the test logs when the test daemons are trying to start. Because of this stacktrace, the tests don't run and the build times out.

04:12:09 Traceback (most recent call last):
04:12:09   File "/testing/tests/runtests.py", line 411, in <module>
04:12:09     main()
04:12:09   File "/testing/tests/runtests.py", line 396, in main
04:12:09     status = parser.run_integration_tests()
04:12:09   File "/testing/tests/runtests.py", line 316, in run_integration_tests
04:12:09     with TestDaemon(self):
04:12:09   File "/testing/tests/integration/__init__.py", line 173, in __enter__
04:12:09     self.start_zeromq_daemons()
04:12:09   File "/testing/tests/integration/__init__.py", line 230, in start_zeromq_daemons
04:12:09     minion = salt.minion.Minion(self.minion_opts)
04:12:09   File "/testing/salt/minion.py", line 606, in __init__
04:12:09     opts['environment'],
04:12:09   File "/testing/salt/pillar/__init__.py", line 91, in compile_pillar
04:12:09     ret = self.sreq.crypted_transfer_decode(load, tries=3, timeout=7200)
04:12:09   File "/testing/salt/transport/__init__.py", line 252, in crypted_transfer_decode
04:12:09     return pcrypt.loads(ret)
04:12:09   File "/testing/salt/crypt.py", line 797, in loads
04:12:09     data = self.decrypt(data)
04:12:09   File "/testing/salt/crypt.py", line 769, in decrypt
04:12:09     sig = data[-self.SIG_SIZE:]
04:12:09 TypeError: unhashable type

@jfindlay
Copy link
Contributor

@rallytime, do you know how to fix this? This seems to be happening consistently for this pull request.

@jfindlay
Copy link
Contributor

Actually, it seems to be related to the code changes. @hvnsweeting, @thatch45, do you have any ideas?

@hvnsweeting
Copy link
Contributor Author

I will try to fix it in weekends as I'm not available for now
On Apr 14, 2015 10:37 PM, "Justin Findlay" notifications@github.com wrote:

Actually, it seems to be related to the code changes. @hvnsweeting
https://github.com/hvnsweeting, @thatch45 https://github.com/thatch45,
do you have any ideas?


Reply to this email directly or view it on GitHub
#22578 (comment).

or salt-minion will die. Returns None to deligate
this job to compile_pillar function handle it,
as it is the only caller of this function for now
@hvnsweeting
Copy link
Contributor Author

I reverted this commit hvnsweeting@776f166#diff-394afce37984c8269c65e4a59c91c581R252
it failed because loads method only accepts a "string", not a dict.

Test should be pass now, anw, I'm concerning about RAET corresponding method as it looks not return the same what ZQM returns hvnsweeting@776f166#diff-394afce37984c8269c65e4a59c91c581R137

but you may handled that in another place/way, so I leave it as it is.

@jfindlay
Copy link
Contributor

@thatch45, this change will apply successfully to 2015.2, whereas on develop the whole file is completely different. It might be worth it to have this in 2015.2.0 even if the code was overhauled in develop.

thatch45 added a commit that referenced this pull request Apr 20, 2015
gracefully handle when salt-minion cannot decrypt key
@thatch45 thatch45 merged commit c45b92b into saltstack:2014.7 Apr 20, 2015
@thatch45
Copy link
Contributor

Thanks @hvnsweeting !

@jfindlay
Copy link
Contributor

@basepi, this will likely lead to a merge forward conflict.

@basepi
Copy link
Contributor

basepi commented Apr 20, 2015

Story of my life. ;)

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

Successfully merging this pull request may close these issues.

None yet

5 participants