-
Notifications
You must be signed in to change notification settings - Fork 50
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
MQL Mimic: Change to PR CI, detect changes #828
Conversation
7de80dd
to
45bb429
Compare
098facd
to
8208499
Compare
2ece166
to
a7afd9d
Compare
6ca67c8
to
d8a1693
Compare
7467890
to
5a8ae96
Compare
5c41f15
to
8145085
Compare
- name: "Trigger MQL Mimic Tests" | ||
env: | ||
trigger_url: '${{ secrets.MQL_MOCK_TRIGGER }}' | ||
branch: ${{ github.event_name == 'pull_request_target' && github.head_ref || github.ref }} |
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.
On main
/etc github.ref
will include "refs/heads/". I decided to just handle that downstream.
This reverts commit dc4dbf3.
@@ -30,6 +30,7 @@ jobs: | |||
with: | |||
ref: ${{ github.head_ref }} | |||
repository: ${{ github.event.pull_request.head.repo.full_name }} | |||
depth: 0 |
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.
This is what's recommended for change detection. It takes 1 second to checkout, so I don't think this is worth even testing the alternatives (which are some higher number, or maybe API based detection but it's unclear if that's compatible with pull_request_target
workflows)
rule_id=$(yq '.id' $file) | ||
|
||
echo "$file has rule ID $rule_id" | ||
altered_rule_ids=$(echo "$rule_id"" ""$altered_rule_ids") |
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.
This is so cursed "$rule_id"" ""$altered_rule_ids"
but I don't know what else I expect from bash.
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.
Haha "$rule_id $altered_rule_ids" should work too but I'll extra
"` wrap often in case I'm forgetting some expansion that's going to happen.
The other side of these changes isn't finished out, but this part appears to be working as needed:
Once the other pieces are in place, we'll only run MQL mimic tests for detection rules touched in a PR. The endpoint will always be hit, but the workflow on the other side will succeed pretty quickly (it doesn't need to setup the platform or pull its large source).
e2abbf6 shows this running on push (as it will on
main
)#829 shows it working as it will on a PR.