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

Adding support for CodeCommit events #5397

Closed
wants to merge 9 commits into from

Conversation

marcelobern
Copy link

Includes event documentation and working templates (git-client & CodeCommit API)

What did you implement:

Adds Feature #1409

Added support for AWS CodeCommit events, includes event documentation and two working templates (git-client & CodeCommit API)

How did you implement it:

CodeCommit (git) event was created by reusing the existing AWS SNS event code and adapting it to meet CodeCommit needs.

How can we verify it:

Test files were created (codecommit/index.test.js) or updated (naming.test.js, create.test.js) for unit testing and code coverage.

The aws-codecommit-nodejs and aws-git-client-nodejs templates include fully functional serverless.yml files which can be used to test the implementation on AWS.

Testing with aws-codecommit-nodejs
Once deployed the lambda function can be triggered by adding a new file to the repository and reviewing the CloudWatch Logs which should output something like:

{
 "Records": [
  {
   "awsRegion": "us-east-1",
   "codecommit": [Object],
   "eventId": "EVENT_ID",
   "eventName": "ReferenceChanges",
   "eventPartNumber": 1,
   "eventSource": "aws:codecommit",
   "eventSourceARN": "arn:aws:codecommit:us-east-1:ACCOUNT_ID:myRepo",
   "eventTime": "2018-10-14T22:08:08.477+0000",
   "eventTotalParts": 1,
   "eventTriggerConfigId": "EVENT_ID",
   "eventTriggerName": "CodeCommitClientGitRepositoryMyRepoTrigger",
   "eventVersion": "1.0",
   "userIdentityARN" "arn:aws:iam::ACCOUNT_ID:user/codeCommit"
  }
 ]
}
{ "branches": [ "master" ] }

Testing with aws-git-client-nodejs
After deploying the git-client template, it is necessary to follow step 3 of adding CodeCommit ssh keys before triggering the lambda function.

Once setup the lambda function can be triggered by adding a new file to the repository and reviewing the CloudWatch Logs which should output something like:

lambda-git: loaded
Command issued: clone
[
 {
  "status": "lambda-git: loaded"
 },
 {
  "status": "sshKey file finished"
 },
 {
  "status": "Cloned master",
  "result": ""
 }
]

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@marcelobern marcelobern mentioned this pull request Oct 14, 2018
9 tasks
@horike37
Copy link
Member

Thank you for working on this @marcelobern 👍

I just reviewed and I wonder why this event doesn't create codecommit repository and the related IAM policy inside the framework core in the following case?

functions:
  git:
    handler: git.handler
    events:
      - git: MyRepo

In other events, event source(like api gateway, sns event, iot) and IAM policy for invoking Lambda functions are generated in the framework core.

@marcelobern
Copy link
Author

Hi @horike37

I left it this way so we could have someone reuse an existing repo (which is not being created by this serverless.yml).

I believe this is consistent with the SNS behavior I based this code on.

Please let me know if you do not believe this would be a suitable use case.

Copy link
Contributor

@dschep dschep left a comment

Choose a reason for hiding this comment

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

Yeah, there's definitely not 100% consistency as to wether resources are expected to exist or are created. I agree that in this case it probably makes sense to have it existing. The other option is to have a preExisting flag so both are supported like is being done in #5642

@marcelobern
Copy link
Author

@horike37 thanks for all the patience and time you spent working to review this PR!

I went back to the testing (serverless/lib/plugins/aws/package/compile/events/codecommit/index.test.js) to check how the code was supposed to work and I am seeing that the only situation where the CodeCommit repository and corresponding permissions are not created is when an arn is used as the git event name.

Can you please clarify where are you testing and seeing that the following does not generate a repo and permission?

functions:
  git:
    handler: git.handler
    events:
      - git: MyRepo

This scenario is (supposedly) specifically covered in test case 4 (should create corresponding resources when CodeCommit events are given) for git: Repo 3.

Test case 4 is written to check that both the repo and corresponding permission are created and the test is passing, so unless I am missing something and the test case is not actually working as I thought it would, the issue should not happen.

I would appreciate any clarification you can provide so I can chase what I might be missing.

Thanks in advance!

@dschep thanks for taking the time to chime in. Question about the use of an arn formatted event name. I could not think of any scenarios where an arn named resource would not be an indication that the resource already exists and should not be created and thus the preExisting flag would be required. So I was eager for the opportunity to learn what scenario I might have overlooked and would appreciate your guidance ;-)

