Skip to content

Commit

Permalink
preprocessor: do not honour CUE_UNSAFE_NETWORK_HOST in multi-step
Browse files Browse the repository at this point in the history
In the general case in a mult-step script, we could run a process that
does an arbitrary amount of networking.

Right now, setting CUE_UNSAFE_NETWORK_HOST to non-empty adds the
--network=host flag to the running of a multi-step script.

This means, however, that whenever a multi-step script does do something
networking-related (for example in a later CL we run the 'cue mod
registry' server) then this "interferes" and is impacted by the host
network situation.

(Note this comment is potentially even true for code blocks, but for now
we fairly carefully "control" the commands that are run in that
situation so we "know" it isn't a problem. Even if something skipped
through review, it would quickly become clear there is a clash/similar).

Therefore, we stop honouring CUE_UNSAFE_NETWORK_HOST being set for
multi-step scripts to ensure we do not create any network clashes. This
change ensures that for those docker runs which do not require
networking they remain fast (establishing a separate network stack is
expensive), and that multi-step script runs are safe from conflicts.

Note: the cost of establishing a per-container network stack could be
somewhat/entirely amortised in the case where that container does not
require any networking by using a newly created, empty stack that is
shared between all such containers (rather than sharing with the host).
Such a temporary stack could be created once, used by many containers
(that don't require a network stack) and then torn down at the end to
avoid leaving any artefacts around. We will test/explore this in a later
CL.

Preprocessor-No-Write-Cache: true
Signed-off-by: Paul Jolly <[email protected]>
Change-Id: I64664fb836cbfe1f7f0e1e94e417595d102e84ed
Dispatch-Trailer: {"type":"trybot","CL":1176742,"patchset":7,"ref":"refs/changes/42/1176742/7","targetBranch":"alpha"}
  • Loading branch information
myitcv authored and cueckoo committed Feb 12, 2024
1 parent a4539a6 commit dd87887
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions internal/cmd/preprocessor/cmd/rootfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,12 +579,9 @@ func (m *multiStepScript) run() (runerr error) {
"-v", fmt.Sprintf("%s:/scripts", scriptsDir),
)

// If the user wants to be unsafe and _not_ isolate the network let them
// It results in much faster running times when working on changes in the
// preprocessor.
if os.Getenv("CUE_UNSAFE_NETWORK_HOST") != "" {
args = append(args, "--network=host")
}
// We cannot perform the --network=host trick here, even if the user wants
// to be unsafe. Because we run cue mod registry which requires its own
// networking isolation (for binding to the port it will use etc).

args = append(args,
// TODO: support per-guide docker images
Expand Down

0 comments on commit dd87887

Please sign in to comment.