Skip to content
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

Custom properties are applied during pull request #734

Open
wernerb opened this issue Jan 7, 2025 · 6 comments
Open

Custom properties are applied during pull request #734

wernerb opened this issue Jan 7, 2025 · 6 comments
Labels
bug Something isn't working

Comments

@wernerb
Copy link

wernerb commented Jan 7, 2025

Problem Description

When in PR mode, adding a custom property results in the property added before being merged into main.

What is actually happening

Can see the changes in the PR, once i did a different unrelated change, i could see autolinks still waiting to be applied but the custom properties were applied.

What is the expected behavior

Custom properties are only applied in main

Error output, if available


Context

Are you using the hosted instance of probot/settings or running your own?

Running our own in PR mode

If running your own instance, are you using it with github.com or GitHub Enterprise?

Github.com

Version of probot/settings

latest

Version of GitHub Enterprise

@wernerb wernerb added the bug Something isn't working label Jan 7, 2025
@decyjphr
Copy link
Collaborator

decyjphr commented Jan 8, 2025

The custom_properties plugin doesn't have the code for nop flag and so it applies the change. It would be a pretty simple fix to resolve this.

@wernerb
Copy link
Author

wernerb commented Jan 9, 2025

The same problem is also with environments unfortunately. These are both from a security perspective, critical components as we use custom properties for targetting rulesets being applied now on branches.

@wernerb
Copy link
Author

wernerb commented Jan 9, 2025

Is then dry-run is effectively unsupported for various plugins?
The documentation says that:

The settings in the default branch are applied. If the settings are changed on a non-default branch and a PR is created to merge the changes, the app runs in a dry-run mode to evaluate and validate the changes. Checks pass or fail based on the dry-run results.

Which led me to believe all changes will be dry-run on PR.

@PendaGTP
Copy link
Contributor

PendaGTP commented Jan 9, 2025

The custom_properties plugin doesn't have the code for nop flag and so it applies the change. It would be a pretty simple fix to resolve this.

@decyjphr I would be happy to contribute on this fix if you're interested. I could prepare and open a PR today or in the next few days.

@decyjphr
Copy link
Collaborator

decyjphr commented Jan 9, 2025

That'd be great. Thanks @PendaGTP

@PendaGTP
Copy link
Contributor

PendaGTP commented Jan 9, 2025

@wernerb

Is then dry-run is effectively unsupported for various plugins? The documentation says that:

The settings in the default branch are applied. If the settings are changed on a non-default branch and a PR is created to merge the changes, the app runs in a dry-run mode to evaluate and validate the changes. Checks pass or fail based on the dry-run results.

Which led me to believe all changes will be dry-run on PR.

Based on my experience reviewing the codebase, the dry-run/nop mode support varies across plugins:

Currently supported:

  • Autolinks
  • Branches
  • Collaborators
  • Issue labels
  • Repository
  • Rulesets
  • Teams
  • Repository name validator

Not supported:

  • Custom properties
  • Milestones

Partially supported:

  • Environments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants