Merge pull request #47723 from vvoland/builder-fix-workdir-slash

builder: Fix `WORKDIR` with a trailing slash causing a cache miss

builder/dockerfile/dispatchers_unix.go

@@ -22,7 +22,7 @@ func normalizeWorkdir(_ string, current string, requested string) (string, error
 	if !filepath.IsAbs(requested) {
 		return filepath.Join(string(os.PathSeparator), current, requested), nil
 	}
-	return requested, nil
+	return filepath.Clean(requested), nil
 }
 
 // resolveCmdLine takes a command line arg set and optionally prepends a platform-specific

container/container.go

@@ -299,8 +299,8 @@ func (container *Container) SetupWorkingDirectory(rootIdentity idtools.Identity)
 		return nil
 	}
 
-	container.Config.WorkingDir = filepath.Clean(container.Config.WorkingDir)
-	pth, err := container.GetResourcePath(container.Config.WorkingDir)
+	workdir := filepath.Clean(container.Config.WorkingDir)
+	pth, err := container.GetResourcePath(workdir)
 	if err != nil {
 		return err
 	}

daemon/container.go

@@ -364,6 +364,6 @@ func translateWorkingDir(config *containertypes.Config) error {
 	if !system.IsAbs(wd) {
 		return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
 	}
-	config.WorkingDir = wd
+	config.WorkingDir = filepath.Clean(wd)
 	return nil
 }

integration-cli/docker_cli_build_test.go

@@ -4296,7 +4296,7 @@ func (s *DockerCLIBuildSuite) TestBuildBuildTimeArgExpansion(c *testing.T) {
 	imgName := "bldvarstest"
 
 	wdVar := "WDIR"
-	wdVal := "/tmp/"
+	wdVal := "/tmp"
 	addVar := "AFILE"
 	addVal := "addFile"
 	copyVar := "CFILE"

integration/build/build_test.go

@@ -689,6 +689,72 @@ func TestBuildPlatformInvalid(t *testing.T) {
 	assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter))
 }
 
+// TestBuildWorkdirNoCacheMiss is a regression test for https://github.com/moby/moby/issues/47627
+func TestBuildWorkdirNoCacheMiss(t *testing.T) {
+	ctx := setupTest(t)
+
+	for _, tc := range []struct {
+		name       string
+		dockerfile string
+	}{
+		{name: "trailing slash", dockerfile: "FROM busybox\nWORKDIR /foo/"},
+		{name: "no trailing slash", dockerfile: "FROM busybox\nWORKDIR /foo"},
+	} {
+		dockerfile := tc.dockerfile
+		t.Run(tc.name, func(t *testing.T) {
+			source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile))
+			defer source.Close()
+
+			apiClient := testEnv.APIClient()
+
+			buildAndGetID := func() string {
+				resp, err := apiClient.ImageBuild(ctx, source.AsTarReader(t), types.ImageBuildOptions{
+					Version: types.BuilderV1,
+				})
+				assert.NilError(t, err)
+				defer resp.Body.Close()
+
+				id := readBuildImageIDs(t, resp.Body)
+				assert.Check(t, id != "")
+				return id
+			}
+
+			firstId := buildAndGetID()
+			secondId := buildAndGetID()
+
+			assert.Check(t, is.Equal(firstId, secondId), "expected cache to be used")
+		})
+	}
+}
+
+func readBuildImageIDs(t *testing.T, rd io.Reader) string {
+	t.Helper()
+	decoder := json.NewDecoder(rd)
+	for {
+		var jm jsonmessage.JSONMessage
+		if err := decoder.Decode(&jm); err != nil {
+			if err == io.EOF {
+				break
+			}
+			assert.NilError(t, err)
+		}
+
+		if jm.Aux == nil {
+			continue
+		}
+
+		var auxId struct {
+			ID string `json:"ID"`
+		}
+
+		if json.Unmarshal(*jm.Aux, &auxId); auxId.ID != "" {
+			return auxId.ID
+		}
+	}
+
+	return ""
+}
+
 func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) {
 	err := w.WriteHeader(&tar.Header{
 		Name:     fn,

integration/container/run_linux_test.go

@@ -326,3 +326,33 @@ func TestStaticIPOutsideSubpool(t *testing.T) {
 
 	assert.Check(t, is.Contains(b.String(), "inet 10.42.1.3/16"))
 }
+
+func TestWorkingDirNormalization(t *testing.T) {
+	ctx := setupTest(t)
+	apiClient := testEnv.APIClient()
+
+	for _, tc := range []struct {
+		name    string
+		workdir string
+	}{
+		{name: "trailing slash", workdir: "/tmp/"},
+		{name: "no trailing slash", workdir: "/tmp"},
+	} {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			cID := container.Run(ctx, t, apiClient,
+				container.WithImage("busybox"),
+				container.WithWorkingDir(tc.workdir),
+			)
+
+			defer container.Remove(ctx, t, apiClient, cID, containertypes.RemoveOptions{Force: true})
+
+			inspect := container.Inspect(ctx, t, apiClient, cID)
+
+			assert.Check(t, is.Equal(inspect.Config.WorkingDir, "/tmp"))
+		})
+
+	}
+}