Extend approval options

[anbraten]

related to #336

To protect secrets and (agents) it should be possible to require approvals for potential harmful pipelines like PRs. To not require every pipeline to be approved the existing gated mode was replaced with 3 options addressing most common cases:

• allow every pipeline except PRs from forks
• require approval for all PRs (new default)
• require approval for every pipeline (current require approval / gated mode)

Those modes could later on be extended. Additionally a webhook extension could be added to get the approval state from an external service.

Changes

• replace IsGated with RequireApproval
• migrate old settings to RequireApproval
• adjust docs

[woodpecker-bot]

Deployment of preview was torn down

[qwerty287]

Actually it seems a quite small list of options could solve most common cases as well:

In my opinion, we should support more here.

For a lot of different ideas here see #2293.
My main point is: You can use the approve feature also as a feature if you don't want to run all pipelines to save resources. Having as many options as possible could help here.

Can't you create an interface which just checks whether the pipeline needs approval or not so we can easily extend the feature by adding a new implementation? Maybe also with support for an external http extension?

[xoxys]

Can't you create an interface which just checks whether the pipeline needs approval or not so we can easily extend the feature by adding a new implementation? Maybe also with support for an external http extension?

If we need that level of flexibility, why not use something like open policy agent, as proposed in the issue? Approval rules can be written by users via the UI or CLI by repo admins without writing external services and without blowing up the server code for every single use case that we have not yet thought about.

[6543]

i still want to have the gated feature to be extendable

[xoxys]

Yes, please... if we dont want to have more options in woodpecker core (I still dont understand why) please provide an http interface like drone ci does so users can implement validation.

[xoxys]

I'm confused about the milestone. Is this one targeted for v3 now or 2.8? And if the target for v3 is correct, why is there a backport-required label also present? See #2142 (reply in thread)

[6543]

@anbraten I think you can merge it any time now :)

[6543]

conflicts :/

[6543]

please backport :)

[xoxys]

please backport :)

This is IMO pretty bad practice. If we merge the PR from someone else, we can't expect that the other one will be able to create the backport soon.

[xoxys]

@6543 See discussion in #3348 (comment)

Migrating a public, not gated repo to RequiredForkApproval is by definition a breaking change as it changes the behavior for users. Yes, you can argue that for security reasons we accept this tradeoff, but then this should get a pretty clear note in the release notes and proper documentation.