Miserlou / Zappa

Serverless Python
https://blog.zappa.io/
MIT License
11.89k stars 1.2k forks source link

Warn users if overriding existing environment variables #1000

Open mjschultz opened 7 years ago

mjschultz commented 7 years ago

Context

The default Lambda environment comes with several variables already defined, odds are a user doesn't want to accidentally override any of these (though it's possible they might). In some cases, overriding one may actually cause Zappa to fail in strange ways (e.g., any of the AWS_* authentication credentials).

It would be nice if Zappa could print a nice warning message when any existing environment variable is overwritten by the software.

Possible Fix

There are several levels of environment variables and ways to get them in, but as a first pass I think a if statement around the injection with a printout would help. It'll only go to the CloudWatch Logs and come before any actual errors that might occur, but I know if I saw a warning at the top I'd do something about it.

wagmiwiz commented 7 years ago

Have you tried with master and aws_env_vars key? This will prevent Lambda vars from getting wiped, see https://github.com/Miserlou/Zappa/pull/962 and later fixes. If that works this could be closed pending a new version cut.

mjschultz commented 7 years ago

I haven't had a chance to try it, but looking through the various PRs it doesn't seem to do anything to the built-in environment variables but rather the Lambda AWS environment variables.

Now, if the remote_env option is totally removed this won't be an issue as it is up to AWS to protect their built-in environment variables from being overridden, e.g., they issue an error like:

The Configuration tab failed to save. Reason: Lambda was unable to configure your environment variables because the environment variables you have provided contains reserved keys that are currently not supported for modification. Reserved keys used in this request: AWS_ACCESS_KEY_ID
wagmiwiz commented 7 years ago

Ah, so you mean if you were to put a special AWS_* env var in aws_environment_variables then its likely your zappa deploy/update may fail? And if you put it in any other env vars, like remote or local ones, then you are 'masking' the ones that AWS Lambda provides and depends on so are being naughty and should be warned.

wagmiwiz commented 7 years ago

@mjschultz wouldn't this be helped by simply having a console warning on deploy/update when someone has AWS_* (or perhaps a specific list of known special ones) in any of the possible env. variable locations?

mjschultz commented 7 years ago

Yeah, that would help I think for all cases except for the remote_env case. I think the remote_env is only loaded at lambda runtime so during deploy checks it would go overlooked. So it would cover 2 of the 3 ways to get environment vars into lambda (maybe 3 of 4?).

wagmiwiz commented 7 years ago

Ah yes, good point.

On Tue, 15 Aug 2017 at 15:48 Michael J. Schultz notifications@github.com wrote:

Yeah, that would help I think for all cases except for the remote_env case. I think the remote_env is only loaded at lambda runtime so during deploy checks it would go overlooked. So it would cover 2 of the 3 ways to get environment vars into lambda (maybe 3 of 4?).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Miserlou/Zappa/issues/1000#issuecomment-322490383, or mute the thread https://github.com/notifications/unsubscribe-auth/ACQOB6HY5xQ2GrlzWch2ASPEfpqOHf1Uks5sYa-fgaJpZM4ORfW_ .