Rebuild layer if npm config/node version changes

fixes paketo-buildpacksgh-153 Signed-off-by: Arjun Sreedharan <asreedharan@vmware.com>
andymoe · May 11, 2021 · 5c7ae9d · 5c7ae9d
1 parent 214cd30
commit 5c7ae9d
Show file tree

Hide file tree

Showing 6 changed files with 203 additions and 10 deletions.
diff --git a/build_process_resolver.go b/build_process_resolver.go
@@ -1,7 +1,9 @@
 package npminstall
 
 import (
+	"bytes"
 	"errors"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 
@@ -101,6 +103,36 @@ func (r BuildProcessResolver) Resolve(workingDir, cacheDir string) (BuildProcess
 	}
 }
 
+// cacheExecutableResponse writes the output of a successfully executed command
+// to a tmp file and returns the file location and possibly and error
+func cacheExecutableResponse(executable Executable, args []string, workingDir string, logger scribe.Logger) (string, error) {
+	buffer := bytes.NewBuffer(nil)
+	err := executable.Execute(pexec.Execution{
+		Args:   args,
+		Dir:    workingDir,
+		Stdout: buffer,
+		Stderr: buffer,
+	})
+	if err != nil {
+		logger.Subprocess("error: %s", buffer.String())
+		return "", err
+	}
+
+	tmpFile, err := ioutil.TempFile(workingDir, "executable_response")
+	if err != nil {
+		logger.Subprocess("error: %s", buffer.String())
+		return "", err
+	}
+
+	err = os.WriteFile(tmpFile.Name(), buffer.Bytes(), 0644)
+	if err != nil {
+		logger.Subprocess("error: %s", buffer.String())
+		return "", err
+	}
+
+	return tmpFile.Name(), nil
+}
+
 func fileExists(path string) (bool, error) {
 	exists := true
 	_, err := os.Stat(path)

diff --git a/ci_build_process.go b/ci_build_process.go
@@ -29,7 +29,20 @@ func NewCIBuildProcess(executable Executable, summer Summer, environment Environ
 }
 
 func (r CIBuildProcess) ShouldRun(workingDir string, metadata map[string]interface{}) (bool, string, error) {
-	sum, err := r.summer.Sum(filepath.Join(workingDir, "package.json"), filepath.Join(workingDir, "package-lock.json"))
+	cachedNodeVersion, err := cacheExecutableResponse(
+		r.executable,
+		[]string{"config", "list"},
+		workingDir,
+		r.logger)
+	if err != nil {
+		return false, "", fmt.Errorf("failed to execute npm config: %w", err)
+	}
+	defer os.Remove(cachedNodeVersion)
+
+	sum, err := r.summer.Sum(
+		filepath.Join(workingDir, "package.json"),
+		filepath.Join(workingDir, "package-lock.json"),
+		cachedNodeVersion)
 	if err != nil {
 		return false, "", err
 	}

diff --git a/ci_build_process_test.go b/ci_build_process_test.go
@@ -26,6 +26,7 @@ func testCIBuildProcess(t *testing.T, context spec.G, it spec.S) {
 		cacheDir      string
 		workingDir    string
 		executable    *fakes.Executable
+		executions    []pexec.Execution
 		summer        *fakes.Summer
 		environment   *fakes.EnvironmentConfig
 		buffer        *bytes.Buffer
@@ -46,6 +47,11 @@ func testCIBuildProcess(t *testing.T, context spec.G, it spec.S) {
 		Expect(err).NotTo(HaveOccurred())
 
 		executable = &fakes.Executable{}
+		executable.ExecuteCall.Stub = func(execution pexec.Execution) error {
+			executions = append(executions, execution)
+			return nil
+		}
+
 		summer = &fakes.Summer{}
 		environment = &fakes.EnvironmentConfig{}
 
@@ -76,10 +82,18 @@ func testCIBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				})
 				Expect(err).NotTo(HaveOccurred())
 
-				Expect(summer.SumCall.Receives.Paths).To(Equal([]string{filepath.Join(workingDir, "package.json"), filepath.Join(workingDir, "package-lock.json")}))
+				Expect(summer.SumCall.Receives.Paths[0]).To(Equal(filepath.Join(workingDir, "package.json")))
+				Expect(summer.SumCall.Receives.Paths[1]).To(Equal(filepath.Join(workingDir, "package-lock.json")))
+				Expect(summer.SumCall.Receives.Paths[2]).To(ContainSubstring("executable_response"))
 
 				Expect(run).To(BeFalse())
 				Expect(sha).To(BeEmpty())
+				lastExecution := executions[len(executions)-1]
+				Expect(lastExecution.Args).To(Equal([]string{
+					"config",
+					"list",
+				}))
+				Expect(lastExecution.Dir).To(Equal(workingDir))
 			})
 		})
 
@@ -94,9 +108,19 @@ func testCIBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				})
 				Expect(err).NotTo(HaveOccurred())
 
