-
Notifications
You must be signed in to change notification settings - Fork 76
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
Add the ability to read the license key from AWS secrets manager #18
Open
dswisher
wants to merge
7
commits into
newrelic:master
Choose a base branch
from
dswisher:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
9bbd8f0
Add the ability to read the license key from AWS secrets manager
dswisher 4a7303a
Merge remote-tracking branch 'upstream/master'
dswisher b826f04
Put boto3 dependency in Pipfile
dswisher accbd05
Only print debug statement if debug enabled
dswisher fcac366
Merge remote-tracking branch 'upstream/master'
dswisher 9ede10a
Resolve merge conflict
dswisher ef7667e
Fix formatting
dswisher File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,3 +20,6 @@ packaged.yaml | |
node_modules | ||
package.json | ||
package-lock.json | ||
|
||
.python-version | ||
build/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ python_version = "3.7" | |
|
||
[packages] | ||
aiohttp = "*" | ||
boto3 = "*" | ||
|
||
[pipenv] | ||
allow_prereleases = true |
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
#!/bin/bash | ||
|
||
# For those unable to use SAM, this builds a zip file that can be deployed using the AWS CLI or terraform or the like. | ||
# The resulting zip will be placed in the build directory. It uses a python3 virtual environment, per the | ||
# instructions here: https://docs.aws.amazon.com/lambda/latest/dg/python-package.html | ||
|
||
# A suffix may be added to the zip name by defining the SUFFIX environment variable before invoking this script. | ||
|
||
# Constants | ||
BUILDDIR=build | ||
|
||
# Extract the version from the template.yaml file | ||
VERSION=`grep SemanticVersion template.yaml | sed -e 's/ *SemanticVersion: *//'` | ||
ZIPFILE=newrelic-log-ingestion-${VERSION}${SUFFIX}.zip | ||
|
||
# Clean up any old build | ||
echo "Cleaning old builds..." | ||
rm -rf ${BUILDDIR} | ||
|
||
# Create the virtual environment, and activate it | ||
echo "Setting up virtual environment..." | ||
python3 -m venv ${BUILDDIR} | ||
source build/bin/activate | ||
|
||
# Install the requirements, excluding boto3, which is pre-installed in AWS | ||
echo "Installing dependencies..." | ||
cat src/requirements.txt | grep -v boto3 > /tmp/requirements.txt | ||
pip3 --disable-pip-version-check install --requirement /tmp/requirements.txt | ||
|
||
# Deactivate the virtual environment | ||
deactivate | ||
|
||
# Package up the dependencies | ||
echo "Creating zip of dependencies..." | ||
(cd ${BUILDDIR}/lib/python*/site-packages && zip --quiet --recurse-paths -9 ../../../${ZIPFILE} .) | ||
|
||
# Add the function to the zip file | ||
echo "Adding function to zip..." | ||
(cd src && zip --grow --quiet ../${BUILDDIR}/${ZIPFILE} function.py) | ||
|
||
# All done! | ||
echo "Done, zipfile: ${BUILDDIR}/${ZIPFILE}" | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,16 @@ | ||
-i https://pypi.org/simple | ||
aiohttp==4.0.0a1 | ||
async-timeout==3.0.1 | ||
attrs==19.3.0 | ||
aiohttp==3.6.2 | ||
async-timeout==3.0.1; python_full_version >= '3.5.3' | ||
attrs==19.3.0; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' | ||
boto3==1.14.10 | ||
botocore==1.17.10 | ||
chardet==3.0.4 | ||
idna==2.9 | ||
multidict==4.7.5 | ||
typing-extensions==3.7.4.1 | ||
yarl==1.4.2 | ||
docutils==0.15.2; python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2, 3.3' | ||
idna==2.9; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' | ||
jmespath==0.10.0; python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2, 3.3' | ||
multidict==4.7.6; python_version >= '3.5' | ||
python-dateutil==2.8.1; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' | ||
s3transfer==0.3.3 | ||
six==1.15.0; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' | ||
urllib3==1.25.9; python_version != '3.4' | ||
yarl==1.4.2; python_version >= '3.5' |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the LICENSE_KEY env var first. If it's set, we use it and are done. Otherwise, we fall back to secrets manager, which has considerable overhead.
In addition, we should move the secret manager fetch to the cold start period, rather than first invocation.
Finally, I suspect the global state you've introduced here is responsible for the test failures. You'll need to adjust the tests so that they don't have side effects, and aren't dependent on execution order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MattWhelan You are correct, the global state makes the unit tests challenging. To address that and your other concerns, I'd like to change the code so that
_get_license_key
is called exactly once, fromlambda_handler
(which I believe is the "cold start period" to which you're referring).That is a much more involved change, as the license key then needs to be passed down to everything that
lambda_handler
calls, but it seems to me like the right approach. Would you agree?