Page MenuHomePhabricator

Create directory for common unprivileged app dependencies
ClosedPublic

Authored by dduvall on Aug 23 2017, 5:12 PM.

Details

Reviewers
thcipriani
mobrovac
mmodell
Group Reviewers
Release-Engineering-Team
Commits
rGBLBR588a5eec5428: Create directory for common unprivileged app dependencies
Patch without arc
git checkout -b D756 && curl -L https://phabricator.wikimedia.org/D756?download=true | git apply
Summary

Establish /opt/lib as the location for installing application
dependencies that are installed via unprivileged execution and from
untrusted sources. The directory is created during the privileged build
phase and owned by the unprivileged runtime user.

Depends on D741

Test Plan

Run go test ./... or arc unit.

Diff Detail

Repository
rGBLBR Blubber
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dduvall created this revision.Aug 23 2017, 5:12 PM
Restricted Application added a reviewer: mmodell. · View Herald TranscriptAug 23 2017, 5:12 PM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald Transcript
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
mobrovac added inline comments.Aug 24 2017, 2:22 PM
config/runs.go
9

Why /opt/local ? Also, would it be possible to configure it rather than hard-code it?

thcipriani accepted this revision.Aug 29 2017, 4:01 PM

I remember talking about the necessity for this directory.

Accepting this revision to unblock additional patches; however, I think this should either be made configurable (as @mobrovac suggests) and/or follow the conventions being established in T169998: RFC: Container path conventions

This revision is now accepted and ready to land.Aug 29 2017, 4:01 PM
dduvall added inline comments.Aug 31 2017, 9:17 PM
config/runs.go
9

I chose /opt/local based on my (probably incorrect) interpretation of its role in the FHS. Looking at FHS 3.0 now, I would say /opt/lib is more correct but I honestly think this is just one of those things that could be debated endlessly.

There are currently limitations of scope between config elements that prevents this from being configurable at the moment but I plan on factoring out those limitations soon.

mobrovac edited edge metadata.Sep 4 2017, 10:51 AM

I'm ok with it with the caveat that I'd like to see a refectoring of the code to allow this path to be configurable.

mobrovac accepted this revision.Sep 4 2017, 10:51 AM

Ups, forgot to accept :)

mmodell accepted this revision.Sep 5 2017, 9:39 AM
dduvall updated this revision to Diff 2026.Sep 5 2017, 4:31 PM
dduvall edited the summary of this revision. (Show Details)

Changed shared lib directory to /opt/lib

This revision was automatically updated to reflect the committed changes.