rocker-org / devcontainer-features

A collection of Dev Container Features.
https://rocker-project.org/images/devcontainer/features.html
MIT License
52 stars 16 forks source link

renv cache directory not owned by remote uid/gid #209

Closed ahaim5357 closed 10 months ago

ahaim5357 commented 10 months ago

When attempting to run the renv-cache feature, the directory created /renv/cache is set to the default user (in this case 1000:1000). However, when updateRemoteUserUID is true, the UID of the cache location isn't updated.

My current understanding is that dev containers and their features when setting ownership is to a non-root user specified by the docker container. Then, as a final postprocessing step, everything within the current workspace for the user is chown to the remote uid/gid. Since /renv/cache wouldn't be in the workspace directory, it does not get updated to the remote ownership.

I'm not sure whether this is a bad setup, intended design by dev container features or an oversight in some part of the pipeline, so I am mentioning the issue here to start.

This was tested in VSCode.

Remote (Local) User UID/GID -> 1001:1001

The devcontainer.json

{
    "name": "R (rocker/r-base)",
    "image": "ghcr.io/rocker-org/devcontainer/r-ver:4.3",
    "features": {
        "ghcr.io/rocker-org/devcontainer-features/renv-cache:latest": {},
        "ghcr.io/rocker-org/devcontainer-features/r-packages:1": {
            "packages": "github::rstudio/renv@v1.0.2"
        }
    },

    "remoteUser": "rstudio",
    "updateRemoteUserUID": true,
}

Running ls -al $RENV_PATHS_CACHE in bash:

total 8
drwxr-xr-x 2 1000 1000 4096 Jan 22 22:33 .
drwxr-xr-x 3 root root 4096 Jan 22 22:33 ..

1000 should say rstudio which has a current UID/GID of 1001:1001

eitsupi commented 10 months ago

Thanks for your report! I did not know about updateRemoteUserUID. Certainly this looks like a bug. Perhaps a lifecycle script needs to be used to do the chown? Like:

https://github.com/rocker-org/devcontainer-features/blob/f9e426dcd910923c678639a2be4514f51af2650d/src/r-history/oncreate.sh#L5-L50

If you would like, please try the r-history Feature. If this works, we can fix it the same way.

ahaim5357 commented 10 months ago

It seems like the lifecycle script fixes the issue.

Running ls -al /dc/r-history results in:

total 8
drwxr-xr-x 2 rstudio rstudio 4096 Jan 23 15:31 .
drwxr-xr-x 3 root    root    4096 Jan 23 15:31 ..

Which is expected.

eitsupi commented 10 months ago

Thanks!

Because the renv-cache Feature is intended to share one volume among multiple containers, maybe postStartCommand is useful. Unlike onCreateCommand, it is executed each time a container is started, so it can be undone even if the permission is updated by another container.

If you don't mind, could you please create a PR for this? If you read the r-history source code, it would be fairly easy to implement the same.

ahaim5357 commented 10 months ago

Sure, give me a moment.

ahaim5357 commented 10 months ago

Okay, done. I was having an issue getting the postscript copied over until I realized that check_r just exits the installation script. So now, everything should work and the test cases pass locally.

I'm unsure how to write the workflow to use a different uid/gid to test through github actions, so I just left it alone.