-				Expect(summer.SumCall.Receives.Paths).To(Equal([]string{filepath.Join(workingDir, "package.json"), filepath.Join(workingDir, "package-lock.json")}))
+				Expect(summer.SumCall.Receives.Paths[0]).To(Equal(filepath.Join(workingDir, "package.json")))
+				Expect(summer.SumCall.Receives.Paths[1]).To(Equal(filepath.Join(workingDir, "package-lock.json")))
+				Expect(summer.SumCall.Receives.Paths[2]).To(ContainSubstring("executable_response"))
+
 				Expect(run).To(BeTrue())
 				Expect(sha).To(Equal("other-cache-sha"))
+
+				lastExecution := executions[len(executions)-1]
+				Expect(lastExecution.Args).To(Equal([]string{
+					"config",
+					"list",
+				}))
+				Expect(lastExecution.Dir).To(Equal(workingDir))
 			})
 		})
 
@@ -109,10 +133,19 @@ func testCIBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				run, sha, err := process.ShouldRun(workingDir, nil)
 				Expect(err).NotTo(HaveOccurred())
 
-				Expect(summer.SumCall.Receives.Paths).To(Equal([]string{filepath.Join(workingDir, "package.json"), filepath.Join(workingDir, "package-lock.json")}))
+				Expect(summer.SumCall.Receives.Paths[0]).To(Equal(filepath.Join(workingDir, "package.json")))
+				Expect(summer.SumCall.Receives.Paths[1]).To(Equal(filepath.Join(workingDir, "package-lock.json")))
+				Expect(summer.SumCall.Receives.Paths[2]).To(ContainSubstring("executable_response"))
 
 				Expect(run).To(BeTrue())
 				Expect(sha).To(Equal("other-cache-sha"))
+
+				lastExecution := executions[len(executions)-1]
+				Expect(lastExecution.Args).To(Equal([]string{
+					"config",
+					"list",
+				}))
+				Expect(lastExecution.Dir).To(Equal(workingDir))
 			})
 		})
 
@@ -128,6 +161,21 @@ func testCIBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				})
 			})
 
+			context("when npm config list fails to execute", func() {
+				it.Before(func() {
+					executable.ExecuteCall.Stub = func(execution pexec.Execution) error {
+						return errors.New("very bad error")
+					}
+					process = npminstall.NewCIBuildProcess(executable, summer, environment, scribe.NewLogger(buffer))
+				})
+
+				it("fails", func() {
+					_, _, err := process.ShouldRun(workingDir, nil)
+					Expect(err).To(MatchError(ContainSubstring("very bad error")))
+					Expect(err).To(MatchError(ContainSubstring("failed to execute npm config")))
+				})
+			})
+
 		})
 	})
 

diff --git a/integration/caching_test.go b/integration/caching_test.go
@@ -180,6 +180,68 @@ func testCaching(t *testing.T, context spec.G, it spec.S) {
 			Expect(secondImage.Buildpacks[1].Layers["modules"].SHA).To(Equal(firstImage.Buildpacks[1].Layers["modules"].SHA))
 			Expect(secondImage.Buildpacks[1].Layers["modules"].Metadata["built_at"]).To(Equal(firstImage.Buildpacks[1].Layers["modules"].Metadata["built_at"]))
 		})
