Page MenuHomePhabricator

Support environment variables
ClosedPublic

Authored by dduvall on Jun 22 2017, 9:38 PM.

Details

Maniphest Tasks
T168425: Support environment variables in configuration
Reviewers
thcipriani
mobrovac
hashar
Jrbranaa
mmodell
Group Reviewers
Release-Engineering-Team
Commits
rGBLBR9f2ef14ba62f: Support environment variables
Patch without arc
git checkout -b D691 && curl -L https://phabricator.wikimedia.org/D691?download=true | git apply
Summary

Added support for definition of environment variables under
runs.environment. Corresponding ENV instructions will be added to
Dockerfile output for the unprivileged build phase.

Fixes T168425

Test Plan

Run go test ./....

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.Jun 22 2017, 9:38 PM
Restricted Application added a reviewer: mmodell. · View Herald TranscriptJun 22 2017, 9:38 PM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald Transcript
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
dduvall updated this revision to Diff 1800.Jun 22 2017, 9:44 PM
dduvall edited the summary of this revision. (Show Details)

Added task reference.

mobrovac edited edge metadata.Jun 23 2017, 8:16 PM

Will this output only one ENV line or will it be one variable per line? In the latter case, +1!

Will this output only one ENV line or will it be one variable per line? In the latter case, +1!

It outputs a single line, actually. That was the recommendation in the Dockerfile docs:

the [single-line] form is preferred because it produces a single cache layer

In D691#13572, @dduvall wrote:

Will this output only one ENV line or will it be one variable per line? In the latter case, +1!

It outputs a single line, actually. That was the recommendation in the Dockerfile docs:

the [single-line] form is preferred because it produces a single cache layer

Oh I see. They are correct, but is there a line length limit? If so, we might want to check that and perhaps split in multiple lines when that is the case?

In D691#13572, @dduvall wrote:

Will this output only one ENV line or will it be one variable per line? In the latter case, +1!

It outputs a single line, actually. That was the recommendation in the Dockerfile docs:

the [single-line] form is preferred because it produces a single cache layer

Oh I see. They are correct, but is there a line length limit? If so, we might want to check that and perhaps split in multiple lines when that is the case?

Looks like they're parsing linewise with bufio.NewScanner which is subject to a 64k buffer by default. That doesn't seem like it would cause us problems but it should also be simple enough to add \ continuations in docker.CompileInstruction.

dduvall updated this revision to Diff 1804.Jun 26 2017, 8:20 PM

Refactored ENV instruction output to separate name/value pairs one per line using the backslash linebreak syntax.

mobrovac accepted this revision.Jun 26 2017, 8:24 PM
This revision is now accepted and ready to land.Jun 26 2017, 8:24 PM
This revision was automatically updated to reflect the committed changes.