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

schema: add support for tmpfs.mode in mount definition #10031

Merged
merged 1 commit into from Dec 2, 2022

Conversation

milas
Copy link
Member

@milas milas commented Nov 30, 2022

⚠️ Draft until compose-spec/compose-go#325 is merged & tagged

What I did
Add support for tmpfs.mode to set permissions on tmpfs mounts

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did
prairie chicken

@milas milas added kind/bug area/run area/up area/volumes dependencies Pull requests that update a dependency file labels Nov 30, 2022
@milas milas requested a review from a team November 30, 2022 19:34
@milas milas self-assigned this Nov 30, 2022
@milas milas requested review from nicksieger, ndeloof, StefanScherer, ulyssessouza, glours and laurazard and removed request for a team November 30, 2022 19:34
@thaJeztah
Copy link
Member

we may have to look if mode is fully functional as well; I recall this ticket, and I don't think we dug to the end of that one yet (at least I haven't found time to dig into it myself);

@milas
Copy link
Member Author

milas commented Nov 30, 2022

we may have to look if mode is fully functional as well

Thanks for sharing that! I think it's still fine to add here unless we're planning on deprecating tmpfs support entirely - once/if the upstream bug is fixed, nested permissions should start just working in Compose after upgrading engine. For now, this will at least give parity in that permissions on unnested tmpfs mounts are now supported

@thaJeztah
Copy link
Member

Thanks for sharing that! I think it's still fine to add here unless we're planning on deprecating tmpfs support entirely - once/if the upstream bug is fixed, nested permissions should start just working in Compose after upgrading engine. For now, this will at least give parity in that permissions on unnested tmpfs mounts are now supported

Yup, change looks okay to me; mostly recalled that there was an issue around this option, and thought I'd mention it to set the expectations right 😄. Trying / testing /digging on that linked issue is always welcome as well (I created that ticket to have it at least written down somewhere as a reminder)

@milas looks like this needs a rebase now (conflict in go.sum)

@milas
Copy link
Member Author

milas commented Nov 30, 2022

Yeah, it's in draft as it needs the updated schema code from compose-go so will be fixing that when I remove my replaces

See compose-spec/compose-go#325 for the acutal spec change. This
propagates it to the Engine API object and adds an E2E test via
Cucumber 🥒

Fixes docker#9873.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@milas milas marked this pull request as ready for review December 2, 2022 15:57
@milas milas merged commit 113fb67 into docker:v2 Dec 2, 2022
@milas milas deleted the tmpfs-mode branch December 2, 2022 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/run area/up area/volumes dependencies Pull requests that update a dependency file kind/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants