Skip to content

ci(e2e): use ok-to-test action for permission checks#2628

Merged
theakshaypant merged 1 commit intotektoncd:mainfrom
zakisk:use-permission-check-from-ok-to-test-repo
Apr 2, 2026
Merged

ci(e2e): use ok-to-test action for permission checks#2628
theakshaypant merged 1 commit intotektoncd:mainfrom
zakisk:use-permission-check-from-ok-to-test-repo

Conversation

@zakisk
Copy link
Copy Markdown
Member

@zakisk zakisk commented Mar 31, 2026

Replace inline permission check script with the standalone pipelines-as-code/ok-to-test GitHub Action.

Assisted-by: Claude Opus 4.6 (via Claude Code)

📝 Description of the Change

🔗 Linked GitHub Issue

Fixes #

🧪 Testing Strategy

  • Unit tests
  • Integration tests
  • End-to-end tests
  • Manual testing
  • Not Applicable

🤖 AI Assistance

AI assistance can be used for various tasks, such as code generation,
documentation, or testing.

Please indicate whether you have used AI assistance
for this PR and provide details if applicable.

  • I have not used any AI assistance for this PR.
  • I have used AI assistance for this PR.

Important

Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer to your commit message.
For example:

Co-authored-by: Claude noreply@anthropic.com

✅ Submitter Checklist

  • 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
  • ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
  • ♽ I have run make test and make lint locally to check for and fix any
    issues. For an efficient workflow, I have considered installing
    pre-commit and running pre-commit install to
    automate these checks.
  • 📖 I have added or updated documentation for any user-facing changes.
  • 🧪 I have added sufficient unit tests for my code changes.
  • 🎁 I have added end-to-end tests where feasible. See README for more details.
  • 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
  • If adding a provider feature, I have filled in the following and updated the provider documentation:
    • GitHub App
    • GitHub Webhook
    • Gitea/Forgejo
    • GitLab
    • Bitbucket Cloud
    • Bitbucket Data Center

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the script used to verify pull request author permissions for pull_request_target events. The deleted logic included checks for trusted bots, organization and team membership, repository collaborator status, and the ok-to-test label. I have no feedback to provide as the file was entirely removed.

@chmouel
Copy link
Copy Markdown
Member

chmouel commented Mar 31, 2026

🤔

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Mar 31, 2026

🤔

I have moved the actions in a separate repo in pipelines-as-code/ok-to-test

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Mar 31, 2026

you can see it working here zakisk#17

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Mar 31, 2026

but I think e2e tests will always fail as it is using workflow from main

Copy link
Copy Markdown
Member

@theakshaypant theakshaypant left a comment

Choose a reason for hiding this comment

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

Given the potential issues with the script and other options, I personally prefer this over the inline script option.

@zakisk zakisk force-pushed the use-permission-check-from-ok-to-test-repo branch 2 times, most recently from 64daa94 to bc92dcc Compare April 1, 2026 07:00
Copy link
Copy Markdown
Member

@theakshaypant theakshaypant left a comment

Choose a reason for hiding this comment

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

LGTM.
I think we need to hold the merge till we get this as an allowed action in tektoncd. cc @vdemeester

Should link the action in the PR description as well

@zakisk
Copy link
Copy Markdown
Member Author

zakisk commented Apr 1, 2026

/kind ok-to-test

1 similar comment
@infernus01
Copy link
Copy Markdown
Member

/kind ok-to-test

@infernus01
Copy link
Copy Markdown
Member

/ok-to-test

Replace inline permission check script with the standalone
pipelines-as-code/ok-to-test GitHub Action.

Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
Assisted-by: Claude Opus 4.6 (via Claude Code)
@zakisk zakisk force-pushed the use-permission-check-from-ok-to-test-repo branch from f18c448 to 6dbf416 Compare April 2, 2026 06:07
@theakshaypant theakshaypant merged commit dfab680 into tektoncd:main Apr 2, 2026
3 of 12 checks passed
@zakisk zakisk deleted the use-permission-check-from-ok-to-test-repo branch April 2, 2026 06:12
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.

4 participants