+
+		context("and the node.js version has changed", func() {
+			it("reinstalls node_modules", func() {
+				var err error
+				source, err = occam.Source(filepath.Join("testdata", "locked_app"))
+				Expect(err).NotTo(HaveOccurred())
+
+				build := pack.Build.
+					WithPullPolicy("never").
+					WithEnv(map[string]string{"BP_NODE_VERSION": "~14"}).
+					WithBuildpacks(nodeURI, buildpackURI, buildPlanURI)
+
+				firstImage, logs, err := build.Execute(name, source)
+				Expect(err).NotTo(HaveOccurred(), logs.String)
+
+				imageIDs[firstImage.ID] = struct{}{}
+
+				Expect(firstImage.Buildpacks).To(HaveLen(3))
+				Expect(firstImage.Buildpacks[1].Key).To(Equal(buildpackInfo.Buildpack.ID))
+				Expect(firstImage.Buildpacks[1].Layers).To(HaveKey("modules"))
+
+				container, err := docker.Container.Run.
+					WithCommand("npm start").
+					WithEnv(map[string]string{"PORT": "8080"}).
+					WithPublish("8080").
+					Execute(firstImage.ID)
+				Expect(err).NotTo(HaveOccurred())
+
+				containerIDs[container.ID] = struct{}{}
+
+				Eventually(container).Should(BeAvailable())
+
+				build = pack.Build.
+					WithPullPolicy("never").
+					WithEnv(map[string]string{"BP_NODE_VERSION": "~15"}).
+					WithBuildpacks(nodeURI, buildpackURI, buildPlanURI)
+
+				secondImage, logs, err := build.Execute(name, source)
+				Expect(err).NotTo(HaveOccurred(), logs.String)
+
+				imageIDs[secondImage.ID] = struct{}{}
+
+				Expect(secondImage.Buildpacks).To(HaveLen(3))
+				Expect(secondImage.Buildpacks[1].Key).To(Equal(buildpackInfo.Buildpack.ID))
+				Expect(secondImage.Buildpacks[1].Layers).To(HaveKey("modules"))
+
+				container, err = docker.Container.Run.
+					WithCommand("npm start").
+					WithEnv(map[string]string{"PORT": "8080"}).
+					WithPublish("8080").
+					Execute(secondImage.ID)
+				Expect(err).NotTo(HaveOccurred())
+
+				containerIDs[container.ID] = struct{}{}
+
+				Eventually(container).Should(BeAvailable())
+
+				Expect(secondImage.ID).NotTo(Equal(firstImage.ID))
+				Expect(secondImage.Buildpacks[1].Layers["modules"].SHA).NotTo(Equal(firstImage.Buildpacks[1].Layers["modules"].SHA))
+				Expect(secondImage.Buildpacks[1].Layers["modules"].Metadata["built_at"]).NotTo(Equal(firstImage.Buildpacks[1].Layers["modules"].Metadata["built_at"]))
+			})
+		})
 	})
 
 	context("when the app is vendored", func() {

diff --git a/rebuild_build_process.go b/rebuild_build_process.go
@@ -29,7 +29,17 @@ func NewRebuildBuildProcess(executable Executable, summer Summer, environment En
 }
 
 func (r RebuildBuildProcess) ShouldRun(workingDir string, metadata map[string]interface{}) (bool, string, error) {
-	sum, err := r.summer.Sum(filepath.Join(workingDir, "node_modules"))
+	cachedNodeVersion, err := cacheExecutableResponse(
+		r.executable,
+		[]string{"config", "list"},
+		workingDir,
+		r.logger)
+	if err != nil {
+		return false, "", fmt.Errorf("failed to execute npm config: %w", err)
+	}
+	defer os.Remove(cachedNodeVersion)
+
+	sum, err := r.summer.Sum(filepath.Join(workingDir, "node_modules"), cachedNodeVersion)
 	if err != nil {
 		return false, "", err
 	}

diff --git a/rebuild_build_process_test.go b/rebuild_build_process_test.go
@@ -91,8 +91,14 @@ func testRebuildBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(run).To(BeFalse())
 				Expect(sha).To(BeEmpty())
-
-				Expect(summer.SumCall.Receives.Paths).To(Equal([]string{filepath.Join(workingDir, "node_modules")}))
+				Expect(summer.SumCall.Receives.Paths[0]).To(Equal(filepath.Join(workingDir, "node_modules")))
+				Expect(summer.SumCall.Receives.Paths[1]).To(ContainSubstring("executable_response"))
+				lastExecution := executions[len(executions)-1]
+				Expect(lastExecution.Args).To(Equal([]string{
+					"config",
+					"list",
+				}))
+				Expect(lastExecution.Dir).To(Equal(workingDir))
 			})
 		})
 
@@ -108,8 +114,14 @@ func testRebuildBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(run).To(BeTrue())
 				Expect(sha).To(Equal("other-cache-sha"))
-
-				Expect(summer.SumCall.Receives.Paths).To(Equal([]string{filepath.Join(workingDir, "node_modules")}))
+				Expect(summer.SumCall.Receives.Paths[0]).To(Equal(filepath.Join(workingDir, "node_modules")))
+				Expect(summer.SumCall.Receives.Paths[1]).To(ContainSubstring("executable_response"))
+				lastExecution := executions[len(executions)-1]
+				Expect(lastExecution.Args).To(Equal([]string{
+					"config",
+					"list",
+				}))
+				Expect(lastExecution.Dir).To(Equal(workingDir))
 			})
 		})
 
@@ -124,7 +136,8 @@ func testRebuildBuildProcess(t *testing.T, context spec.G, it spec.S) {
 				Expect(run).To(BeTrue())
 				Expect(sha).To(Equal("other-cache-sha"))
 
-				Expect(summer.SumCall.Receives.Paths).To(Equal([]string{filepath.Join(workingDir, "node_modules")}))
+				Expect(summer.SumCall.Receives.Paths[0]).To(Equal(filepath.Join(workingDir, "node_modules")))
+				Expect(summer.SumCall.Receives.Paths[1]).To(ContainSubstring("executable_response"))
 			})
 		})
 
@@ -139,6 +152,21 @@ func testRebuildBuildProcess(t *testing.T, context spec.G, it spec.S) {
 					Expect(err).To(MatchError("checksummer error"))
 				})
 			})
+
+			context("when npm config list fails to execute", func() {
+				it.Before(func() {
+					executable.ExecuteCall.Stub = func(execution pexec.Execution) error {
+						return errors.New("very bad error")
+					}
+					process = npminstall.NewRebuildBuildProcess(executable, summer, environment, scribe.NewLogger(buffer))
+				})
+
+				it("fails", func() {
+					_, _, err := process.ShouldRun(workingDir, nil)
+					Expect(err).To(MatchError(ContainSubstring("very bad error")))
+					Expect(err).To(MatchError(ContainSubstring("failed to execute npm config")))
+				})
+			})
 		})
 	})