Browse code

Windows: Consistent build workdir handling

Signed-off-by: John Howard <jhoward@microsoft.com>

John Howard authored on 2016/04/20 12:55:30
Showing 4 changed files
... ...
@@ -264,12 +264,37 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str
264 264
 	// This is from the Dockerfile and will not necessarily be in platform
265 265
 	// specific semantics, hence ensure it is converted.
266 266
 	workdir := filepath.FromSlash(args[0])
267
-
268
-	if !system.IsAbs(workdir) {
269
-		current := filepath.FromSlash(b.runConfig.WorkingDir)
270
-		workdir = filepath.Join(string(os.PathSeparator), current, workdir)
267
+	current := filepath.FromSlash(b.runConfig.WorkingDir)
268
+	if runtime.GOOS == "windows" {
269
+		// Windows is a little more complicated than Linux. This code ensures
270
+		// we end up with a workdir which is consistent in terms of platform
271
+		// semantics. This means C:\somefolder, specifically in the format:
272
+		// UPPERCASEDriveLetter-Colon-Backslash-FolderName. We are already
273
+		// guaranteed that `current`, if set, is consistent. This allows us to
274
+		// cope correctly with any of the following in a Dockerfile:
275
+		//	WORKDIR a                       --> C:\a
276
+		//	WORKDIR c:\\foo                 --> C:\foo
277
+		//	WORKDIR \\foo                   --> C:\foo
278
+		//	WORKDIR /foo                    --> C:\foo
279
+		//	WORKDIR c:\\foo \ WORKDIR bar   --> C:\foo --> C:\foo\bar
280
+		//	WORKDIR C:/foo \ WORKDIR bar    --> C:\foo --> C:\foo\bar
281
+		//	WORKDIR C:/foo \ WORKDIR \\bar  --> C:\foo --> C:\bar
282
+		//	WORKDIR /foo \ WORKDIR c:/bar   --> C:\foo --> C:\bar
283
+		if len(current) == 0 || system.IsAbs(workdir) {
284
+			if (workdir[0] == os.PathSeparator) ||
285
+				(len(workdir) > 1 && string(workdir[1]) != ":") ||
286
+				(len(workdir) == 1) {
287
+				workdir = filepath.Join(`C:\`, workdir)
288
+			}
289
+		} else {
290
+			workdir = filepath.Join(current, workdir)
291
+		}
292
+		workdir = strings.ToUpper(string(workdir[0])) + workdir[1:] // Upper-case drive letter
293
+	} else {
294
+		if !filepath.IsAbs(workdir) {
295
+			workdir = filepath.Join(string(os.PathSeparator), current, workdir)
296
+		}
271 297
 	}
272
-
273 298
 	b.runConfig.WorkingDir = workdir
274 299
 
275 300
 	return b.commit("", b.runConfig.Cmd, fmt.Sprintf("WORKDIR %v", workdir))
... ...
@@ -213,13 +213,59 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
213 213
 
214 214
 	// Twiddle the destination when its a relative path - meaning, make it
215 215
 	// relative to the WORKINGDIR
216
-	if !system.IsAbs(dest) {
217
-		hasSlash := strings.HasSuffix(dest, string(os.PathSeparator))
218
-		dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(b.runConfig.WorkingDir), dest)
219 216
 
220
-		// Make sure we preserve any trailing slash
221
-		if hasSlash {
222
-			dest += string(os.PathSeparator)
217
+	endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator))
218
+
219
+	if runtime.GOOS == "windows" {
220
+		// On Windows, this is more complicated. We are guaranteed that the
221
+		// WorkingDir is already platform consistent meaning in the format
222
+		// UPPERCASEDriveLetter-Colon-Backslash-Foldername. However, Windows
223
+		// for now also has the limitation that ADD/COPY can only be done to
224
+		// the C: (system) drive, not any drives that might be present as a
225
+		// result of bind mounts.
226
+		//
227
+		// So... if the path specified is Linux-style absolute (/foo or \\foo),
228
+		// we assume it is the system drive. If it is a Windows-style absolute
229
+		// (DRIVE:\\foo), error if DRIVE is not C. And finally, ensure we
230
+		// strip any configured working directories drive letter so that it
231
+		// can be subsequently legitimately converted to a Windows volume-style
232
+		// pathname.
233
+
234
+		// Not a typo - filepath.IsAbs, not system.IsAbs on this next check as
235
+		// we only want to validate where the DriveColon part has been supplied.
236
+		if filepath.IsAbs(dest) {
237
+			if strings.ToUpper(string(dest[0])) != "C" {
238
+				return fmt.Errorf("Windows does not support %s with a destinations not on the system drive (C:)", cmdName)
239
+			}
240
+			dest = dest[2:] // Strip the drive letter
241
+		}
242
+
243
+		// Cannot handle relative where WorkingDir is not the system drive.
244
+		if len(b.runConfig.WorkingDir) > 0 {
245
+			if !system.IsAbs(b.runConfig.WorkingDir[2:]) {
246
+				return fmt.Errorf("Current WorkingDir %s is not platform consistent", b.runConfig.WorkingDir)
247
+			}
248
+			if !system.IsAbs(dest) {
249
+				if string(b.runConfig.WorkingDir[0]) != "C" {
250
+					return fmt.Errorf("Windows does not support %s with relative paths when WORKDIR is not the system drive", cmdName)
251
+				}
252
+
253
+				dest = filepath.Join(string(os.PathSeparator), b.runConfig.WorkingDir[2:], dest)
254
+
255
+				// Make sure we preserve any trailing slash
256
+				if endsInSlash {
257
+					dest += string(os.PathSeparator)
258
+				}
259
+			}
260
+		}
261
+	} else {
262
+		if !system.IsAbs(dest) {
263
+			dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(b.runConfig.WorkingDir), dest)
264
+
265
+			// Make sure we preserve any trailing slash
266
+			if endsInSlash {
267
+				dest += string(os.PathSeparator)
268
+			}
223 269
 		}
224 270
 	}
225 271
 
... ...
@@ -250,6 +250,13 @@ func (container *Container) GetResourcePath(path string) (string, error) {
250 250
 
251 251
 	cleanPath := cleanResourcePath(path)
252 252
 	r, e := symlink.FollowSymlinkInScope(filepath.Join(container.BaseFS, cleanPath), container.BaseFS)
253
+
254
+	// Log this here on the daemon side as there's otherwise no indication apart
255
+	// from the error being propagated all the way back to the client. This makes
256
+	// debugging significantly easier and clearly indicates the error comes from the daemon.
257
+	if e != nil {
258
+		logrus.Errorf("Failed to FollowSymlinkInScope BaseFS %s cleanPath %s path %s %s\n", container.BaseFS, cleanPath, path, e)
259
+	}
253 260
 	return r, e
254 261
 }
255 262
 
... ...
@@ -2074,22 +2074,13 @@ func (s *DockerSuite) TestBuildRelativeWorkdir(c *check.C) {
2074 2074
 		expected4     string
2075 2075
 		expectedFinal string
2076 2076
 	)
2077
-	// TODO Windows: The expectedFinal needs fixing to match Windows
2078
-	// filepath semantics. However, this is a non-trivial change. @jhowardmsft
2079
-	// Short story - need to make the configuration file platform semantically
2080
-	// consistent, so even if `WORKDIR /test2/test3` is specified in a Dockerfile,
2081
-	// the configuration should be stored as C:\test2\test3. Something similar to
2082
-	// 	if runtime.GOOS == "windows" && len(workdir) > 2 && string(workdir[0]) == `\` {
2083
-	//		workdir = "C:" + workdir
2084
-	//  }
2085
-	// in builder\dockerfile\dispatchers.go, function workdir(), but also
2086
-	// ironing out all other cases where this causes other failures.
2077
+
2087 2078
 	if daemonPlatform == "windows" {
2088 2079
 		expected1 = `C:/`
2089 2080
 		expected2 = `C:/test1`
2090 2081
 		expected3 = `C:/test2`
2091 2082
 		expected4 = `C:/test2/test3`
2092
-		expectedFinal = `\test2\test3`
2083
+		expectedFinal = `C:\test2\test3` // Note inspect is going to return Windows paths, as it's not in busybox
2093 2084
 	} else {
2094 2085
 		expected1 = `/`
2095 2086
 		expected2 = `/test1`
... ...
@@ -2117,12 +2108,238 @@ func (s *DockerSuite) TestBuildRelativeWorkdir(c *check.C) {
2117 2117
 	}
2118 2118
 }
2119 2119
 
2120
+// #22181 Regression test. Validates combinations of supported
2121
+// WORKDIR dockerfile directives in Windows and non-Windows semantics.
2122
+func (s *DockerSuite) TestBuildWindowsWorkdirProcessing(c *check.C) {
2123
+	testRequires(c, DaemonIsWindows)
2124
+	name := "testbuildwindowsworkdirprocessing"
2125
+	_, err := buildImage(name,
2126
+		`FROM busybox
2127
+		WORKDIR a
2128
+		RUN sh -c "[ "$PWD" = "C:/a" ]"
2129
+		WORKDIR c:\\foo
2130
+		RUN sh -c "[ "$PWD" = "C:/foo" ]"
2131
+		WORKDIR \\foo
2132
+		RUN sh -c "[ "$PWD" = "C:/foo" ]"
2133
+		WORKDIR /foo
2134
+		RUN sh -c "[ "$PWD" = "C:/foo" ]"
2135
+		WORKDIR C:/foo
2136
+		WORKDIR bar
2137
+		RUN sh -c "[ "$PWD" = "C:/foo/bar" ]"
2138
+		WORKDIR c:/foo
2139
+		WORKDIR bar
2140
+		RUN sh -c "[ "$PWD" = "C:/foo/bar" ]"
2141
+		WORKDIR c:/foo
2142
+		WORKDIR \\bar
2143
+		RUN sh -c "[ "$PWD" = "C:/bar" ]"
2144
+		WORKDIR /foo
2145
+		WORKDIR c:\\bar
2146
+		RUN sh -c "[ "$PWD" = "C:/bar" ]"
2147
+		`,
2148
+		true)
2149
+	if err != nil {
2150
+		c.Fatal(err)
2151
+	}
2152
+}
2153
+
2154
+// #22181 Regression test. Validates combinations of supported
2155
+// COPY dockerfile directives in Windows and non-Windows semantics.
2156
+func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) {
2157
+	testRequires(c, DaemonIsWindows)
2158
+	name := "testbuildwindowsaddcopypathprocessing"
2159
+	// TODO Windows (@jhowardmsft). Needs a follow-up PR to 22181 to
2160
+	// support backslash such as .\\ being equivalent to ./ and c:\\ being
2161
+	// equivalent to c:/. This is not currently (nor ever has been) supported
2162
+	// by docker on the Windows platform.
2163
+	dockerfile := `
2164
+		FROM busybox
2165
+			# First cases with no workdir, all end up in the root directory of the system drive
2166
+			COPY a1 ./
2167
+			ADD  a2 ./
2168
+			RUN sh -c "[ $(cat c:/a1) = 'helloa1' ]"
2169
+			RUN sh -c "[ $(cat c:/a2) = 'worlda2' ]"
2170
+
2171
+			COPY b1 /
2172
+			ADD  b2 /
2173
+			RUN sh -c "[ $(cat c:/b1) = 'hellob1' ]"
2174
+			RUN sh -c "[ $(cat c:/b2) = 'worldb2' ]"
2175
+
2176
+			COPY c1 c:/
2177
+			ADD  c2 c:/
2178
+			RUN sh -c "[ $(cat c:/c1) = 'helloc1' ]"
2179
+			RUN sh -c "[ $(cat c:/c2) = 'worldc2' ]"
2180
+
2181
+			COPY d1 c:/
2182
+			ADD  d2 c:/
2183
+			RUN sh -c "[ $(cat c:/d1) = 'hellod1' ]"
2184
+			RUN sh -c "[ $(cat c:/d2) = 'worldd2' ]"
2185
+
2186
+			COPY e1 .
2187
+			ADD  e2 .
2188
+			RUN sh -c "[ $(cat c:/e1) = 'helloe1' ]"
2189
+			RUN sh -c "[ $(cat c:/e2) = 'worlde2' ]"
2190
+			
2191
+			# Now with a workdir
2192
+			WORKDIR c:\\wa12
2193
+			COPY wa1 ./
2194
+			ADD wa2 ./
2195
+			RUN sh -c "[ $(cat c:/wa12/wa1) = 'hellowa1' ]"
2196
+			RUN sh -c "[ $(cat c:/wa12/wa2) = 'worldwa2' ]"
2197
+
2198
+			# No trailing slash on COPY/ADD, Linux-style path. 
2199
+			# Results in dir being changed to a file
2200
+			WORKDIR /wb1
2201
+			COPY wb1 .
2202
+			WORKDIR /wb2
2203
+			ADD wb2 .
2204
+			WORKDIR c:/
2205
+			RUN sh -c "[ $(cat c:/wb1) = 'hellowb1' ]"
2206
+			RUN sh -c "[ $(cat c:/wb2) = 'worldwb2' ]"
2207
+
2208
+			# No trailing slash on COPY/ADD, Windows-style path. 
2209
+			# Results in dir being changed to a file
2210
+			WORKDIR /wc1
2211
+			COPY wc1 c:/wc1
2212
+			WORKDIR /wc2
2213
+			ADD wc2 c:/wc2
2214
+			WORKDIR c:/
2215
+			RUN sh -c "[ $(cat c:/wc1) = 'hellowc1' ]"
2216
+			RUN sh -c "[ $(cat c:/wc2) = 'worldwc2' ]"			
2217
+
2218
+			# Trailing slash on COPY/ADD, Windows-style path. 
2219
+			WORKDIR /wd1
2220
+			COPY wd1 c:/wd1/
2221
+			WORKDIR /wd2
2222
+			ADD wd2 c:/wd2/
2223
+			RUN sh -c "[ $(cat c:/wd1/wd1) = 'hellowd1' ]"
2224
+			RUN sh -c "[ $(cat c:/wd2/wd2) = 'worldwd2' ]"
2225
+			`
2226
+	ctx, err := fakeContext(dockerfile, map[string]string{
2227
+		"a1":  "helloa1",
2228
+		"a2":  "worlda2",
2229
+		"b1":  "hellob1",
2230
+		"b2":  "worldb2",
2231
+		"c1":  "helloc1",
2232
+		"c2":  "worldc2",
2233
+		"d1":  "hellod1",
2234
+		"d2":  "worldd2",
2235
+		"e1":  "helloe1",
2236
+		"e2":  "worlde2",
2237
+		"wa1": "hellowa1",
2238
+		"wa2": "worldwa2",
2239
+		"wb1": "hellowb1",
2240
+		"wb2": "worldwb2",
2241
+		"wc1": "hellowc1",
2242
+		"wc2": "worldwc2",
2243
+		"wd1": "hellowd1",
2244
+		"wd2": "worldwd2",
2245
+	})
2246
+	if err != nil {
2247
+		c.Fatal(err)
2248
+	}
2249
+	defer ctx.Close()
2250
+	_, err = buildImageFromContext(name, ctx, false)
2251
+	if err != nil {
2252
+		c.Fatal(err)
2253
+	}
2254
+}
2255
+
2256
+// #22181 Regression test.
2257
+func (s *DockerSuite) TestBuildWindowsCopyFailsNonSystemDrive(c *check.C) {
2258
+	testRequires(c, DaemonIsWindows)
2259
+	name := "testbuildwindowscopyfailsnonsystemdrive"
2260
+	dockerfile := `
2261
+		FROM busybox
2262
+		cOpY foo d:/
2263
+		`
2264
+	ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
2265
+	if err != nil {
2266
+		c.Fatal(err)
2267
+	}
2268
+	defer ctx.Close()
2269
+	_, err = buildImageFromContext(name, ctx, false)
2270
+	if err == nil {
2271
+		c.Fatal(err)
2272
+	}
2273
+	if !strings.Contains(err.Error(), "Windows does not support COPY with a destinations not on the system drive (C:)") {
2274
+		c.Fatal(err)
2275
+	}
2276
+}
2277
+
2278
+// #22181 Regression test.
2279
+func (s *DockerSuite) TestBuildWindowsCopyFailsWorkdirNonSystemDrive(c *check.C) {
2280
+	testRequires(c, DaemonIsWindows)
2281
+	name := "testbuildwindowscopyfailsworkdirsystemdrive"
2282
+	dockerfile := `
2283
+		FROM busybox
2284
+		WORKDIR d:/
2285
+		cOpY foo .
2286
+		`
2287
+	ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
2288
+	if err != nil {
2289
+		c.Fatal(err)
2290
+	}
2291
+	defer ctx.Close()
2292
+	_, err = buildImageFromContext(name, ctx, false)
2293
+	if err == nil {
2294
+		c.Fatal(err)
2295
+	}
2296
+	if !strings.Contains(err.Error(), "Windows does not support COPY with relative paths when WORKDIR is not the system drive") {
2297
+		c.Fatal(err)
2298
+	}
2299
+}
2300
+
2301
+// #22181 Regression test.
2302
+func (s *DockerSuite) TestBuildWindowsAddFailsNonSystemDrive(c *check.C) {
2303
+	testRequires(c, DaemonIsWindows)
2304
+	name := "testbuildwindowsaddfailsnonsystemdrive"
2305
+	dockerfile := `
2306
+		FROM busybox
2307
+		AdD foo d:/
2308
+		`
2309
+	ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
2310
+	if err != nil {
2311
+		c.Fatal(err)
2312
+	}
2313
+	defer ctx.Close()
2314
+	_, err = buildImageFromContext(name, ctx, false)
2315
+	if err == nil {
2316
+		c.Fatal(err)
2317
+	}
2318
+	if !strings.Contains(err.Error(), "Windows does not support ADD with a destinations not on the system drive (C:)") {
2319
+		c.Fatal(err)
2320
+	}
2321
+}
2322
+
2323
+// #22181 Regression test.
2324
+func (s *DockerSuite) TestBuildWindowsAddFailsWorkdirNonSystemDrive(c *check.C) {
2325
+	testRequires(c, DaemonIsWindows)
2326
+	name := "testbuildwindowsaddfailsworkdirsystemdrive"
2327
+	dockerfile := `
2328
+		FROM busybox
2329
+		WORKDIR d:/
2330
+		AdD foo .
2331
+		`
2332
+	ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"})
2333
+	if err != nil {
2334
+		c.Fatal(err)
2335
+	}
2336
+	defer ctx.Close()
2337
+	_, err = buildImageFromContext(name, ctx, false)
2338
+	if err == nil {
2339
+		c.Fatal(err)
2340
+	}
2341
+	if !strings.Contains(err.Error(), "Windows does not support ADD with relative paths when WORKDIR is not the system drive") {
2342
+		c.Fatal(err)
2343
+	}
2344
+}
2345
+
2120 2346
 func (s *DockerSuite) TestBuildWorkdirWithEnvVariables(c *check.C) {
2121 2347
 	name := "testbuildworkdirwithenvvariables"
2122 2348
 
2123 2349
 	var expected string
2124 2350
 	if daemonPlatform == "windows" {
2125
-		expected = `\test1\test2`
2351
+		expected = `C:\test1\test2`
2126 2352
 	} else {
2127 2353
 		expected = `/test1/test2`
2128 2354
 	}