fugue / credstash

A little utility for managing credentials in the cloud
Apache License 2.0
2.06k stars 214 forks source link

Misleading Error message when credential-store table is empty #261

Closed achilds-vela closed 4 years ago

achilds-vela commented 4 years ago

Prerequisites: An empty dynamodb table in aws

Action: credstash getall

Expected results: Either an empty json, {}, an error saying that the table is empty, or something like that.

Actual result:

Traceback (most recent call last):
  File "./copy_credstash_values.py", line 128, in <module>
    main()
  File "./copy_credstash_values.py", line 121, in main
    backup_tables()
  File "./copy_credstash_values.py", line 86, in backup_tables
    bkp_data = getall(table_name, table_region)
  File "./copy_credstash_values.py", line 77, in getall
    return credstash.getAllSecrets(table=table, region=region)
  File "/xxx/xxx/venv/lib/python3.7/site-packages/credstash.py", line 359, in getAllSecrets
    pool = ThreadPool(min(len(names), THREAD_POOL_MAX_SIZE))
  File "/usr/local/Cellar/python/3.7.4_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/multiprocessing/dummy/__init__.py", line 124, in Pool
    return ThreadPool(processes, initializer, initargs)
  File "/usr/local/Cellar/python/3.7.4_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/multiprocessing/pool.py", line 805, in __init__
    Pool.__init__(self, processes, initializer, initargs)
  File "/usr/local/Cellar/python/3.7.4_1/Frameworks/Python.framework/Versions/3.7/lib/python3.7/multiprocessing/pool.py", line 172, in __init__
    raise ValueError("Number of processes must be at least 1")
ValueError: Number of processes must be at least 1

Diagnostic info: Credstash version:

(venv) xxx$ pip show credstash
Name: credstash
Version: 1.16.1

Python version: Python 3.7.4

Additional notes: For whatever it's worth, I dug into the code to figure out where it was happening: https://github.com/fugue/credstash/blob/master/credstash.py#L359

def getAllSecrets(version="", region=None, table="credential-store",
                  context=None, credential=None, session=None, **kwargs):
...
    pool = ThreadPool(min(len(names), THREAD_POOL_MAX_SIZE))

In context, the above is creating a threadpool and setting the "processes" argument to the minimum of either the length of the of the set of keys, or the THREAD_POOL_MAX_SIZE. If there are no keys, it gets set to 0, which subsequently throws the ValueError.

I don't particularly care whether it's an error or an empty dict/json returned, but as it is, the Value Error isn't very helpful.

mike-luminal commented 4 years ago

Thanks for your work on this, I'll take a look now

achilds-vela commented 4 years ago

@mike-luminal Awesome! Thanks for fixing it!