Fix privileged steps in kubernetes

[anbraten]

closes #3537

[qwerty287]

Shouldn't it only possible to use this when the repo is trusted? Where is this checked?

[xoxys]

Yep, it looks like it ignores the entire discussion from the linked PR.

[anbraten]

Step privileged is only set in case its a plugin and in the list of privileged plugins:

https://github.com/woodpecker-ci/woodpecker/blob/75803dba41a0a276af34606f6330fedf944873b9/pipeline/frontend/yaml/compiler/convert.go#L130

Or if the privileged: true option is set.

In case a user tries to set that option without having a trusted repo he will get a linter error from:

https://github.com/woodpecker-ci/woodpecker/blob/b2cfa37682b002360a63b65213e473d290a32e7d/pipeline/frontend/yaml/linter/linter.go#L115

https://github.com/woodpecker-ci/woodpecker/blob/b2cfa37682b002360a63b65213e473d290a32e7d/pipeline/frontend/yaml/linter/linter.go#L153

[qwerty287]

Yes, but there's no linter for securityContext option. You can set that on any step and make the step privileged.

[anbraten]

Setting the security-context / user / group to 0 (root) or fs-group etc is checked here and will only work for privileged steps which can only be allowed by trusted plugins or by setting privileged: true which can only be done if the repo is trusted:

https://github.com/woodpecker-ci/woodpecker/blob/9cc5d0875c9172c0c7ffd09e150d42bed94c9e3d/pipeline/backend/kubernetes/pod.go#L391

https://github.com/woodpecker-ci/woodpecker/blob/9cc5d0875c9172c0c7ffd09e150d42bed94c9e3d/pipeline/backend/kubernetes/pod.go#L445

[anbraten]

Yep, it looks like it ignores the entire discussion from the linked PR.

It solves the issue that the woodpeckerci/plugin-docker-buildx plugin can only be used if a securityContext is explicitly configured. For already privileged steps (trusted plugins) this wont be necessary anymore now.

[qwerty287]

And this is the only option that could be dangerous?

[anbraten]

At least those are the options we have and the checks we do:

https://github.com/woodpecker-ci/woodpecker/blob/b08133bb63a9bec766f0225e0fb20bfacff1b3de/pipeline/backend/kubernetes/pod.go#L389-L409

[anbraten]

My goal was to simply fix the actual issue #3537 first 🤷🏾 . Changing the logic to require a trusted repo for the buildx plugin is somehow a different change IMO (at least that's how the docker backend works and the docs explain it to the users) .

Requiring a trusted repo to manually set the securityContext could be done separately.

[qwerty287]

Thanks, yes, in general it should work like docker. However, users should not be allowed to set dangerous options without the repo being trusted. Maybe we can merge this as a hotfix so we can finally release 2.5.0 and revisit this whole topic again? I'm not really in the discussion of this issue so I can't really tell you whether this solution was discussed already and rejected or whatever. Maybe @xoxys you can tell us more about k8s and this discussion I think?

[xoxys]

I would suggest to involve the people that have created, contributed and tested the original PR. This entire discussion should have been done in the existing PR instead of just starting a new one...

[qwerty287]

What about this now? Would it be fine to get it merged as hotfix and then rework the whole thing again? I really would like to get 2.5.0 out.

[HannesDampft]

I would like to briefly share my perspective as a user of the Kubernetes backend. Recently, I had to downgrade to version 2.3 because it is not feasible to ask every user to adapt to a bug in Woodpecker that will eventually be reverted.

If this PR is skipped in the release, most Kubernetes users would likely have to skip this release, which would be unfortunate.

I sincerely appreciate the inclusion of this PR in the next release, as I expect most of us users would.

[anbraten]

Thanks for approving.

Please use a new discussion to revert these changes and make it how it should work ideally, but the bug should be fixed asap.

We can just move on with #3538 as this is part of it.