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

feat(aws-lambda-ssm-string-parameter): New aws-lambda-ssm-string-parameter pattern implementation (#64) #175

Merged

Conversation

danielmatuki
Copy link
Contributor

closes #64

Issue #, if available: #64

Description of changes:
Implements the AWS Solutions Construct that creates an AWS Lambda function and AWS Systems Manager Parameter Store String parameter with the least privileged permissions.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

danielmatuki and others added 19 commits April 21, 2021 11:03
…ng-parameter' into danielmatuki/aws-lambda-ssm-string-parameter

# Conflicts:
#	source/patterns/@aws-solutions-constructs/aws-lambda-ssm-string-parameter/package.json
#	source/patterns/@aws-solutions-constructs/core/lib/vpc-helper.ts
#	source/patterns/@aws-solutions-constructs/core/package.json
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
  • Commit ID: 957eed7
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@hnishar
Copy link
Contributor

hnishar commented May 11, 2021

Does the word string in the construct name aws-lambda-ssm-string-parameter imply it will only support the string SSM parameter type and not the other types e.g. StringList and possibly SecureString in future ?

Copy link
Contributor

@biffgaut biffgaut left a comment

Choose a reason for hiding this comment

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

A few changes, mostly naming and the like.

@@ -0,0 +1,101 @@
# aws-lambda-ssm-string-parameter module
Copy link
Contributor

Choose a reason for hiding this comment

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

Name should be aws-lambda-ssmstringparameter

*/
constructor(scope: Construct, id: string, props: LambdaToSsmStringParameterProps) {
super(scope, id);

Copy link
Contributor

Choose a reason for hiding this comment

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

See PR just added for addition of CheckProps() function. You will want to add it here, and add a check for stringParameter to input-validation.ts and the associated unit test.

super(scope, id);

if (props.deployVpc || props.existingVpc) {
if (props.deployVpc && props.existingVpc) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We check for this in CheckProps now, so you can drop it here.

*/
readonly lambdaFunctionProps?: lambda.FunctionProps;
/**
* Existing instance of SSM String parameter object, If this is set then the stringParameterProps is ignored.
Copy link
Contributor

Choose a reason for hiding this comment

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

Behavior here is changed with CheckProps - sending both now generates an error. Copy what was done in the last PR.


// Setup
const app = new App();
const stack = new Stack(app, 'test-lambda-ssm-string-parameter');
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a nit - use the correct construct name

handler: 'index.handler',
code: lambda.Code.fromAsset(`${__dirname}/lambda`)
};
const existingFuntion = defaults.deployLambdaFunction(stack, lambdaFunctionProps);
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in variable name - existingFunction

});

// --------------------------------------------------------------
// Test minimal deployment with an existing VPC and existing Lambda function not in a VPCs
Copy link
Contributor

Choose a reason for hiding this comment

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

...not in a VPC. (no s)

// Helper declaration
new LambdaToSsmStringParameter(stack, "lambda-to-ssm-stack", {
lambdaFunctionProps: {
runtime: lambda.Runtime.NODEJS_10_X,
Copy link
Contributor

Choose a reason for hiding this comment

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

NodeJS version...

* @param stringParameterProps
*/
export function buildSsmStringParamter(scope: Construct, id: string, stringParameterProps: StringParameterProps): StringParameter {
return new StringParameter(scope, id, stringParameterProps);
Copy link
Contributor

Choose a reason for hiding this comment

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

So I'm looking at possible defaults, and see that there really aren't any. But I also see parameter Type as a member of StringParameterProps. What happens if you send SECURE_STRING or STRINGLIST? Does it get ignored? The docs are pretty clear that you can't create a SECURE_STRING parameter with the CDK, and StringList has a completely separate L2 construct.

If it must be STRING, then we should overrule whatever the user sent and dump a warning that we are doing so. Also should point it out in the docs.

@@ -0,0 +1,26 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

should be ssm-string-parameter-helper

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
  • Commit ID: 4093afe
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
  • Commit ID: fe86781
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-fkVQbXRiQi6A
  • Commit ID: 61bfbaf
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@biffgaut biffgaut merged commit b0567e4 into awslabs:main May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New pattern: aws-lambda-ssm-parameter
4 participants