Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

genpolicy: add bind mounts for image volumes #10136

Merged

Conversation

danmihai1
Copy link
Member

@danmihai1 danmihai1 commented Aug 7, 2024

Add bind mounts for volumes defined by docker container images, unless those mounts have been defined in the input K8s YAML file too.

For example, quay.io/opstree/redis defines two mounts:
/data
/node-conf
Before these changes, if these mounts were not defined in the YAML file too, the auto-generated policy did not allow this container image to start.

@katacontainersbot katacontainersbot added the size/large Task of significant size label Aug 7, 2024
@katexochen
Copy link
Contributor

Thanks Dan, that's an issue I recently ran into. Afaik containerd will try to mount a tmpfs in case the the volume isn't specified in the YAML. I was wondering if we could allow this mount of a tmpfs at that location in case the volume isn't specified instead of requiring the volume to be explicitly defined. It should be safe as the tmpfs is in the guest, so whatever we write there won't be accessible by the host. Also, I think is quite a common case (the redis image defines such VOLUME, for example) and many users will likely run into it often.

@danmihai1 danmihai1 force-pushed the danmihai1/docker-image-volume2 branch from 8d3f01b to d8c035f Compare August 13, 2024 15:06
@danmihai1 danmihai1 changed the title genpolicy: require definition for image volume mounts genpolicy: add bind mounts for image volumes Aug 13, 2024
@danmihai1
Copy link
Member Author

Thanks Dan, that's an issue I recently ran into. Afaik containerd will try to mount a tmpfs in case the the volume isn't specified in the YAML. I was wondering if we could allow this mount of a tmpfs at that location in case the volume isn't specified instead of requiring the volume to be explicitly defined. It should be safe as the tmpfs is in the guest, so whatever we write there won't be accessible by the host. Also, I think is quite a common case (the redis image defines such VOLUME, for example) and many users will likely run into it often.

Good to know - thanks @katexochen ! I made that genpolicy change and added a test for redis.

@danmihai1 danmihai1 force-pushed the danmihai1/docker-image-volume2 branch from d8c035f to ce32a03 Compare August 13, 2024 20:46
@danmihai1 danmihai1 force-pushed the danmihai1/docker-image-volume2 branch 2 times, most recently from f2c13e7 to e337976 Compare August 15, 2024 23:08
Add bind mounts for volumes defined by docker container images, unless
those mounts have been defined in the input K8s YAML file too.

For example, quay.io/opstree/redis defines two mounts:
/data
/node-conf
Before these changes, if these mounts were not defined in the YAML file
too, the auto-generated policy did not allow this container image to
start.

Signed-off-by: Dan Mihai <[email protected]>
@danmihai1 danmihai1 force-pushed the danmihai1/docker-image-volume2 branch from e337976 to c22ac4f Compare August 16, 2024 15:11
@danmihai1 danmihai1 merged commit 79c1d0a into kata-containers:main Aug 16, 2024
290 of 300 checks passed
@danmihai1 danmihai1 deleted the danmihai1/docker-image-volume2 branch October 8, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants