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

Fail lambda if shouldFail flag provided #233

Merged
merged 4 commits into from
Sep 19, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ Currently available middlewares:
- [`jsonBodyParser`](/docs/middlewares.md#jsonbodyparser): Automatically parses HTTP requests with JSON body and converts the body into an object. Also handles gracefully broken JSON if used in combination of
`httpErrorHandler`.
- [`s3KeyNormalizer`](/docs/middlewares.md#s3keynormalizer): Normalizes key names in s3 events.
- [`Secrets Manager`](/docs/middlewares.md#secretsmanager): Fetches parameters from AWS Secrets Manager.
- [`ssm`](/docs/middlewares.md#ssm): Fetches parameters from [AWS Systems Manager Parameter Store](https://docs.aws.amazon.com/systems-manager/latest/userguide/systems-manager-paramstore.html).
- [`validator`](/docs/middlewares.md#validator): Automatically validates incoming events and outgoing responses against custom schemas
- [`urlEncodeBodyParser`](/docs/middlewares.md#urlencodebodyparser): Automatically parses HTTP requests with URL encoded body (typically the result of a form submit).
Expand Down
1 change: 1 addition & 0 deletions README.md.hb
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ Currently available middlewares:
- [`jsonBodyParser`](/docs/middlewares.md#jsonbodyparser): Automatically parses HTTP requests with JSON body and converts the body into an object. Also handles gracefully broken JSON if used in combination of
`httpErrorHandler`.
- [`s3KeyNormalizer`](/docs/middlewares.md#s3keynormalizer): Normalizes key names in s3 events.
- [`Secrets Manager`](/docs/middlewares.md#secretsmanager): Fetches parameters from AWS Secrets Manager.
- [`ssm`](/docs/middlewares.md#ssm): Fetches parameters from [AWS Systems Manager Parameter Store](https://docs.aws.amazon.com/systems-manager/latest/userguide/systems-manager-paramstore.html).
- [`validator`](/docs/middlewares.md#validator): Automatically validates incoming events and outgoing responses against custom schemas
- [`urlEncodeBodyParser`](/docs/middlewares.md#urlencodebodyparser): Automatically parses HTTP requests with URL encoded body (typically the result of a form submit).
Expand Down
1 change: 1 addition & 0 deletions docs/middlewares.md
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ For each secret, you also provide the name under which its value should be added
- `secrets` (object) : Map of secrets to fetch from Secrets Manager, where the key is the destination, and value is secret name in Secrets Manager.
Example: `{secrets: {RDS_LOGIN: 'dev/rds_login'}}`
- `awsSdkOptions` (object) (optional): Options to pass to AWS.SecretsManager class constructor.
- `shouldFail` (boolean) (optional): Defaults to `false`. Set it to `true` if you want your lambda to fail in case call to AWS Secrets Manager fails (secrets don't exist or internal error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe name it more explicitly to better convey the intent?

Like throwOnMissingParam or ignoreMissingParams

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I couldn't think of any good name, your proposal doesn't cover errors other than missing param in store - how about throwOnFailedCall?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, agree, doesn't cover throttling errors etc, so another thing I can think of is to borrow bail name from jest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I think throw would be more descriptive here, bail is not so common


NOTES:
* Lambda is required to have IAM permission for `secretsmanager:GetSecretValue` action
Expand Down
53 changes: 52 additions & 1 deletion src/middlewares/__tests__/secretsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('🔒 SecretsManager Middleware', () => {
promise = promise.then(() => {
return new Promise((resolve, reject) => {
handler(event, context, (error, response) => {
if (error) reject(error)
sdomagala marked this conversation as resolved.
Show resolved Hide resolved
try {
cb(error, {event, context, response})
resolve()
Expand All @@ -68,7 +69,7 @@ describe('🔒 SecretsManager Middleware', () => {
}
})
})
promise.then(done).catch(err => done(err))
promise.then(done).catch(done)
}

test(`It should set secrets to context`, (done) => {
Expand Down Expand Up @@ -217,6 +218,56 @@ describe('🔒 SecretsManager Middleware', () => {
})
})

test(`It should fail if "shouldFail" flag provided and call failed`, (done) => {
const errorMessage = 'Internal Error / Secret doesn\'t exist'
getSecretValueMock.mockReturnValueOnce({
promise: () => Promise.reject(new Error(errorMessage))
})

const errHandler = err => {
getSecretValueMock.mockClear()
expect(err.message).toEqual(errorMessage)
done()
}
return testScenario({
mockResponse: {},
middlewareOptions: {
secrets: {
KEY_NAME: 'failed_call'
},
shouldFail: true
},
callbacks: [
(_, {context}) => {
throw new Error('Not supposed to be called')
}
],
done: errHandler
})
})

test(`It should resolve if "shouldFail" flag not provided and call failed`, (done) => {
const errorMessage = 'Internal Error / Secret doesn\'t exist'
getSecretValueMock.mockReturnValueOnce({
promise: () => Promise.reject(new Error(errorMessage))
})
return testScenario({
mockResponse: {},
middlewareOptions: {
secrets: {
KEY_NAME: 'failed_call'
}
},
callbacks: [
(_, {context}) => {
sdomagala marked this conversation as resolved.
Show resolved Hide resolved
expect(getSecretValueMock).toBeCalled()
getSecretValueMock.mockClear()
}
],
done
})
})

test(`It should only refresh once per cache expiry window`, (done) => {
// with cache expiry of 50ms, test what happens when one refresh fails, and
// that the middleware doesn't retry for another 50ms
Expand Down
4 changes: 4 additions & 0 deletions src/middlewares/secretsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = opts => {
const defaults = {
awsSdkOptions: {},
secrets: {}, // e.g. { RDS_SECRET: 'dev/rds_login', API_SECRET: '...' }
shouldFail: false,
cache: false,
cacheExpiryInMillis: undefined,
secretsLoaded: false,
Expand Down Expand Up @@ -57,6 +58,9 @@ module.exports = opts => {
// if we already have a cached secrets, then reset the timestamp so we don't
// keep retrying on every invocation which can cause performance problems
// when there's temporary problems with Secrets Manager
if (options.shouldFail) {
throw err
}
if (options.secretsCache) {
options.secretsLoadedAt = new Date()
}
Expand Down