Allow default clone step to run unprivileged#6312
Conversation
|
have you considered local backend? and other slef impl. clome steps of users likely break ... but i'll infestigate this idea |
|
@6543 I didn't consider the local backend tbh. I only considered the Docker and Kubernetes backends. I'll try tomorrow with local backend and see if everything works as expected. That said, I don't believe this poses any problem to other backends, as the pluginWorkspaceBase directory is already assumed to exist by the existing code. From the perspective of the plugin-git binary, this only changes In both scenarios /$a is assumed to exist, and the final cloned path is always /$a/$b. The backend is also assumed to be able to create /$a/$b, so it should have write privileges for /$a Another approach I initially considered that would be more self-contained to the kubernetes backend was using an initContainer in the pod to precreate the actual container workingDir. I discarded it after working a bit on it because it was too much code that could be simplified by just updating the default clone step working directory and target. Regarding the |
|
I would prefer your alternative approach from the other PR. |
|
#6322 got merged |
Currently, the default clone step requires root privileges to run in Kubernetes (and probably on other container based runtimes).
As explained in #5346 (comment):
This PR fixes the issue by setting the working directory of the default clone step to
pluginWorkspaceBase, which should exists as it's the volume the agent mounts for the container by default.In addition to that, to preserve the same behavior as before, it sets the
targetoption ofplugin-gittopath.Join(pluginWorkspaceBase, c.workspacePath), so that the git repo contents are cloned to the same directory as before. Theplugin-gitimage already does amkdirop if the target directory doesn't exists. In this case the directory gets created with the correct permissions and ownership, as opposed to what happens when we let the container runtime create it.Finally, it sets the
homeoption ofplugin-gitto/tmp, which is a directory that already exists in Alpine images and is writable by everybody. I'm unsure wether thishomefix needs to be here or is better to directly fix it in theplugin-gitimage itself.This PR, in addition to #6307 and #6310, should close #5346.
While both PRs need to be merged to properly close #5346, this PR changes are self-contained and don't require either PR to be merged first.