Define `NODE_ENV` and always define `NODE_PATH`

Summary:
Define `NODE_ENV` based on the configuration and define `NODE_PATH` when
either `npm.install` or `npm.env` are true. The latter behavior is
necessary for final stages of multi-stage builds where npm modules are
not installed but the application requires node-related environment
variables to run.

Depends on D757
Ref T174626

Rename npm module/config to node

Renamed `npm` yaml entry to `node` and `NpmConfig` to `NodeConfig`.
Since we're dealing with general node builds and configuration, this
seems to make more sense.

Test Plan:
Run `go test ./...` or `arc unit`. Run blubber against at least the
`blubber.example.yaml`.

Reviewers: thcipriani, mobrovac, mmodell, #release-engineering-team

Reviewed By: thcipriani, #release-engineering-team

Tags: #release-engineering-team

Maniphest Tasks: T174626

Differential Revision: https://phabricator.wikimedia.org/D759

8 files changed, 119 insertions, 85 deletions

diff --git a/README.md b/README.md
index 0da5f6a..38cdcae 100644
--- a/README.md
+++ b/README.md
@@ -14,8 +14,8 @@ running ad-hoc commands.
 base: debian:jessie
 apt:
   packages: [libjpeg, libyaml]
-npm:
-  install: true
+node:
+  dependencies: true
 runs:
   in: /srv/service
   as: runuser
@@ -39,13 +39,13 @@ variants:
 
   production:
     base: debian:jessie-slim
-    npm:
+    node:
       env: production
     artifacts:
       - from: test
         source: /srv/service
         destination: .
-    entrypoint: [npm, start]
+    entrypoint: [node, server.js]
 ```
 
 ## Variants
@@ -68,8 +68,8 @@ becomes:
 base: debian:jessie
 apt:
   packages: [libjpeg, libyaml, libjpeg-dev, libyaml-dev, chromium]
-npm:
-  install: true
+node:
+  dependencies: true
 runs:
   in: /srv/service
   as: runuser
diff --git a/blubber.example.yaml b/blubber.example.yaml
index e160ace..4cea7c3 100644
--- a/blubber.example.yaml
+++ b/blubber.example.yaml
@@ -2,8 +2,8 @@
 base: debian:jessie
 apt:
   packages: [libjpeg, libyaml]
-npm:
-  install: true
+node:
+  dependencies: true
 runs:
   in: /srv/service
   as: runuser
@@ -27,10 +27,10 @@ variants:
 
   production:
     base: debian:jessie-slim
-    npm:
+    node:
       env: production
     artifacts:
       - from: test
         source: /srv/service
         destination: .
-    entrypoint: [npm, start]
+    entrypoint: [node, server.js]
diff --git a/config/common.go b/config/common.go
index f627b05..454ae2e 100644
--- a/config/common.go
+++ b/config/common.go
@@ -7,7 +7,7 @@ import (
 type CommonConfig struct {
 	Base         string     `yaml:"base"`
 	Apt          AptConfig  `yaml:"apt"`
-	Npm          NpmConfig  `yaml:"npm"`
+	Node         NodeConfig `yaml:"node"`
 	Runs         RunsConfig `yaml:"runs"`
 	SharedVolume Flag       `yaml:"sharedvolume"`
 	EntryPoint   []string   `yaml:"entrypoint"`
@@ -19,7 +19,7 @@ func (cc1 *CommonConfig) Merge(cc2 CommonConfig) {
 	}
 
 	cc1.Apt.Merge(cc2.Apt)
-	cc1.Npm.Merge(cc2.Npm)
+	cc1.Node.Merge(cc2.Node)
 	cc1.Runs.Merge(cc2.Runs)
 	cc1.SharedVolume.Merge(cc2.SharedVolume)
 
@@ -29,7 +29,7 @@ func (cc1 *CommonConfig) Merge(cc2 CommonConfig) {
 }
 
 func (cc *CommonConfig) PhaseCompileableConfig() []build.PhaseCompileable {
-	return []build.PhaseCompileable{cc.Apt, cc.Npm, cc.Runs}
+	return []build.PhaseCompileable{cc.Apt, cc.Node, cc.Runs}
 }
 
 func (cc *CommonConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
diff --git a/config/common_test.go b/config/common_test.go
index 1cce81d..69fb02d 100644
--- a/config/common_test.go
+++ b/config/common_test.go
@@ -11,9 +11,6 @@ import (
 func TestCommonConfig(t *testing.T) {
 	cfg, err := config.ReadConfig([]byte(`---
     base: fooimage
