Repository navigation
Can't get PR evaluation to run in dry-run mode (run via GitHub actions) #739
Description
Problem Description
We can't get safe-settings to run in dry-run (nop?) mode when evaluating pull requests on topic branch, run via GitHub actions. It always enforces the rules for both github triggers pull_request and push from main branch even though on topic branch.
Are we missing a setting?
We are on rc version 2.1.15-rc.1, feel free to discard this issue if we are reporting it premature. We are ready to wait for future versions, I just wanted to report this back. Great work by the way! 👍
What is actually happening
We see that the settings evaluated are not from the topic branch (they are from main branch) and that the debug statements display that it is not run in nop mode. Any changes are enforced from main branch even though it's evaluation intended.
It's as if it doesn't recognize that it should be run in dry-run (nop) mode and picks up the topic branch name.
What is the expected behavior
That when run with event trigger is pull_request that the settings evaluated will be from the topic branch (not main branch) and that the changes on topic branch ARE NOT enforced, just gets output on the PR.
Error output, if available
The log from full-sync.js when evaluating a pull request with LOG_LEVEL: debug displays that
a) The GET statement doesn't invoke the GitHub api for topic branch, but just main branch. I would expect that it tried to look the file from ?ref=topic-branch or something like that
DEBUG (github): GitHub request: GET https://api.github.com/repos/OurOrg/our-repo/contents/safe-settings%2Frepos%test-repo.yml - 200
b) The output from probat displays it's not run in nop:
DEBUG (probot): Not run in nop
Context
Running on GitHub Enterprise. Running safe-settings via GitHub Action. Run with pull_request trigger.
on:
# PR - Pull request trigger
pull_request:Run with settings mentioned in example file. Almost as example. The admin repo has a custom name.
SAFE_SETTINGS_VERSION: 2.1.15-rc.1
...
- run: npm run full-sync
working-directory: ${{env.SAFE_SETTINGS_CODE_DIR}}
env:
# Configuration files placed in safe-settings-folder in the ADMIN_REPO
ADMIN_REPO: our-admin-repo
CONFIG_PATH: safe-settings
SETTINGS_PATH: ${{github.workspace}}/safe-settings/settings.yml
DEPLOYMENT_CONFIG_FILE: ${{github.workspace}}/safe-settings/deployment-settings.yml
# GitHub App settings
GH_ORG: ${{vars.SAFE_SETTINGS_GH_ORG}}
APP_ID: ${{vars.SAFE_SETTINGS_APP_ID}}
PRIVATE_KEY: ${{secrets.SAFE_SETTINGS_PRIVATE_KEY}}
GITHUB_CLIENT_ID: ${{vars.SAFE_SETTINGS_GITHUB_CLIENT_ID}}
GITHUB_CLIENT_SECRET: ${{secrets.SAFE_SETTINGS_GITHUB_CLIENT_SECRET}}
# Logging
LOG_LEVEL: debug # info, trace, debug
Activity
sumokadet commentedon Jan 10, 2025
I can't share own run since it's in a private organization, but it's identical to what we see here (PR evaluation on branch),
2.1.14:(the call is without
ref=branchparam, hence the lookup in on main branch)(not run in nop)
PendaGTP commentedon Jan 10, 2025
@sumokadet
Regarding the issue where safe-settings doesn't execute in nop (dry-run) mode (when running
full-sync), you can track the progress through these links:PendaGTP commentedon Jan 10, 2025
@decyjphr - Regarding the issue with the
full-syncscript always evaluating settings from the default branch:I'd be happy to contribute to fixing this. We can we start by creating a dedicated issue to discuss the implementation details, since we currently can't retrieve the context in the same way as with event handlers.
My initial proposal would be to add a new environment variable called
FULL_SYNC_FROM_REF(defaulting to the default branch). This could be easily configured in CI environments.@sumokadet - Would adding an environment variable to specify which ref to use for settings meet your needs?
Note: This change would also require addressing the
nopmode in full-sync (related to #733).Please let me know your thoughts on this approach.
P.S. @decyjphr - I realize I might be tagging you too frequently. Please let me know if this is bothersome - I'm still learning how best to collaborate in open source and want to be respectful of everyone's notifications.
sumokadet commentedon Jan 13, 2025
@PendaGTP - Thanks and kudos for quick response! The combination of input env vars
FULL_SYNC_NOOP(dry-run) (from #733) andFULL_SYNC_FROM_REF(or namedFULL_SYNC_GIT_REF) (which ref to get settings from) will be good for our needs.From a "user perspective" in a workflow it could probably look a little bit like this. It would be nice if
FULL_SYNC_FROM_REFdefaults to same value as${{ github.ref }}so it both works when the var is mentioned and when it is not provided (should be default branch).PendaGTP commentedon Jan 18, 2025
@sumokadet
Thanks for your feedback!
I think one thing that could also help from a user perspective would be to create a GitHub Action. This way, the user would only need to provide the necessary inputs and the action would take care of the rest logic/setup.
Any thoughts on this approach? I'd be happy to create an issue to discuss this further and potentially submit a PR with a GHA implementation if this seems like a helpful direction to pursue.
decyjphr commentedon Jan 20, 2025
@sumokadet @PendaGTP Another way we could make this work is using the feature in
probotto simulate the app getting a webhook event.So this would be basically having a GHA in the
adminrepo which will listen to push and and PR events and then call the app simulating the webhook and the payload.PendaGTP commentedon Jan 20, 2025
@decyjphr Thank you for your feedback. This approach could make the handling more consistent with our event processing. I'll review our current logic around nop, checks, and PR change previews to better understand the current workflow.
sumokadet commentedon Jan 27, 2025
@PendaGTP regarding "Creating a wrapper action" #739 (comment)
Good idea with a separate issue. An action would be nice in the future.
However, I'm a bit in doubt about if the users of safe-settings (which I believe are super-users/administrative types) will go to the action first. I assume they need to deep dive into how this safe-settings works since it potentially alters all repos in their organization, hence they maybe want to alter how it's called. Anyways, recommendations in documentation for this should be added.
powerman commentedon Jun 15, 2025
No matter what we are using at work, we also have personal repos!
I've own server and run a dozen of docker containers there with different services, but even to me it feels too annoying to add another one with safe-settings just to setup settings for some personal repos in manageable way.
So, using safe-settings as an action looks like the best of available possibilities for quick personal use.
Absent support for webhooks is okay, but missing PR checks/comments/dry-run feature is not so good.