-
Notifications
You must be signed in to change notification settings - Fork 632
ci: Automatically push updates to stubs to the current branch #4802
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Chad Dombrova <[email protected]>
Signed-off-by: Chad Dombrova <[email protected]>
I'm not sure I understand the security implications of this approach. |
73b70a4
to
271b979
Compare
I'm not totally sure either, tbh. Once I get it working I'll explore finer grained permissions, unless you'd like me to stop. This approach was inspired by some feedback from @zachlewis over at AcademySoftwareFoundation/OpenColorIO#2162 (comment) I don't have permissions to run any CI in that repo, so I thought I'd test it out here. |
ab54707
to
a8e964b
Compare
92312c8
to
945e8d4
Compare
Signed-off-by: Chad Dombrova <[email protected]>
Status update. My goal is to push updates to the pull request branch in my fork. I got pretty close to making this work, but I think I've reached an impasse. First, I configured the I'm using https://github.com/stefanzweifel/git-auto-commit-action action to auto-commit-and-push changes, and it's doing what I expect -- pushing to my fork and branch -- but it fails with a permissions issue:
The PR is set to allow pushes from maintainers, but apparently the actions bot does not count. I'm not sure how to get past this, but it might require me to setup a personal access token, which would make this workflow prohibitively cumbersome. Some alternatives to pursue:
|
@chadrik, I'm still a little nervous about a fully automatic push to the PR author's branch (50% - will they not like that; 50% - is there any way somebody can abuse it to push things to our own repo before a merge?). But in this Slack conversation, @JeanChristopheMorinPerso outlines a way where, after a PR fails for this particular reason, we could add a tag that would run a special "fix stubs" workflow that would do the fix, push the branch to the author to amend the PR, and then remove the tag. Not sure how you feel about that approach, or if you fully understand how to do it (I get it conceptually but would probably have to do a lot of documentation reading and trial-and-error to know how to implement it, because I'm a workflow newbie), but it feels to me like it's inherently safer to require that minor manual step to kick off the fixing and pushing, rather than it being completely automatic and irreversible simply upon receipt of the PR. OTOH, if @JeanChristopheMorinPerso is not concerned about the security of the "automatic" approach (he both knows more than me about this and seems pretty conservative about it), then I guess I'm willing to go along with that if nobody voices a concern. |
Yes. The GHA token has relatively limited permissions and it can't impersonate you. One way to solve this is to use a PAT like you said. Another way is to create an app, grant it appropriate permissions and I think that would remove some maintenance burden (since the app token would never expire, as opposed to a PAT). But that requires the use of |
# commit_message: "Automatic update to python stubs" | ||
# # if: failure() | ||
|
||
- uses: parkerbxyz/[email protected] |
Check warning
Code scanning / Scorecard
Pinned-Dependencies Medium
Click Remediation section below to solve this issue
Signed-off-by: Chad Dombrova <[email protected]>
Description
This simplifies developer workflow by automatically pushing freshly built stubs to the current branch for review.
Tests
Checklist:
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.