Bring back pr actions

[svrnm]

Reverts #6663 with an improved workflow to run fix commands via GitHub comment:

• add hardened runners by StepSecurity
• split the creation of the changes & the application of them into two separate steps to disable the possibility to inject something malicious into the workflow run

@open-telemetry/docs-approvers @trask PTAL!

@theletterf I remember you had some code that would also check for the handle if the person is allowed to trigger the workflow (e.g. only org members, maintainers, etc.), we could add this as well.

NOTE: As always this github action has not been yet tested in full details, so unfortunately we will need to merge it at some point and then iterate on it until it works as expected.

[theletterf]

I like it! For user checks, you'd have to add this to all actions, or similar:

    if: |
      github.event.issue.pull_request &&
      startsWith(github.event.comment.body, '/fix:') &&
      (contains(github.event.comment.user.roles, 'admin') || 
       contains(github.event.comment.user.roles, 'write') || 
       github.event.comment.author_association == 'MEMBER' || 
       github.event.comment.author_association == 'OWNER' || 
       github.event.comment.author_association == 'COLLABORATOR')

[svrnm]

I like it! For user checks, you'd have to add this to all actions, or similar:

I'll try to incorporate that, thanks!

[chalin]

Great! Thanks so much for this @svrnm!

I don't have the time today to review this in detail (I'll be mostly AFK), so I'll let @trask et all work through the details. I'm glad to RSLGTM, get this merged and give it a spin! (I'm also looking forward to study this more closely later.)

[trask]

approving with the two permission updates above

[trask]

@svrnm now that you have a safe way to do this, what do you think of running this (or at least the most common ones?) on every PR build and automatically applying it? (not waiting for someone to add the comment)

[trask]

@svrnm now that you have a safe way to do this, what do you think of running this (or at least the most common ones?) on every PR build and automatically applying it? (not waiting for someone to add the comment)

just saw your slack comment, which makes sense why you don't want to do this:

[applying] updates without any user intervention, which may be OK in a repo that is 90% contributed by the same folks, but we have way to many drive by contributors which will struggle with pushing and pulling and sometimes even don't know have to do git properly

[theletterf]

@trask The rerun action one would likely come after this one!

[svrnm]

@svrnm now that you have a safe way to do this, what do you think of running this (or at least the most common ones?) on every PR build and automatically applying it? (not waiting for someone to add the comment)

just saw your slack comment, which makes sense why you don't want to do this:

[applying] updates without any user intervention, which may be OK in a repo that is 90% contributed by the same folks, but we have way to many drive by contributors which will struggle with pushing and pulling and sometimes even don't know have to do git properly

Yeah, and thinking about it, there is actually another reason why we don't want to do that: some people update their PRs with lots of small commits, so I am not sure how this would behave, i.e. would we have a flooded action pipeline?

[chalin]

@svrnm now that you have a safe way to do this, what do you think of running this (or at least the most common ones?) on every PR build and automatically applying it? (not waiting for someone to add the comment)

If we're aiming to streamline the PR process for 80% the cases, I'd be willing to give this a try. Often, drive-by PRs need either fix:format or a refcache update. If we find that it doesn't make it easier for contributors and us maintainers, we can always pull it back.

Anyhow, let's get this merged first and see if the old /fix:... commands are working again. WDYT?

[trask]

I think the git push below is going to fail, because this is read-only: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#error-failed-to-push-some-refs

[svrnm]

good find! Previously we had a separate step checking out the branch, maybe we need to bring that back

[trask]

heads up, CodeQL is complaining security flaws in my PR where I'm testing out this approach: open-telemetry/opentelemetry-java-instrumentation#13872

I'm working through them now to understand and hopefully address...

[svrnm]

I tested the command in my own repo, it worked for /fix:format, it tripped with /fix:all but that might be related to the drift detection, I also had some issues with npm dependencies&caching, might be related to some things missing in my repo what we have here, so we can revisit that.

[chalin]

@trask - anything left to do before we can merge and test?

[trask]

I think it can be simplified, but have no issue with this being merged as-is

[trask]

I think this simplifies things, because then you can just git push at the end (see below)

Suggested change

	- run: gh pr checkout $PR_NUM -b "pr-action-${RANDOM}"
	- run: gh pr checkout $PR_NUM

[svrnm]

If I remember correctly we introduced this back in the day because we had some issues with branch names that were used on the fork and upstream (think: patch-1 or similar), I could dig up the issue for that

[trask]

this simplified version is working in the (new) Java instrumentation auto apply action: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/.github/workflows/auto-update-pull-request.yml

[trask]

Suggested change

	# gh pr checkout sets some git configs that we can use to make sure
	# we push to the right repo & to the right branch
	remote_repo=$(git config --get branch.${current_branch}.remote)
	echo remote_repo=$remote_repo
	remote_branch=$(git config --get branch.${current_branch}.merge)
	echo remote_branch=$remote_branch
	git commit -m "Results from /fix:${PR_ACTION}"
	git push ${remote_repo} HEAD:${remote_branch}
	# gh pr checkout sets some git configs that we can use to make sure
	# we push to the right repo & to the right branch
	git commit -m "Results from /fix:${PR_ACTION}"
	git push

[chalin]

Merging. @svrnm et al. will follow through with the pending comments via another PR. Thanks all!