-
Notifications
You must be signed in to change notification settings - Fork 133
Implement '/lgtm' ChatOps Workflow. #612
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
Changes from 3 commits
6692afe
71a218e
4203f6c
077f8de
85a7d53
bf39544
bb36230
60cb0ff
2f3dbb7
a7fb2b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| name: ChatOps Dispatcher | ||
| on: | ||
| issue_comment: | ||
| types: [created] | ||
|
|
||
| jobs: | ||
| dispatch: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Slash Command Dispatch | ||
| uses: peter-evans/slash-command-dispatch@v3 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| commands: lgtm | ||
| # 1. Basic Security: Only allow users with 'write' access to even trigger this | ||
| permission: write | ||
| issue-type: pull-request | ||
| reactions: false |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,149 @@ | ||||||
| # ============================================================================ | ||||||
| # LGTM Command Worker | ||||||
| # ============================================================================ | ||||||
| # Handles /lgtm commands from chatops-dispatcher | ||||||
| # | ||||||
| # Flow: | ||||||
| # 1. User comments /lgtm on PR | ||||||
| # 2. chatops-dispatcher catches it and dispatches here | ||||||
| # 3. This workflow: | ||||||
| # - Verifies user is in OWNERS file | ||||||
| # - Checks if PR is draft | ||||||
| # - Checks for blocking labels (hold/wip/do-not-merge) | ||||||
| # - Adds lgtm label | ||||||
| # - Waits 5 minutes (allows gatekeeper to run) | ||||||
| # - Enables auto-merge | ||||||
| # ============================================================================ | ||||||
|
|
||||||
| name: LGTM Command Worker | ||||||
| on: | ||||||
| repository_dispatch: | ||||||
| types: [lgtm-command] | ||||||
|
|
||||||
| jobs: | ||||||
| apply-lgtm: | ||||||
| runs-on: ubuntu-latest | ||||||
| permissions: | ||||||
| contents: write | ||||||
| pull-requests: write | ||||||
| issues: write | ||||||
| steps: | ||||||
| - uses: actions/checkout@v4 | ||||||
|
|
||||||
| # ----------------------------------------------------------------------- | ||||||
| # STEP 1: AUTHORIZATION - Verify user is in OWNERS file | ||||||
| # ----------------------------------------------------------------------- | ||||||
| # Only users listed as approvers in OWNERS can use /lgtm | ||||||
| # This prevents unauthorized users from approving PRs | ||||||
| - name: Check Permissions | ||||||
| id: check | ||||||
| env: | ||||||
| ACTOR: ${{ github.event.client_payload.github.actor }} | ||||||
| run: | | ||||||
| if grep -q "^\s*-\s*$ACTOR\s*$" OWNERS; then | ||||||
| echo "authorized=true" >> $GITHUB_OUTPUT | ||||||
| else | ||||||
| echo "::error::User $ACTOR is not an approver." | ||||||
|
||||||
| echo "::error::User $ACTOR is not an approver." | |
| echo "::error:: User $ACTOR is not an approver." |
nit: adding space after ::error:: could make it easier to read.
If you decide to change, please change the same format across all error messages.
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.
Thanks will change.
Outdated
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.
is there a way to bail out in an earlier step (when not authorized) instead of repeating the check in all steps?
if the repetition is required than steps following this should also check that the PR is not in draft, etc.
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 will look into that. Thanks
Outdated
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.
nit: consider making these a variable at the top of the step for easier changed in the future
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.
Sure will do that Thanks
Outdated
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.
Q: how do we issue and maintain a bot token? who owns it (e.g., user associated, expiration time, ...)
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 need to investigate how to properly configure a Personal Access Token (PAT) for this repository.
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've updated the workflows to use a GitHub App via tibdex/github-app-token@v1, which generates short-lived access tokens automatically. This eliminates the need for a dedicated, long-lived Personal Access Token (PAT) and ensures that bot-triggered events successfully spawn subsequent workflow runs.
Outdated
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.
is the sleep required?
having a job wait for 5m slows down the loop. Is there a way to structure the events so the wait is not needed?
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.
No, its not needed and can be removed. I wanted to give the pr writer or other approver an opportunity to take a final look at the checks or the code before the merge sequence officially started when the testing is finishes quickly. I will remove it.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # ============================================================================ | ||
| # LGTM Gatekeeper - Required Status Check | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: there seems to be quite a bit of overlap between this file and the previous. Can you explain how they differ and what purpuse they serve.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| # ============================================================================ | ||
| # Rules Enforced: | ||
| # 1. PR MUST have "lgtm" label | ||
| # 2. PR MUST NOT have blocking labels (hold, wip, do-not-merge) | ||
| # ============================================================================ | ||
|
|
||
| name: LGTM Gatekeeper | ||
| on: | ||
| pull_request: | ||
| # Run on PR open/reopen and label changes | ||
| # NOT on synchronize (handled by lgtm-reset.yml) | ||
| types: [opened, labeled, unlabeled, reopened] | ||
|
|
||
| jobs: | ||
| validate-pr: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Enforce LGTM & Blockers | ||
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| REPO: ${{ github.repository }} | ||
| run: | | ||
| # Fetch current labels | ||
| LABELS=$(gh pr view $PR_NUMBER --repo "$REPO" --json labels --jq '.labels[].name') | ||
|
|
||
| # Check 1: IS IT BLOCKED? | ||
| if echo "$LABELS" | grep -Eiq "^(hold|wip|do-not-merge)$"; then | ||
| echo "::error::⛔ FAILED: PR is blocked by a 'hold', 'wip', or 'do-not-merge' label." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Check 2: IS IT APPROVED? | ||
| # If Reset workflow removed the label, this check fails immediately. | ||
| if ! echo "$LABELS" | grep -Fqx "lgtm"; then | ||
| echo "::error::⛔ FAILED: PR is missing the 'lgtm' label." | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✅ PASSED: LGTM present and no blockers." | ||
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # ============================================================================ | ||
| # LGTM Reset - Auto-Remove LGTM on New Commits | ||
| # ============================================================================ | ||
| # Kubernetes Prow behavior: When new commits are pushed, approval is invalidated | ||
| # | ||
| # What It Does: | ||
| # 1. Detects when new commits are pushed to a PR | ||
| # 2. Removes the "lgtm" label (if present) | ||
| # 3. Disables auto-merge (safety net) | ||
| # 4. Posts a comment explaining why | ||
| # ============================================================================ | ||
|
|
||
| name: LGTM Reset | ||
| on: | ||
| pull_request: | ||
| types: [synchronize] # Triggers instantly on new commits | ||
|
|
||
| jobs: | ||
| reset-lgtm: | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| pull-requests: write | ||
| steps: | ||
| - name: Invalidate LGTM | ||
| env: | ||
| GH_TOKEN: ${{ secrets.BOT_TOKEN }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| REPO: ${{ github.repository }} | ||
| run: | | ||
| echo "🚨 New code pushed. Resetting LGTM status..." | ||
|
|
||
| # 1. Remove the label (This triggers the Gatekeeper to run again) | ||
| gh pr edit $PR_NUMBER --repo "$REPO" --remove-label "lgtm" || true | ||
|
|
||
| # 2. Disable Auto-Merge (Safety net) | ||
| gh pr merge --disable-auto $PR_NUMBER --repo "$REPO" || true | ||
|
|
||
| # 3. Notify user | ||
| gh issue comment $PR_NUMBER --repo "$REPO" --body "🔄 **Reset**: New commits pushed. LGTM removed." | ||
|
||
This file was deleted.
This file was deleted.

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 checks the entire OWNERS file?
I think there are different groups in it so this should only be from reviewers or maintainers?
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.
It should! It looks like that block was accidentally removed during a previous refactoring cycle. I missed it, but I will restore it now. Thanks for catching that.