Thanks in advance!

@dschep
Copy link
Contributor

dschep commented Jan 8, 2019

yup, it's only an ARN or name, feel free to make the preexisting behavior automatic!

@horike37
Copy link
Member

horike37 commented Jan 8, 2019

Thank you for the response @marcelobern 👍
I actually tested with aws-codecommit-nodejs template you implemented but that wasn't created AWS::CodeCommit::Repository with serverless deploy. Am I something wrong? and Why does the template has roleCodeCommit within resource section of serverless.yml. I expected that the role could be generated behind the scenes without definition to serverless.yml.

It is no problem the role can be allowed within resource section as a functionality of the framework but those templates should be simple. i.e it would be good to something like this.

# Welcome to Serverless!
#
# This file is the main config file for your service.
#
# For full config options, check the docs:
#    docs.serverless.com
#
# Happy Coding!

service: aws-codecommit-nodejs # NOTE: update this with your service name

# You can pin your service to only deploy with a specific Serverless version
# which supports git (CodeCommit) events.
# Check out our docs for more details
# frameworkVersion: "=X.X.X"

provider:
  name: aws
  runtime: nodejs8.10

# you can overwrite defaults here
#  stage: dev
#  region: us-east-1

functions:
  codeCommitClient:
    handler: index.handler
    events:
      - git: myRepo # your repo name
      # you can add other CodeCommit events here
      # - git: otherRepository

@marcelobern
Copy link
Author

@horike37 If you are using the aws-codecommit-nodejs template I believe you will need to change its package.json as follows

{
  "name": "aws-codecommit-nodejs",
  "version": "1.0.0",
  "description": "CodeCommit example using AWS API",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "dependencies": {},
  "devDependencies": {
    "serverless": "marcelobern/serverless"
  },
  "author": "Marcelo Bernardes (https://github.com/marcelobern)",
  "license": "MIT"
}

This will basically change it so the serverless module being used will be pulled from the github repo marcelobern/serverless, where the git event is defined. Currently it is pulling serverless ^1.35.0 which of course does not implement the git event and as such will not generate the repository.

As for roleCodeCommit (the same applies for userCodeCommit), I left it (them) in the resources section for a few reasons:

  1. CodeCommit charges per active users. A role (and a user) happens to be a potentially chargeable item and so the role (user) could actually be created outside serverless.yml to be used across repos thus reducing the AWS CodeCommit cost incurred. This is somewhat commented in the serverless.yml files and detailed in the Note About CodeCommit Users & Roles section of documentation (serverless/docs/providers/aws/events/codecommit.md).
  2. It might be desirable to have "permanent" users, so one can have traceability of changes to repos, so having roles and users automatically generated by the serverless service is not necessarily a good thing for this use case.

So while roles and users for CodeCommit are not necessarily ideal to be automatically generated in the serverless.yml, I wanted to provide a working sample, and so I added them to the resources section and made a note that users should be careful (about charges) when using it.

Hopefully this makes sense to you but please feel free to challenge any piece that does not make sense (I rather get this done as well as we can the first go around ;-) )

Thanks again for all your help and questions!!!

@marcelobern
Copy link
Author

This discussion reminded me that once we know in which serverless release this PR will be added to, we should update the serverless versions in package.json of both templates (lib/plugins/create/templates/aws-codecommit-nodejs/package.json and lib/plugins/create/templates/aws-git-client-nodejs/package.json) to match that release!

@marcelobern
Copy link
Author

@horike37 sorry but I had completely forgotten about this PR. Anything you need from me?

Copy link
Contributor

@pmuens pmuens left a comment

Choose a reason for hiding this comment

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

Hey @marcelobern!
First of all: Thanks a ton for taking the time to work on this! This is one of the intense PRs and we greatly appreciate you jumping in here to implement this feature. 🙇 🙇‍♀

It's been a while and a lot of changed in the AWS landscape. It looks like the introduction of the EventBridge service makes it possible to listen to CodeCommit events and trigger Lambda functions based on different filters one can configure.

I'll close this PR for now given that EventBridge can be used to achieve the same outcome.

@marcelobern thanks again for taking the time to work on this. We're really happy to have such engaged members in our community! 🙏 💯 🥇

@pmuens pmuens closed this Jul 31, 2019
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.

None yet

4 participants