Skip to content

Conversation

unlox775-code-dot-org
Copy link

@unlox775-code-dot-org unlox775-code-dot-org commented May 9, 2025

Description

I'm replacing the old “hard-coded GitHub actor regex” CodeBuild webhook hack with an AWS EventBridge → Lambda → CodeBuild flow.
On each pull‐request event:

  1. EventBridge catches the PR “state change” on the configured branch.
  2. It invokes a Lambda authorizer that fetches your GitHub PAT from Secrets Manager, calls the GitHub API to verify the PR author’s permission (write/maintain/admin), and only then
  3. invokes StartBuild on the PR-build CodeBuild project.

Links

Testing story

  • Manual EventBridge simulation
    aws events put-events --entries '[{
      "Source": "aws.partner/github.com/code-dot-org/aiproxy",
      "DetailType": "Pull Request State Change",
      "Detail": "{\"action\":\"opened\",\"pull_request\":{\"base\":{\"ref\":\"main\"},\"user\":{\"login\":\"YOUR_GITHUB_ID\"}}}"
    }]' --region us-east-1
  • SAM local invoke
    1. Install AWS SAM CLI.
    2. Create an event.json matching the structure above.
    3. Run:
      cd cicd/2-cicd
      sam local invoke PullRequestAuthorizerFunction --event event.json
  • Check CloudWatch logs and/or the CodeBuild console to confirm whether the build was (or wasn’t) started.

Follow-up work

  • Add unit and integration tests for the Lambda authorizer.
  • Instrument metrics on GitHub API calls & CodeBuild starts.
  • Update onboarding docs for adding new maintainers (no more regex).

PR Checklist

  • Documentation updated (cicd/README.md, CFN template).
  • Manual tests performed and verified.
  • Lambda authorizer comments & code clarity reviewed.

Comment on lines +44 to +50
post_build:
commands:
- echo "Verifying build success before moving on: $CODEBUILD_BUILD_SUCCEEDING"
- |
if [ "$CODEBUILD_BUILD_SUCCEEDING" != "1" ]; then
echo "Previous phase failed; aborting post_build"
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Never seen this before, what makes it necessary?

Properties:
Handler: index.handler
Runtime: python3.9
InlineCode: |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a duplicate of "cicd/2-cicd/authorizer.py"? Should we be loading that file instead of inlining it here?

coverage==7.3.2
scikit-learn~=1.3.2
boto3==1.34.30
moto==4.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

should we use a separate requirements.txt for the authorizer, to avoid mixing cicd dependencies with app dependencies?

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.

2 participants