Issues with environment variable configuration options
diasjorge opened this issue · 10 comments
According to the new option to add configurations in environment variables to set a region password one would have to provide a key like bless_ca_us-east-1_password as shown in this test https://github.com/Netflix/bless/blob/master/tests/config/test_bless_config.py#L40
Unfortunately this is not a valid environment variable name for lambda.
One alternative to fix this would be to replace the "-" with "_" and so name the environment variable bless_ca_us_east_1_password. Would this be acceptable?
Another thing I noticed after deploying this is that there is a 4KB limit on the environment variables so when I tried creating just one key "bless_ca_ca_private_key" with the base64 encoded value of my certificate it already exceeds the 4kb limit making it impractical for the purpose of not having the key bundled with the zipfile. Am I missing anything? maybe @ikben could comment on this?
I think using bless_ca_us_east_1_password is good solution. Also pretty easy to change by editing the _environment_key function: https://github.com/Netflix/bless/blob/master/bless/config/bless_config.py#L147
We use the bless_ca_default_password environment variable, so I didn't run into this issue.
We also ran into the issue of exceeding 4 KB. Using a 2048 bit private key will stay below this limit.
My colleague has been thinking about a solution to include a 4096 bit key in the environment (beside some metadata the key is already base64 encoded), but this would require bigger changes to the code base.
I've talked to some colleagues but we don't want to use shorter private keys. What kind of ideas do you have for that?
I'm also an advocate of using a 4096 bit key for this. After taking a look, I can reproduce the issue with the api.
From the API responses: Value "must satisfy regular expression pattern: a-zA-Z+]" and "Lambda was unable to configure your environment variables because the environment variables you have provided exceeded the 4KB limit."
If you wanted to use gzip, it looks like a 4096 bit key gzipped and base64 encoded works out to about 3.3KB.
Thanks @russell-lewis I'll give it a try using gzip and see how that works and what kind of impact it would have on the code base.
Gzip is a great solution, I whish I've had tought of it before.
If we do a try/catch around zlib.decompress when reading the keyfile, we can support both compressed and uncompressed keys in the same setting.
Or is there a preference for adding an extra settings key (eg bless_ca_ca_private_key_compression = ['zlib'|None])
I'd rather have the compression set by an additional parameter, who's default is None.
Something like this? https://github.com/ikben/bless/blob/env-v2/bless/config/bless_config.py#L109
I'm still thinking about how to write tests for this, tips are welcome. So are comments on that implementation
Just my opinion but I think for testing you could focus on the getprivatekey method.
Setup the config with CA_PRIVATE_KEY_OPTION and a uncompressed base64 encoded key check the returned value is the key.
Another test with compression set to zlib and an encrypted key.
Another that tests the exception when an invalid format is set.
And for completion another one when the file option is set that reads the file maybe using http://www.voidspace.org.uk/python/mock/helpers.html#mock-open.
I don't think the value of the key is necessarily important to be a valid rss key though.
For unit testing, I've included an encrypted private key:
https://github.com/Netflix/bless/blob/master/tests/aws_lambda/only-use-for-unit-tests.pem
https://github.com/Netflix/bless/blob/master/tests/ssh/vectors.py#L3-L6
You can add a gzipped then base64 encoded version of that key to the vectors.py.
In addition to testing BlessConfig.getprivatekey() you can add a test of the lambda itself that will produce a valid ssh-cert with an environment loaded, gzipped ssh key like this:
https://github.com/russell-lewis/bless/blob/PR43_enhancements/tests/aws_lambda/test_bless_lambda.py#L161-L176
While that branch isn't merged yet, the environment config support is, so you can add tests like that now on master.