DOP-4414: Lambda function to handle automated test deploys #1011
Conversation
|
Your feature branch infrastructure has been deployed! Your webhook URL is: https://rmjgqxhir7.execute-api.us-east-2.amazonaws.com/prod/webhook/githubEndpoint/trigger/build For more information on how to use this endpoint, follow these instructions. |
anabellabuckvar
changed the title
anabellabuckvar
changed the title
anabellabuckvar
changed the title
| # paths: ['src/**', 'cdk-infra/lib/constructs/worker/**', 'Dockerfile', 'modules/**'] | ||
| branches: | ||
| - 'main' | ||
| - 'integration' | ||
|
|
||
| - 'DOP-4414' |
There was a problem hiding this comment.
idk if this is purely for testing purposes, but marking to see if we need revert this back
api/controllers/v2/github.ts
Outdated
| // if the build was not building main branch, no need for smoke test sites | ||
| // if (body.workflow_run.head_branch != 'main' || body.repository.fork) { | ||
| // console.log('Build was not on master branch in main repo, sites will not deploy as no smoke tests are needed'); | ||
| // return { | ||
| // statusCode: 202, | ||
| // headers: { 'Content-Type': 'text/plain' }, | ||
| // body: `Build on branch ${body.workflow_run.head_branch} will not trigger site deployments as it was not on main branch in upstream repo`, | ||
| // }; | ||
| // } | ||
|
|
There was a problem hiding this comment.
Remove? Or maybe you're actively working on this.
| url = 'https://github.com/' + repoOwner + '/' + repoName; | ||
| action = 'automatedTest'; | ||
| jobType = 'productionDeploy'; | ||
| branchName = 'master'; |
There was a problem hiding this comment.
not blocking (I don't think): I believe we've instituted a way within the parser to deal with a content repo's master branch to be named either "master" or "main". I wonder if we need to find a way to be agnostic here as well... once again, this is maybe a different ticket. To my knowledge all content repos use "master"...
There was a problem hiding this comment.
Good point, thank you. I did consider this, but I checked at least the specific content repos that Allison requested always be deployed for smoke tests, and those at least all use "master" as the master branch.
If, at a later date, someone, for some reason, adds a repo to the list that uses main, they will have to add additional logic to handle that then. However, I agree that I don't believe it should be blocking right now for that hypothetical
src/job/productionJobHandler.ts
Outdated
| // getPathPrefix(): string { | ||
| // try { | ||
| // if (this.currJob.payload.prefix && this.currJob.payload.prefix === '') { | ||
| // return this.currJob.payload.urlSlug ?? ''; | ||
| // } | ||
| // if (this.currJob.payload.urlSlug) { | ||
| // if (this.currJob.payload.urlSlug === '') { | ||
| // return this.currJob.payload.prefix; | ||
| // } else { | ||
| // return `${this.currJob.payload.prefix}/${this.currJob.payload.urlSlug}`; | ||
| // } | ||
| // } | ||
| // return this.currJob.payload.prefix; | ||
| // } catch (error) { | ||
| // this.logger.save(this.currJob._id, error).then(); | ||
| // throw new InvalidJobError(error.message); | ||
| // } | ||
| // } | ||
|
|
There was a problem hiding this comment.
It was commented out so I could do some testing! It's added back in now :)
| @@ -73,7 +73,7 @@ export class RepoEntitlementsRepository extends BaseRepository { | |||
| `Mongo Timeout Error: Timedout while retrieving entitlements for ${slackUserId}` | |||
| ); | |||
| // if user has specific entitlements | |||
| if ((entitlementsObject?.repos?.length ?? 0) > 0) { | |||
| if (entitlementsObject?.repos && entitlementsObject.repos.length > 0) { | |||
There was a problem hiding this comment.
nit: we can shorten this to entitlementsObject?.repos?.length > 0
src/job/productionJobHandler.ts
Outdated
| try { | ||
| if (this.currJob.payload.prefix && this.currJob.payload.prefix === '') { | ||
| return this.currJob.payload.urlSlug ?? ''; | ||
| } |
There was a problem hiding this comment.
I'm only worried about this because it is in the productionJobHandler and it's a difference in pathing that does not solely rely on making sure this is a deploy_all situation. Could we also make sure that this has a conditional dependent on if this job is specifically a deploy all slack command?
There was a problem hiding this comment.
When I click on the link to the version of the productionJobHandler file you reviewed, it shows that it is only reflecting changes from the past 4 commits- the changes on these lines within those commits are actually just reverting previous changes made. If you look at those lines in src/job/productionJobHandler.ts here you'll actually see that there aren't any differences in comparison to the main branch there!
There was a problem hiding this comment.
ohhhhhhh wonderful.
You're correct, I must have been looking at the "changes since I last looked"
Stories/Links:
DOP-4414
Notes
This PR includes a new webhook that is triggered by workflow action updates. However, there is logic in docs-worker-pool such that the smoke test deploy lambda will only be triggered, and therefore, only run, upon successful completion of a Deploy Staging ECS push job to main.
This PR also includes some refactoring and simplification of preexisting code for slack deploys and feature branch push jobs.
Please take note of error handling and the status codes returned for different cases and leave feedback if applicable!
Testing
Webhook, endpoint, and lambda were all tested by pushing to this branch and therefore triggering the workflow. Github pushes on feature branches were also tested and verified to still be working as expected; although this PR did not make meaningful changes to that functionality, it was incorporated in refactoring
Cloud Docs Build example:
Build Logs
Build URL
README updates