Page MenuHomePhabricator

Fix ownership on artifact copies
ClosedPublic

Authored by dduvall on Mar 9 2018, 11:56 PM.

Details

Reviewers
thcipriani
mmodell
hashar
demon
Group Reviewers
Release-Engineering-Team
Commits
rGBLBR50c5793952a7: Fix ownership on artifact copies
Patch without arc
git checkout -b D1002 && curl -L https://phabricator.wikimedia.org/D1002?download=true | git apply
Summary

The implementation of D984 did not include enforcing ownership for
build.CopyFrom instruction and so artifacts copied from one image to
another via copies: were problematically owned as root.

In order to fix this behavior:

  1. config.ArtifactConfig build.CopyFrom instructions are now injected duration build.PhaseInstall
  2. config.VariantConfig calls build.ApplyUser for these artifact instructions as well using the runs.as user
  3. build.CopyAs was refactored to wrap any build.Instruction which should only really be used with build.Copy or build.CopyFrom.
Test Plan

Run go test ./.... Run blubber against configuration with a variant that
uses copies and verify that the COPY --from instructions also include a
--chown flag.

Diff Detail

Repository
rGBLBR Blubber
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dduvall edited the summary of this revision. (Show Details)

Refactored build.CopyAs to use a generic Instruction anonymouse field instread of separate fields for Copy and CopyFrom usage.

Works. Refactor makes building a CopyAs cleaner than having 2 embedded structs.

This revision is now accepted and ready to land.Mar 22 2018, 5:50 PM
This revision was automatically updated to reflect the committed changes.