diff --git a/pkg/skaffold/build/buildpacks/init.go b/pkg/skaffold/build/buildpacks/init.go index 1b7e9f31bc0..ab1a28ac12b 100644 --- a/pkg/skaffold/build/buildpacks/init.go +++ b/pkg/skaffold/build/buildpacks/init.go @@ -49,7 +49,7 @@ func (c ArtifactConfig) Describe() string { } // ArtifactType returns the type of the artifact to be built. -func (c ArtifactConfig) ArtifactType() latest.ArtifactType { +func (c ArtifactConfig) ArtifactType(_ string) latest.ArtifactType { return latest.ArtifactType{ BuildpackArtifact: &latest.BuildpackArtifact{ Builder: c.Builder, diff --git a/pkg/skaffold/build/buildpacks/init_test.go b/pkg/skaffold/build/buildpacks/init_test.go index bb40148dab2..80a072ca8c9 100644 --- a/pkg/skaffold/build/buildpacks/init_test.go +++ b/pkg/skaffold/build/buildpacks/init_test.go @@ -149,7 +149,7 @@ func TestArtifactType(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - at := test.config.ArtifactType() + at := test.config.ArtifactType("ignored") // buildpacks doesn't include file references in its artifacts t.CheckDeepEqual(test.expectedType, at) }) diff --git a/pkg/skaffold/build/jib/init.go b/pkg/skaffold/build/jib/init.go index 38aeb5e1a5f..1a1d482c018 100644 --- a/pkg/skaffold/build/jib/init.go +++ b/pkg/skaffold/build/jib/init.go @@ -59,7 +59,7 @@ func (c ArtifactConfig) Describe() string { } // ArtifactType returns the type of the artifact to be built. -func (c ArtifactConfig) ArtifactType() latest.ArtifactType { +func (c ArtifactConfig) ArtifactType(_ string) latest.ArtifactType { return latest.ArtifactType{ JibArtifact: &latest.JibArtifact{ Project: c.Project, diff --git a/pkg/skaffold/build/jib/init_test.go b/pkg/skaffold/build/jib/init_test.go index d479803287d..c7c257bfba1 100644 --- a/pkg/skaffold/build/jib/init_test.go +++ b/pkg/skaffold/build/jib/init_test.go @@ -242,7 +242,7 @@ func TestArtifactType(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - at := test.config.ArtifactType() + at := test.config.ArtifactType("ignored") // jib doesn't include file references in its artifacts t.CheckDeepEqual(test.expectedType, at) }) diff --git a/pkg/skaffold/docker/docker_init.go b/pkg/skaffold/docker/docker_init.go index 75c2b8f374b..1e57076292c 100644 --- a/pkg/skaffold/docker/docker_init.go +++ b/pkg/skaffold/docker/docker_init.go @@ -52,12 +52,19 @@ func (c ArtifactConfig) Describe() string { } // ArtifactType returns the type of the artifact to be built. -func (c ArtifactConfig) ArtifactType() latest.ArtifactType { +func (c ArtifactConfig) ArtifactType(workspace string) latest.ArtifactType { dockerfile := filepath.Base(c.File) + if workspace != "" { + // attempt to relativize the path + if rel, err := filepath.Rel(workspace, c.File); err == nil { + dockerfile = rel + } + } return latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{ - DockerfilePath: dockerfile, + // to make skaffold.yaml more portable across OS-es we should always generate /-delimited filePaths + DockerfilePath: filepath.ToSlash(dockerfile), }, } } diff --git a/pkg/skaffold/docker/docker_init_test.go b/pkg/skaffold/docker/docker_init_test.go index d4f8137b3b6..ca702e02a6f 100644 --- a/pkg/skaffold/docker/docker_init_test.go +++ b/pkg/skaffold/docker/docker_init_test.go @@ -89,6 +89,7 @@ func TestDescribe(t *testing.T) { func TestArtifactType(t *testing.T) { tests := []struct { description string + workspace string config ArtifactConfig expectedType latest.ArtifactType }{ @@ -110,10 +111,20 @@ func TestArtifactType(t *testing.T) { }, }, }, + { + description: "with workspace", + config: ArtifactConfig{File: filepath.Join("path", "to", "Dockerfile")}, + workspace: "path", + expectedType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + DockerfilePath: "to/Dockerfile", + }, + }, + }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - at := test.config.ArtifactType() + at := test.config.ArtifactType(test.workspace) t.CheckDeepEqual(test.expectedType, at) }) diff --git a/pkg/skaffold/initializer/analyze/analyze.go b/pkg/skaffold/initializer/analyze/analyze.go index c0a57c47cc5..c649e76c118 100644 --- a/pkg/skaffold/initializer/analyze/analyze.go +++ b/pkg/skaffold/initializer/analyze/analyze.go @@ -152,7 +152,7 @@ func (a *ProjectAnalysis) Analyze(dir string) error { } } - // to make skaffold.yaml more portable across OS-es we should always generate / based filePaths + // to make skaffold.yaml more portable across OS-es we should always generate /-delimited filePaths filePath = strings.ReplaceAll(filePath, string(os.PathSeparator), "/") for _, analyzer := range a.analyzers() { if err := analyzer.analyzeFile(filePath); err != nil { diff --git a/pkg/skaffold/initializer/analyze/builder.go b/pkg/skaffold/initializer/analyze/builder.go index d194d1fde9a..3cde8190dce 100644 --- a/pkg/skaffold/initializer/analyze/builder.go +++ b/pkg/skaffold/initializer/analyze/builder.go @@ -79,7 +79,8 @@ func (a *builderAnalyzer) detectBuilders(path string, detectJib bool) ([]build.I if strings.Contains(strings.ToLower(base), "dockerfile") { if docker.Validate(path) { results = append(results, docker.ArtifactConfig{ - File: path, + // Docker expects forward slashes (for Linux containers at least) + File: filepath.ToSlash(path), }) } } diff --git a/pkg/skaffold/initializer/build/builders.go b/pkg/skaffold/initializer/build/builders.go index b37db71e053..14ce761f6c3 100644 --- a/pkg/skaffold/initializer/build/builders.go +++ b/pkg/skaffold/initializer/build/builders.go @@ -36,8 +36,9 @@ type InitBuilder interface { // Must be unique between artifacts. Describe() string - // ArtifactType returns the type of the artifact to be built. - ArtifactType() latest.ArtifactType + // ArtifactType returns the type of the artifact to be built. Paths should be relative to the workspace. + // To make skaffold.yaml more portable across OS-es we should always generate /-delimited filepaths. + ArtifactType(workspace string) latest.ArtifactType // ConfiguredImage returns the target image configured by the builder, or an empty string if no image is configured. // This should be a cheap operation. diff --git a/pkg/skaffold/initializer/build/resolve.go b/pkg/skaffold/initializer/build/resolve.go index ab9cff60c86..9909285aa20 100644 --- a/pkg/skaffold/initializer/build/resolve.go +++ b/pkg/skaffold/initializer/build/resolve.go @@ -79,7 +79,7 @@ func (d *defaultBuildInitializer) resolveBuilderImagesForcefully() error { } func builderRank(builder InitBuilder) int { - a := builder.ArtifactType() + a := builder.ArtifactType("") switch { case a.DockerArtifact != nil: return 1 diff --git a/pkg/skaffold/initializer/build/util.go b/pkg/skaffold/initializer/build/util.go index c63d83e61ba..823b774abbb 100644 --- a/pkg/skaffold/initializer/build/util.go +++ b/pkg/skaffold/initializer/build/util.go @@ -65,17 +65,18 @@ func Artifacts(artifactInfos []ArtifactInfo) []*latest.Artifact { var artifacts []*latest.Artifact for _, info := range artifactInfos { - artifact := &latest.Artifact{ - ImageName: info.ImageName, - ArtifactType: info.Builder.ArtifactType(), - } - workspace := info.Workspace if workspace == "" { workspace = filepath.Dir(info.Builder.Path()) } + artifact := &latest.Artifact{ + ImageName: info.ImageName, + ArtifactType: info.Builder.ArtifactType(workspace), + } + if workspace != "." { - artifact.Workspace = workspace + // to make skaffold.yaml more portable across OS-es we should always generate /-delimited filepaths + artifact.Workspace = filepath.ToSlash(workspace) } artifacts = append(artifacts, artifact) diff --git a/pkg/skaffold/initializer/init_test.go b/pkg/skaffold/initializer/init_test.go index 42ef521a83d..68ed568a287 100644 --- a/pkg/skaffold/initializer/init_test.go +++ b/pkg/skaffold/initializer/init_test.go @@ -23,6 +23,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" @@ -35,6 +36,7 @@ import ( func TestDoInit(t *testing.T) { tests := []struct { name string + shouldSkip bool dir string config initconfig.Config expectedError string @@ -89,6 +91,20 @@ func TestDoInit(t *testing.T) { }, }, }, + { + name: "windows paths use forward slashes", + shouldSkip: runtime.GOOS != "windows", + dir: `testdata\init\windows`, + config: initconfig.Config{ + Force: true, + CliArtifacts: []string{ + `{"builder":"Docker","context":"apps\\web","payload":{"path":"apps\\web\\build\\Dockerfile"},"image":"gcr.io/k8s-skaffold/leeroy-web"}`, + }, + Opts: config.SkaffoldOptions{ + ConfigurationFile: "skaffold.yaml.out", + }, + }, + }, { name: "CLI artifacts + manifest placeholders", dir: "testdata/init/allcli", @@ -212,6 +228,10 @@ See https://skaffold.dev/docs/pipeline-stages/deployers/helm/ for a detailed gui } for _, test := range tests { testutil.Run(t, test.name, func(t *testutil.T) { + if test.shouldSkip { + t.Logf("Skipped test %q", test.name) + return + } t.Chdir(test.dir) err := DoInit(context.TODO(), os.Stdout, test.config) diff --git a/pkg/skaffold/initializer/testdata/init/windows/apps/web/build/Dockerfile b/pkg/skaffold/initializer/testdata/init/windows/apps/web/build/Dockerfile new file mode 100644 index 00000000000..14bded7fdcc --- /dev/null +++ b/pkg/skaffold/initializer/testdata/init/windows/apps/web/build/Dockerfile @@ -0,0 +1,7 @@ +FROM golang:1.12.9-alpine3.10 as builder +COPY web.go . +RUN go build -o /web . + +FROM alpine:3.10 +CMD ["./web"] +COPY --from=builder /web . diff --git a/pkg/skaffold/initializer/testdata/init/windows/apps/web/deployment.yaml b/pkg/skaffold/initializer/testdata/init/windows/apps/web/deployment.yaml new file mode 100644 index 00000000000..fd8ad0bc032 --- /dev/null +++ b/pkg/skaffold/initializer/testdata/init/windows/apps/web/deployment.yaml @@ -0,0 +1,37 @@ +apiVersion: v1 +kind: Service +metadata: + name: leeroy-web + labels: + app: leeroy-web +spec: + type: NodePort + ports: + - port: 8080 + targetPort: 8080 + protocol: TCP + name: leeroy-web + selector: + app: leeroy-web +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: leeroy-web + labels: + app: leeroy-web +spec: + replicas: 1 + selector: + matchLabels: + app: leeroy-web + template: + metadata: + labels: + app: leeroy-web + spec: + containers: + - name: leeroy-web + image: gcr.io/k8s-skaffold/leeroy-web + ports: + - containerPort: 8080 diff --git a/pkg/skaffold/initializer/testdata/init/windows/apps/web/web.go b/pkg/skaffold/initializer/testdata/init/windows/apps/web/web.go new file mode 100644 index 00000000000..69f92785651 --- /dev/null +++ b/pkg/skaffold/initializer/testdata/init/windows/apps/web/web.go @@ -0,0 +1,25 @@ +package main + +import ( + "io" + "net/http" + + "log" +) + +func handler(w http.ResponseWriter, r *http.Request) { + resp, err := http.Get("http://leeroy-app:50051") + if err != nil { + panic(err) + } + defer resp.Body.Close() + if _, err := io.Copy(w, resp.Body); err != nil { + panic(err) + } +} + +func main() { + log.Print("leeroy web server ready") + http.HandleFunc("/", handler) + http.ListenAndServe(":8080", nil) +} diff --git a/pkg/skaffold/initializer/testdata/init/windows/skaffold.yaml b/pkg/skaffold/initializer/testdata/init/windows/skaffold.yaml new file mode 100644 index 00000000000..913ffbd2e83 --- /dev/null +++ b/pkg/skaffold/initializer/testdata/init/windows/skaffold.yaml @@ -0,0 +1,14 @@ +apiVersion: skaffold/v2beta11 +kind: Config +metadata: + name: windows +build: + artifacts: + - image: gcr.io/k8s-skaffold/leeroy-web + context: apps/web + docker: + dockerfile: build/Dockerfile +deploy: + kubectl: + manifests: + - apps/web/deployment.yaml