From dd87887e7111763bdd30e3e020fa6995320a16b1 Mon Sep 17 00:00:00 2001 From: Paul Jolly Date: Mon, 12 Feb 2024 06:26:17 +0000 Subject: [PATCH] preprocessor: do not honour CUE_UNSAFE_NETWORK_HOST in multi-step 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 Change-Id: I64664fb836cbfe1f7f0e1e94e417595d102e84ed Dispatch-Trailer: {"type":"trybot","CL":1176742,"patchset":7,"ref":"refs/changes/42/1176742/7","targetBranch":"alpha"} --- internal/cmd/preprocessor/cmd/rootfile.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/cmd/preprocessor/cmd/rootfile.go b/internal/cmd/preprocessor/cmd/rootfile.go index 7767f0657..80b7d839a 100644 --- a/internal/cmd/preprocessor/cmd/rootfile.go +++ b/internal/cmd/preprocessor/cmd/rootfile.go @@ -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