Enterprise-CMCS / macpro-quickstart-serverless

Other
18 stars 14 forks source link

Check for IAM Role Name Length #564

Closed leslie-corbalt closed 2 years ago

leslie-corbalt commented 2 years ago

Purpose

This PR checks that the length of the AWS IamRoleLambdaExecution.RoleName for every lambda before the QuickStart is deployed is less than 65 characters.

I added the check in .scripts/deploy.sh because:

  1. the check should be done before the deploy
  2. the check should be done when a developer deploys to their feature branch from the command line
  3. developer feature branch names can be long, which means there is a good change some combination of stage+service+function is too long

I did not add this check to the .github/scripts/branch_name_validation.sh. You might want to consider whether that script is still necessary.

Background

serverless.com creates a IAM RoleName with various fields: {service}-{stage}-{function}-{region}-lambdaRole per serverless-iam-roles-per-function plugin

In this PR, I assume:

  1. {region} is 9 characters (i.e., us-east-1)
  2. MAXLENGTH = 64 - {region}
  3. restrict the length of {service} + {stage} + {function} <= MAXLENGTH

Linked Issues to Close

(https://jiraent.cms.gov/browse/CMCSMACD-528)

Learning

I learned that serverless.com constructs the default Iam RoleName using {service}-{stage}-{region}-lambaRole The plugin listed above adds an additional field: -{function}- This can easily cause the deploy to fail for some combination of stage, service, function. This is especially frustrating when the failure happens towards the end of the deploy.

Assorted Notes/Considerations

Notes are linked in the jira ticket, which is linked above.

Pull Request Creator Checklist

Pull Request Reviewer/Assignee Checklist

leslie-corbalt commented 2 years ago

Hey have you considered making this a pre commit hook of some type? That might be ideal, what do you think

@mdial89f I'll look into making this a pre-commit hook. Are you thinking that I add the bash functions I created to the file .githooks/pre-commit and then invoke checkStageServiceFunctionLength in the pre-commit file? That means the check is invoked for each commit. However, when doing development, before I commit, I deploy from my local machine, and I also want the check invoked before any services are deployed. That's why I added the check to the deploy.sh script. Please let me know what you think.

mdial89f commented 2 years ago

Hey have you considered making this a pre commit hook of some type? That might be ideal, what do you think

@mdial89f I'll look into making this a pre-commit hook. Are you thinking that I add the bash functions I created to the file .githooks/pre-commit and then invoke checkStageServiceFunctionLength in the pre-commit file? That means the check is invoked for each commit. However, when doing development, before I commit, I deploy from my local machine, and I also want the check invoked before any services are deployed. That's why I added the check to the deploy.sh script. Please let me know what you think.

@leslie-corbalt ahhh fair point there, do what you think is best, good for either

leslie-corbalt commented 2 years ago

Hey @leslie-corbalt I'm good with this as is, or if you moved to a hook. Just let me know if I should merge now or wait for changes. Thanks

@mdial89f I did not move to a hook. Thanks for approving. Please merge.