-
Notifications
You must be signed in to change notification settings - Fork 415
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: Implement GitHub Webhook #3906
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3906 +/- ##
=======================================
Coverage 95.92% 95.93%
=======================================
Files 1134 1135 +1
Lines 35956 36028 +72
=======================================
+ Hits 34492 34564 +72
Misses 1464 1464 ☔ View full report in Codecov by Sentry. |
@@ -245,6 +245,10 @@ | |||
"name": "SSE_AUTHENTICATION_TOKEN", | |||
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SSE_AUTHENTICATION_TOKEN::" | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we added these secrete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet, I don't have the credentials or authorization to do it. Could you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we added it to staging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added instructions in the description on how to update the GitHub App once we have a secret defined and set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we can't merge it without it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For another env var added @matthewelwell asked me to do first the PR declaring the env var in the infrastructure folder. And he set the value after that, however, I would say that he has the last word.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that makes sense, but we still can't merge it before the secrets are added because the deployment will fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthewelwell have set the env var secret.
Thanks for submitting a PR! Please check the boxes below:
pre-commit
to check lintingdocs/
if required so people know about the feature!Changes
Implement GitHub Webhook to receive events from GitHub App.
Initially we only handle the Integration Delete action. We delete the configuration and any related object in the DB when an installation is removed from GitHub.
Also, the env var
GITHUB_WEBHOOK_SECRET
was added to ECS task definition web secrets. This env var is not used by the tasks executed by the task processor. It is only used by the GitHub webhook.Once we generate the
![image](https://private-user-images.githubusercontent.com/41410593/329141713-9178cf49-5e91-4703-9e4a-1331a30a46d1.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkzNTc1NTEsIm5iZiI6MTczOTM1NzI1MSwicGF0aCI6Ii80MTQxMDU5My8zMjkxNDE3MTMtOTE3OGNmNDktNWU5MS00NzAzLTllNGEtMTMzMWEzMGE0NmQxLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTIlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjEyVDEwNDczMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTVlY2MwMGEwY2Q2ZmFhNmU4NjRlZmQ2N2QwNDkyMzA5ZTM5ZjVkOTViZmVmOTNkYjZlZDdmYWFjYWE1NzUwYzQmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.ritg2WEGSdKyo0ldjhsYMnSdGC_FcN4t64gdZwaTyQA)
GITHUB_WEBHOOK_SECRET
we need to update our GitHub App to use it as shown herehttps://github.com/organizations/Flagsmith/settings/apps/flagsmith
How did you test this code?
Unit tests added.