-    apt: {}
-    npm: {}
-    runs: {}
     sharedvolume: true
     entrypoint: ["/bin/foo"]
     variants:
diff --git a/config/flag_test.go b/config/flag_test.go
index e249e60..fb22647 100644
--- a/config/flag_test.go
+++ b/config/flag_test.go
@@ -10,12 +10,12 @@ import (
 
 func TestFlagOverwrite(t *testing.T) {
 	cfg, err := config.ReadConfig([]byte(`---
-    npm: { install: true }
+    node: { dependencies: true }
     sharedvolume: false
     variants:
       development:
         sharedvolume: true
-        npm: { install: false }`))
+        node: { dependencies: false }`))
 
 	assert.Nil(t, err)
 
@@ -23,6 +23,6 @@ func TestFlagOverwrite(t *testing.T) {
 
 	assert.Nil(t, err)
 
-	assert.False(t, variant.Npm.Install.True)
+	assert.False(t, variant.Node.Dependencies.True)
 	assert.True(t, variant.SharedVolume.True)
 }
diff --git a/config/node.go b/config/node.go
new file mode 100644
index 0000000..5693711
--- /dev/null
+++ b/config/node.go
@@ -0,0 +1,52 @@
+package config
+
+import (
+	"path"
+	"phabricator.wikimedia.org/source/blubber.git/build"
+)
+
+type NodeConfig struct {
+	Dependencies Flag   `yaml:"dependencies"`
+	Env          string `yaml:"env"`
+}
+
+func (nc *NodeConfig) Merge(nc2 NodeConfig) {
+	nc.Dependencies.Merge(nc2.Dependencies)
+
+	if nc2.Env != "" {
+		nc.Env = nc2.Env
+	}
+}
+
+func (nc NodeConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
+	switch phase {
+	case build.PhasePreInstall:
+		if nc.Dependencies.True {
+			npmInstall := build.RunAll{[]build.Run{
+				{"cd", []string{LocalLibPrefix}},
+				{"npm install", []string{}},
+			}}
+
+			if nc.Env == "production" {
+				npmInstall.Runs[1].Arguments = []string{"--production"}
+				npmInstall.Runs = append(npmInstall.Runs,
+					build.Run{"npm dedupe", []string{}},
+				)
+			}
+
+			return []build.Instruction{
+				build.Copy{[]string{"package.json"}, LocalLibPrefix},
+				npmInstall,
+			}
+		}
+	case build.PhasePostInstall:
+		if nc.Env != "" {
+			return []build.Instruction{build.Env{map[string]string{
+				"NODE_ENV":  nc.Env,
+				"NODE_PATH": path.Join(LocalLibPrefix, "node_modules"),
+			}}}
+		}
+	}
+
+	return []build.Instruction{}
+}
diff --git a/config/npm_test.go b/config/node_test.go
index da46fcc..7ce66fc 100644
--- a/config/npm_test.go
+++ b/config/node_test.go
@@ -9,30 +9,30 @@ import (
 	"phabricator.wikimedia.org/source/blubber.git/config"
 )
 
-func TestNpmConfig(t *testing.T) {
+func TestNodeConfig(t *testing.T) {
 	cfg, err := config.ReadConfig([]byte(`---
-    npm:
-      install: true
+    node:
+      dependencies: true
       env: foo
     variants:
       build:
-        npm:
-          install: false
+        node:
+          dependencies: false
           env: bar`))
 
 	assert.Nil(t, err)
 
-	assert.Equal(t, true, cfg.Npm.Install.True)
-	assert.Equal(t, "foo", cfg.Npm.Env)
+	assert.Equal(t, true, cfg.Node.Dependencies.True)
+	assert.Equal(t, "foo", cfg.Node.Env)
 
 	variant, err := config.ExpandVariant(cfg, "build")
 
-	assert.Equal(t, false, variant.Npm.Install.True)
-	assert.Equal(t, "bar", variant.Npm.Env)
+	assert.Equal(t, false, variant.Node.Dependencies.True)
+	assert.Equal(t, "bar", variant.Node.Env)
 }
 
-func TestNpmConfigInstructionsNoInstall(t *testing.T) {
-	cfg := config.NpmConfig{Install: config.Flag{True: false}}
+func TestNodeConfigInstructionsNoDependencies(t *testing.T) {
+	cfg := config.NodeConfig{Dependencies: config.Flag{True: false}}
 
 	t.Run("PhasePrivileged", func(t *testing.T) {
 		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivileged))
@@ -51,8 +51,8 @@ func TestNpmConfigInstructionsNoInstall(t *testing.T) {
 	})
 }
 
-func TestNpmConfigInstructionsNonProduction(t *testing.T) {
-	cfg := config.NpmConfig{Install: config.Flag{True: true}, Env: "foo"}
+func TestNodeConfigInstructionsNonProduction(t *testing.T) {
+	cfg := config.NodeConfig{Dependencies: config.Flag{True: true}, Env: "foo"}
 
 	t.Run("PhasePrivileged", func(t *testing.T) {
 		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivileged))
@@ -78,15 +78,18 @@ func TestNpmConfigInstructionsNonProduction(t *testing.T) {
 	t.Run("PhasePostInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
-				build.Env{map[string]string{"NODE_PATH": "/opt/lib/node_modules"}},
+				build.Env{map[string]string{
+					"NODE_ENV":  "foo",
+					"NODE_PATH": "/opt/lib/node_modules",
+				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),
 		)
 	})
 }
 
-func TestNpmConfigInstructionsProduction(t *testing.T) {
-	cfg := config.NpmConfig{Install: config.Flag{True: true}, Env: "production"}
+func TestNodeConfigInstructionsProduction(t *testing.T) {
+	cfg := config.NodeConfig{Dependencies: config.Flag{True: true}, Env: "production"}
 
 	t.Run("PhasePrivileged", func(t *testing.T) {
 		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivileged))
@@ -113,7 +116,38 @@ func TestNpmConfigInstructionsProduction(t *testing.T) {
 	t.Run("PhasePostInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
-				build.Env{map[string]string{"NODE_PATH": "/opt/lib/node_modules"}},
+				build.Env{map[string]string{
+					"NODE_ENV":  "production",
+					"NODE_PATH": "/opt/lib/node_modules",
+				}},
+			},
+			cfg.InstructionsForPhase(build.PhasePostInstall),
+		)
+	})
+}
+
+func TestNodeConfigInstructionsEnvironmentOnly(t *testing.T) {
+	cfg := config.NodeConfig{Env: "production"}
+
+	t.Run("PhasePrivileged", func(t *testing.T) {
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivileged))
+	})
+
+	t.Run("PhasePrivilegeDropped", func(t *testing.T) {
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivilegeDropped))
+	})
+
+	t.Run("PhasePreInstall", func(t *testing.T) {
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePreInstall))
+	})
+
+	t.Run("PhasePostInstall", func(t *testing.T) {
+		assert.Equal(t,
+			[]build.Instruction{
+				build.Env{map[string]string{
+					"NODE_ENV":  "production",
+					"NODE_PATH": "/opt/lib/node_modules",
+				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),
 		)
diff --git a/config/npm.go b/config/npm.go
deleted file mode 100644
index ba3dc26..0000000
--- a/config/npm.go
+++ /dev/null
@@ -1,49 +0,0 @@
-package config
-
-import (
-	"path"
-	"phabricator.wikimedia.org/source/blubber.git/build"
-)
-
-type NpmConfig struct {
-	Install Flag   `yaml:"install"`
-	Env     string `yaml:"env"`
-}
-
-func (npm *NpmConfig) Merge(npm2 NpmConfig) {
-	npm.Install.Merge(npm2.Install)
-
-	if npm2.Env != "" {
-		npm.Env = npm2.Env
-	}
-}
-
-func (npm NpmConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
-	if npm.Install.True {
-		switch phase {
-		case build.PhasePreInstall:
-			npmInstall := build.RunAll{[]build.Run{
-				{"cd", []string{LocalLibPrefix}},
-				{"npm install", []string{}},
-			}}
-
-			if npm.Env == "production" {
-				npmInstall.Runs[1].Arguments = []string{"--production"}
-				npmInstall.Runs = append(npmInstall.Runs,
-					build.Run{"npm dedupe", []string{}},
-				)
-			}
-
-			return []build.Instruction{
-				build.Copy{[]string{"package.json"}, LocalLibPrefix},
-				npmInstall,
-			}
-		case build.PhasePostInstall:
-			return []build.Instruction{
-				build.Env{map[string]string{"NODE_PATH": path.Join(LocalLibPrefix, "node_modules")}},
-			}
-		}
-	}
-
-	return []build.Instruction{}
-}