From 0029055060c6e588d8a29499e7367772fa8311ab Mon Sep 17 00:00:00 2001 From: Halvard Skogsrud Date: Sat, 13 Aug 2022 05:52:33 +1000 Subject: [PATCH] feat(ko): Inferred file sync for ko static assets (#7728) This change adds inferred file sync mode to the ko builder. Skaffold syncs static assets from the workspace to a running container, based on the `/kodata/` convention: https://github.com/google/ko#static-assets Tracking: #7131 --- .../en/docs/pipeline-stages/builders/ko.md | 41 +++- .../en/docs/pipeline-stages/filesync.md | 25 ++- .../en/docs/pipeline-stages/builders/ko.md | 41 +++- .../en/docs/pipeline-stages/filesync.md | 25 ++- integration/examples/ko-sync-infer/README.md | 18 ++ integration/examples/ko-sync-infer/go.mod | 17 ++ .../examples/ko-sync-infer/k8s/example.yaml | 45 +++++ .../examples/ko-sync-infer/kodata/index.html | 27 +++ integration/examples/ko-sync-infer/main.go | 43 +++++ .../examples/ko-sync-infer/skaffold.yaml | 42 +++++ pkg/skaffold/build/ko/sync/sync.go | 108 +++++++++++ pkg/skaffold/build/ko/sync/sync_test.go | 176 ++++++++++++++++++ pkg/skaffold/schema/validation/validation.go | 26 +++ .../schema/validation/validation_test.go | 84 ++++++++- pkg/skaffold/sync/sync.go | 16 ++ pkg/skaffold/sync/sync_test.go | 37 ++++ 16 files changed, 750 insertions(+), 21 deletions(-) create mode 100644 integration/examples/ko-sync-infer/README.md create mode 100644 integration/examples/ko-sync-infer/go.mod create mode 100644 integration/examples/ko-sync-infer/k8s/example.yaml create mode 100644 integration/examples/ko-sync-infer/kodata/index.html create mode 100644 integration/examples/ko-sync-infer/main.go create mode 100644 integration/examples/ko-sync-infer/skaffold.yaml create mode 100644 pkg/skaffold/build/ko/sync/sync.go create mode 100644 pkg/skaffold/build/ko/sync/sync_test.go diff --git a/docs-v2/content/en/docs/pipeline-stages/builders/ko.md b/docs-v2/content/en/docs/pipeline-stages/builders/ko.md index 86c5ac7c498..72546680b50 100644 --- a/docs-v2/content/en/docs/pipeline-stages/builders/ko.md +++ b/docs-v2/content/en/docs/pipeline-stages/builders/ko.md @@ -389,7 +389,46 @@ To learn more about how Skaffold debugs Go applications, read the ### File sync -File `sync` is not supported while the ko builder feature is in Alpha. +The `ko` builder can +[sync files to a running container]({{< relref "/docs/pipeline-stages/filesync" >}}) +when you run `skaffold dev`. + +The sync feature for the `ko` builder only works for +[static assets bundled with the container image](https://github.com/google/ko#static-assets). + +Use `infer` mode to specify patterns for the files you want to sync. The +infer patterns are relative to the `context` directory. + +For instance, if your main package is in the `context` directory, you can use +this configuration to sync all the static files bundled with the container +image: + +```yaml + sync: + infer: + - kodata/**/* +``` + +Note that the file sync feature requires the `tar` command to be available in +the container. The default `ko` builder base image does not include the `tar` +command. Use the `fromImage` field in the `ko` builder configuration in your +`skaffold.yaml` file to specify a base image that contains the `tar` command, +such as `gcr.io/distroless/base:debug`. + +You can use [profiles]({{< relref "/docs/environment/profiles" >}}) with +activation by command to override the `fromImage` value only when running +`skaffold dev`, such as in this example: + +```yaml +profiles: +- name: sync + activation: + - command: dev + patches: + - op: add + path: /build/artifacts/0/ko/fromImage + value: gcr.io/distroless/base:debug +``` ### Remote builds diff --git a/docs-v2/content/en/docs/pipeline-stages/filesync.md b/docs-v2/content/en/docs/pipeline-stages/filesync.md index c641d32dbbf..f571cc484df 100644 --- a/docs-v2/content/en/docs/pipeline-stages/filesync.md +++ b/docs-v2/content/en/docs/pipeline-stages/filesync.md @@ -14,11 +14,12 @@ This tar file is sent to and extracted on the corresponding containers. Multiple types of sync are supported by Skaffold: + `manual`: The user must specify both the files in their local workspace and the destination in the running container. - This is supported by every type of artifact. + This sync mode is supported by every type of artifact. + `infer`: The destinations for each changed file is inferred from the builder. The docker and kaniko builders examine instructions in a Dockerfile. This inference is also supported for custom artifacts that **explicitly declare a dependency on a Dockerfile.** + The ko builder can sync static content using this sync mode. + `auto`: Skaffold automatically configures the sync. This mode is only supported by Jib and Buildpacks artifacts. Auto sync mode is enabled by default for Buildpacks artifacts. @@ -46,13 +47,18 @@ The following example showcases manual filesync: ### Inferred sync mode -For docker artifacts, Skaffold knows how to infer the desired destination from the artifact's `Dockerfile` +For Docker artifacts, Skaffold knows how to infer the desired destination from the artifact's `Dockerfile` by examining the `ADD` and `COPY` instructions. -To enable syncing, you only need to specify which files are eligible for syncing in the sync rules. -The sync rules for inferred sync mode is just a list of glob patterns. -The following example showcases this filesync mode: -Given a Dockerfile such as +For Ko artifacts, Skaffold infers the destination from the structure of your +codebase. + +To enable syncing, you specify which files are eligible for syncing in the sync rules. +The sync rules for inferred sync mode is a list of glob patterns. + +The following example showcases this filesync mode for Docker artifacts: + +Given this Dockerfile: ```Dockerfile FROM hugo @@ -74,12 +80,15 @@ And a `skaffold.yaml` with the following sync configuration: - The last rule enables synchronization for all `md` files below the `content/en`. For example, `content/en/sub/index.md` ↷ `content/sub/index.md` but _not_ `content/en_GB/index.md`. -Inferred sync mode only applies to modified and added files. -File deletion will always cause a complete rebuild. +For Docker artifacts, inferred sync mode only applies to modified and added +files; file deletion will cause a complete rebuild. For multi-stage Dockerfiles, Skaffold only examines the last stage. Use manual sync rules to sync file copies from other stages. +[Ko artifacts supports syncing static content]({{}}), +and the sync rules apply to added, modified, and deleted files. + ### Auto sync mode In auto sync mode, Skaffold automatically generates sync rules for known file types. diff --git a/docs/content/en/docs/pipeline-stages/builders/ko.md b/docs/content/en/docs/pipeline-stages/builders/ko.md index 86c5ac7c498..72546680b50 100644 --- a/docs/content/en/docs/pipeline-stages/builders/ko.md +++ b/docs/content/en/docs/pipeline-stages/builders/ko.md @@ -389,7 +389,46 @@ To learn more about how Skaffold debugs Go applications, read the ### File sync -File `sync` is not supported while the ko builder feature is in Alpha. +The `ko` builder can +[sync files to a running container]({{< relref "/docs/pipeline-stages/filesync" >}}) +when you run `skaffold dev`. + +The sync feature for the `ko` builder only works for +[static assets bundled with the container image](https://github.com/google/ko#static-assets). + +Use `infer` mode to specify patterns for the files you want to sync. The +infer patterns are relative to the `context` directory. + +For instance, if your main package is in the `context` directory, you can use +this configuration to sync all the static files bundled with the container +image: + +```yaml + sync: + infer: + - kodata/**/* +``` + +Note that the file sync feature requires the `tar` command to be available in +the container. The default `ko` builder base image does not include the `tar` +command. Use the `fromImage` field in the `ko` builder configuration in your +`skaffold.yaml` file to specify a base image that contains the `tar` command, +such as `gcr.io/distroless/base:debug`. + +You can use [profiles]({{< relref "/docs/environment/profiles" >}}) with +activation by command to override the `fromImage` value only when running +`skaffold dev`, such as in this example: + +```yaml +profiles: +- name: sync + activation: + - command: dev + patches: + - op: add + path: /build/artifacts/0/ko/fromImage + value: gcr.io/distroless/base:debug +``` ### Remote builds diff --git a/docs/content/en/docs/pipeline-stages/filesync.md b/docs/content/en/docs/pipeline-stages/filesync.md index c641d32dbbf..f571cc484df 100644 --- a/docs/content/en/docs/pipeline-stages/filesync.md +++ b/docs/content/en/docs/pipeline-stages/filesync.md @@ -14,11 +14,12 @@ This tar file is sent to and extracted on the corresponding containers. Multiple types of sync are supported by Skaffold: + `manual`: The user must specify both the files in their local workspace and the destination in the running container. - This is supported by every type of artifact. + This sync mode is supported by every type of artifact. + `infer`: The destinations for each changed file is inferred from the builder. The docker and kaniko builders examine instructions in a Dockerfile. This inference is also supported for custom artifacts that **explicitly declare a dependency on a Dockerfile.** + The ko builder can sync static content using this sync mode. + `auto`: Skaffold automatically configures the sync. This mode is only supported by Jib and Buildpacks artifacts. Auto sync mode is enabled by default for Buildpacks artifacts. @@ -46,13 +47,18 @@ The following example showcases manual filesync: ### Inferred sync mode -For docker artifacts, Skaffold knows how to infer the desired destination from the artifact's `Dockerfile` +For Docker artifacts, Skaffold knows how to infer the desired destination from the artifact's `Dockerfile` by examining the `ADD` and `COPY` instructions. -To enable syncing, you only need to specify which files are eligible for syncing in the sync rules. -The sync rules for inferred sync mode is just a list of glob patterns. -The following example showcases this filesync mode: -Given a Dockerfile such as +For Ko artifacts, Skaffold infers the destination from the structure of your +codebase. + +To enable syncing, you specify which files are eligible for syncing in the sync rules. +The sync rules for inferred sync mode is a list of glob patterns. + +The following example showcases this filesync mode for Docker artifacts: + +Given this Dockerfile: ```Dockerfile FROM hugo @@ -74,12 +80,15 @@ And a `skaffold.yaml` with the following sync configuration: - The last rule enables synchronization for all `md` files below the `content/en`. For example, `content/en/sub/index.md` ↷ `content/sub/index.md` but _not_ `content/en_GB/index.md`. -Inferred sync mode only applies to modified and added files. -File deletion will always cause a complete rebuild. +For Docker artifacts, inferred sync mode only applies to modified and added +files; file deletion will cause a complete rebuild. For multi-stage Dockerfiles, Skaffold only examines the last stage. Use manual sync rules to sync file copies from other stages. +[Ko artifacts supports syncing static content]({{}}), +and the sync rules apply to added, modified, and deleted files. + ### Auto sync mode In auto sync mode, Skaffold automatically generates sync rules for known file types. diff --git a/integration/examples/ko-sync-infer/README.md b/integration/examples/ko-sync-infer/README.md new file mode 100644 index 00000000000..841a7fec8b3 --- /dev/null +++ b/integration/examples/ko-sync-infer/README.md @@ -0,0 +1,18 @@ +### Example: inferred file sync using the ko builder + +This example uses +[inferred file sync](https://skaffold.dev/docs/pipeline-stages/filesync/#inferred-sync-mode) +for static assets with the +[`ko` builder](https://skaffold.dev/docs/pipeline-stages/builders/ko/) +for a Go web app. + +To observe the behavior of file sync, run this command: + +```shell +skaffold dev +``` + +Try changing the HTML file in the `kodata` directory to see how Skaffold +syncs the file. + +If change the the `main.go` file, Skaffold will rebuild and redeploy the image. diff --git a/integration/examples/ko-sync-infer/go.mod b/integration/examples/ko-sync-infer/go.mod new file mode 100644 index 00000000000..00bfaad19e5 --- /dev/null +++ b/integration/examples/ko-sync-infer/go.mod @@ -0,0 +1,17 @@ +// Copyright 2022 The Skaffold Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +module github.com/GoogleContainerTools/skaffold/examples/ko-sync-infer + +go 1.13 diff --git a/integration/examples/ko-sync-infer/k8s/example.yaml b/integration/examples/ko-sync-infer/k8s/example.yaml new file mode 100644 index 00000000000..58fd8e043d3 --- /dev/null +++ b/integration/examples/ko-sync-infer/k8s/example.yaml @@ -0,0 +1,45 @@ +# Copyright 2022 The Skaffold Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: Service +metadata: + name: ko-sync-infer +spec: + ports: + - name: http + port: 80 + targetPort: 8080 + selector: + app: ko-sync-infer + type: ClusterIP +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: ko-sync-infer +spec: + selector: + matchLabels: + app: ko-sync-infer + template: + metadata: + labels: + app: ko-sync-infer + spec: + containers: + - image: skaffold-ko-sync-infer + name: ko-sync-infer + ports: + - containerPort: 8080 diff --git a/integration/examples/ko-sync-infer/kodata/index.html b/integration/examples/ko-sync-infer/kodata/index.html new file mode 100644 index 00000000000..d87458dc857 --- /dev/null +++ b/integration/examples/ko-sync-infer/kodata/index.html @@ -0,0 +1,27 @@ + + + + + + + Skaffold ko builder inferred file sync example + + + +

Try changing this text in dev mode.

+ + diff --git a/integration/examples/ko-sync-infer/main.go b/integration/examples/ko-sync-infer/main.go new file mode 100644 index 00000000000..2699b16a530 --- /dev/null +++ b/integration/examples/ko-sync-infer/main.go @@ -0,0 +1,43 @@ +/* +Copyright 2022 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "log" + "net/http" + "os" +) + +const defaultStaticPath = "kodata" // the value to use for local development + +func main() { + if err := run(); err != nil { + fmt.Fprintf(os.Stderr, "%+v", err) + os.Exit(1) + } +} + +func run() error { + staticPath := os.Getenv("KO_DATA_PATH") + if staticPath == "" { + staticPath = defaultStaticPath + } + http.Handle("/", http.FileServer(http.Dir(staticPath))) + log.Println("Listening on port 8080") + return http.ListenAndServe(":8080", nil) +} diff --git a/integration/examples/ko-sync-infer/skaffold.yaml b/integration/examples/ko-sync-infer/skaffold.yaml new file mode 100644 index 00000000000..503056c38b1 --- /dev/null +++ b/integration/examples/ko-sync-infer/skaffold.yaml @@ -0,0 +1,42 @@ +# Copyright 2022 The Skaffold Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: skaffold/v3alpha1 +kind: Config +build: + artifacts: + - image: skaffold-ko-sync-infer + ko: + dependencies: + paths: + - "**/*.go" + - go.* + - kodata/**/* + sync: + infer: + - kodata/**/* +portForward: +- resourceType: Deployment + resourceName: ko-sync-infer + port: 8080 + localPort: 8080 +# override the default base image with one that supports file sync when running 'skaffold dev' +profiles: +- name: sync + activation: + - command: dev + patches: + - op: add + path: /build/artifacts/0/ko/fromImage + value: gcr.io/distroless/base:debug diff --git a/pkg/skaffold/build/ko/sync/sync.go b/pkg/skaffold/build/ko/sync/sync.go new file mode 100644 index 00000000000..42fc0d55789 --- /dev/null +++ b/pkg/skaffold/build/ko/sync/sync.go @@ -0,0 +1,108 @@ +/* +Copyright 2022 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sync + +import ( + "context" + "fmt" + "path/filepath" + "strings" + + "github.com/bmatcuk/doublestar" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/filemon" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" +) + +// kodataRoot is the directory in the container where ko places static assets. +// See https://github.com/google/ko/blob/2f230b88c4891ee3a71b01c1fa65e85e8d6b5f5b/README.md#static-assets +// and https://github.com/google/ko/blob/2f230b88c4891ee3a71b01c1fa65e85e8d6b5f5b/pkg/build/gobuild.go#L514 +const kodataRoot = "/var/run/ko" + +// Infer syncs static content in the kodata directory based on matching file name patterns. +// It returns maps of files to be copied and deleted. +func Infer(ctx context.Context, a *latest.Artifact, e filemon.Events) (toCopy map[string][]string, toDelete map[string][]string, err error) { + toCopy, err = inferSync(ctx, a, append(e.Modified, e.Added...)) + if err != nil { + return nil, nil, err + } + toDelete, err = inferSync(ctx, a, e.Deleted) + if err != nil { + return nil, nil, err + } + return +} + +// inferSync determines if the files match any of the inferred file sync patterns configured for the artifact. +// For files that matches at least one pattern, the function determines the destination path. +// The return value is a map of source file location to destination path. +func inferSync(ctx context.Context, a *latest.Artifact, files []string) (map[string][]string, error) { + localBasePath, err := findLocalKodataPath(a) + if err != nil { + return nil, err + } + toSync := map[string][]string{} + for _, f := range files { + dest, err := syncDest(f, a.Workspace, localBasePath, a.Sync.Infer) + if err != nil { + return nil, err + } + if dest != "" { + log.Entry(ctx).Debugf("Syncing %q to %q", f, dest) + toSync[f] = []string{dest} + } else { + log.Entry(ctx).Debugf("File %q does not match any sync pattern. Skipping sync", f) + } + } + return toSync, nil +} + +// syncDest returns the destination file paths if the input file path matches at least one of the patterns. +// If the file doesn't match any of the patterns, the function returns zero values. +func syncDest(f string, workspace string, localBasePath string, patterns []string) (string, error) { + relPath, err := filepath.Rel(workspace, f) + if err != nil { + return "", err + } + for _, p := range patterns { + matches, err := doublestar.PathMatch(filepath.FromSlash(p), relPath) + if err != nil { + return "", fmt.Errorf("pattern error for file %q and pattern %s: %w", relPath, p, err) + } + if matches { + // find path to file relative to local static file directory + localFile, err := filepath.Rel(localBasePath, f) + if err != nil { + return "", fmt.Errorf("relative path error for path %q and file %q: %w", localBasePath, f, err) + } + dest := strings.ReplaceAll(filepath.Join(kodataRoot, localFile), "\\", "/") + return dest, nil + } + } + return "", nil +} + +// findLocalKodataPath returns the local path to static content for ko artifacts. +func findLocalKodataPath(a *latest.Artifact) (string, error) { + if strings.Contains(a.KoArtifact.Main, "...") { + // this error should be caught by validation earlier + return "", fmt.Errorf("unable to infer file sync when ko.main contains the '...' wildcard") + } + path := filepath.Join(a.Workspace, a.KoArtifact.Dir, a.KoArtifact.Main, "kodata") + return path, nil +} diff --git a/pkg/skaffold/build/ko/sync/sync_test.go b/pkg/skaffold/build/ko/sync/sync_test.go new file mode 100644 index 00000000000..12c946f69b1 --- /dev/null +++ b/pkg/skaffold/build/ko/sync/sync_test.go @@ -0,0 +1,176 @@ +/* +Copyright 2022 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package sync + +import ( + "context" + "path/filepath" + "testing" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestInferSync(t *testing.T) { + tests := []struct { + description string + artifact *latest.Artifact + files []string + expected map[string][]string + }{ + { + description: "matching and non-matching files", + artifact: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{}, + }, + Sync: &latest.Sync{ + Infer: []string{"kodata/**/*.js"}, + }, + }, + files: []string{ + "kodata/foo.js", + "kodata/bar.css", + "kodata/baz/frob.js", + "main.go", + }, + expected: map[string][]string{ + "kodata/foo.js": {"/var/run/ko/foo.js"}, + "kodata/baz/frob.js": {"/var/run/ko/baz/frob.js"}, + }, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + actual, err := inferSync(context.TODO(), test.artifact, test.files) + t.CheckNoError(err) + t.CheckDeepEqual(test.expected, actual) + }) + } +} + +func TestSyncDest(t *testing.T) { + tests := []struct { + description string + file string + workspace string + localBasePath string + patterns []string + expectedDest string + wantErr bool + }{ + { + description: "matching pattern simple", + file: "kodata/page.html", + localBasePath: "kodata", + patterns: []string{"kodata/page.html"}, + expectedDest: "/var/run/ko/page.html", + }, + { + description: "matching pattern doublestar", + file: "kodata/page.html", + localBasePath: "kodata", + patterns: []string{"kodata/**/*"}, + expectedDest: "/var/run/ko/page.html", + }, + { + description: "both non-matching and matching pattern", + file: "kodata/page.html", + localBasePath: "kodata", + patterns: []string{"kodata/*.css", "kodata/**/*"}, + expectedDest: "/var/run/ko/page.html", + }, + { + description: "no matching pattern", + file: "kodata/page.html", + localBasePath: "kodata", + expectedDest: "", + }, + { + description: "non-default directories", + file: "workspace/cmd/foo/kodata/page.html", + workspace: "workspace", + localBasePath: "workspace/cmd/foo/kodata", + patterns: []string{"cmd/foo/kodata/**/*"}, + expectedDest: "/var/run/ko/page.html", + }, + { + description: "error on invalid pattern", + file: "kodata/page.html", + localBasePath: "kodata", + patterns: []string{"["}, + wantErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + actualDest, err := syncDest(test.file, test.workspace, test.localBasePath, test.patterns) + t.CheckError(test.wantErr, err) + t.CheckDeepEqual(test.expectedDest, actualDest) + }) + } +} + +func TestFindLocalKodataPath(t *testing.T) { + tests := []struct { + description string + artifact *latest.Artifact + expectedPath string + wantErr bool + }{ + { + description: "default", + artifact: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{}, + }, + }, + expectedPath: "kodata", + }, + { + description: "all values set", + artifact: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{ + Dir: "dir", + Main: "main", + }, + }, + Workspace: "workspace", + }, + expectedPath: filepath.Join("workspace", "dir", "main", "kodata"), + }, + { + description: "error due to main wildcard", + artifact: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{ + Main: "./...", + }, + }, + }, + wantErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + actualPath, err := findLocalKodataPath(test.artifact) + t.CheckError(test.wantErr, err) + t.CheckDeepEqual(test.expectedPath, actualPath) + }) + } +} diff --git a/pkg/skaffold/schema/validation/validation.go b/pkg/skaffold/schema/validation/validation.go index b70e3a4c595..58ac6e1fd3a 100644 --- a/pkg/skaffold/schema/validation/validation.go +++ b/pkg/skaffold/schema/validation/validation.go @@ -80,6 +80,7 @@ func ProcessToErrorWithLocation(configs parser.SkaffoldConfigSet, validateConfig errs = append(errs, validateSyncRules(config, config.Build.Artifacts)...) errs = append(errs, validatePortForwardResources(config, config.PortForward)...) errs = append(errs, validateJibPluginTypes(config, config.Build.Artifacts)...) + errs = append(errs, validateKoSync(config, config.Build.Artifacts)...) errs = append(errs, validateLogPrefix(config, config.Deploy.Logs)...) errs = append(errs, validateArtifactTypes(config, config.Build)...) errs = append(errs, validateTaggingPolicy(config, config.Build)...) @@ -606,6 +607,31 @@ func validateJibPluginTypes(cfg *parser.SkaffoldConfigEntry, artifacts []*latest return } +// validateKoSync ensures that infer sync patterns contain the `kodata` string, since infer sync for the ko builder only supports static assets. +func validateKoSync(cfg *parser.SkaffoldConfigEntry, artifacts []*latest.Artifact) []ErrorWithLocation { + var cfgErrs []ErrorWithLocation + for i, a := range artifacts { + if a.KoArtifact == nil || a.Sync == nil { + continue + } + if len(a.Sync.Infer) > 0 && strings.Contains(a.KoArtifact.Main, "...") { + cfgErrs = append(cfgErrs, ErrorWithLocation{ + Error: fmt.Errorf("artifact %s cannot use inferred file sync when the ko.main field contains the '...' wildcard. Instead, specify the path to the main package without using wildcards", a.ImageName), + Location: cfg.YAMLInfos.LocateField(cfg.Build.Artifacts[i].KoArtifact, "Main"), + }) + } + for _, pattern := range a.Sync.Infer { + if !strings.Contains(pattern, "kodata") { + cfgErrs = append(cfgErrs, ErrorWithLocation{ + Error: fmt.Errorf("artifact %s has an invalid pattern %s for inferred file sync with the ko builder. The pattern must specify the 'kodata' directory. For instance, if you want to sync all static content, and your main package is in the workspace directory, you can use the pattern 'kodata/**/*'", a.ImageName, pattern), + Location: cfg.YAMLInfos.LocateField(cfg.Build.Artifacts[i].Sync, "Infer"), + }) + } + } + } + return cfgErrs +} + // validateArtifactTypes checks that the artifact types are compatible with the specified builder. func validateArtifactTypes(cfg *parser.SkaffoldConfigEntry, bc latest.BuildConfig) []ErrorWithLocation { cfgErrs := []ErrorWithLocation{} diff --git a/pkg/skaffold/schema/validation/validation_test.go b/pkg/skaffold/schema/validation/validation_test.go index 9fc1b20a9bb..3fde31fde28 100644 --- a/pkg/skaffold/schema/validation/validation_test.go +++ b/pkg/skaffold/schema/validation/validation_test.go @@ -1155,6 +1155,82 @@ func TestValidateJibPluginType(t *testing.T) { } } +func TestValidateKoSync(t *testing.T) { + tests := []struct { + description string + artifacts []*latest.Artifact + wantErr bool + }{ + { + description: "basic infer sync with no errors", + artifacts: []*latest.Artifact{{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{}, + }, + ImageName: "test", + Sync: &latest.Sync{ + Infer: []string{"kodata/**/*"}, + }, + }}, + }, + { + description: "no error for wildcard in Main when no infer sync set up", + artifacts: []*latest.Artifact{{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{ + Main: "./...", + }, + }, + ImageName: "test", + }}, + }, + { + description: "error for wildcard in Main when infer sync is set up", + artifacts: []*latest.Artifact{{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{ + Main: "./...", + }, + }, + Sync: &latest.Sync{ + Infer: []string{"kodata/**/*"}, + }, + }}, + wantErr: true, + }, + { + description: "error for patterns outside kodata", + artifacts: []*latest.Artifact{{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{}, + }, + Sync: &latest.Sync{ + Infer: []string{"**/*"}, + }, + }}, + wantErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + cfg := &parser.SkaffoldConfigEntry{ + SkaffoldConfig: &latest.SkaffoldConfig{ + APIVersion: latest.Version, + Kind: "Config", + Pipeline: latest.Pipeline{ + Build: latest.BuildConfig{ + Artifacts: test.artifacts, + }, + }, + }, + YAMLInfos: configlocations.NewYAMLInfos(), + } + err := Process(parser.SkaffoldConfigSet{cfg}, Options{CheckDeploySource: false}) + t.CheckError(test.wantErr, err) + }) + } +} + func TestValidateLogsConfig(t *testing.T) { tests := []struct { prefix string @@ -1333,9 +1409,11 @@ func TestValidateAcyclicDependencies(t *testing.T) { // setDependencies constructs a graph of artifact dependencies using the map as an adjacency list representation of indices in the artifacts array. // For example: // m = { -// 0 : {1, 2}, -// 2 : {3}, -//} +// +// 0 : {1, 2}, +// 2 : {3}, +// +// } // implies that a[0] artifact depends on a[1] and a[2]; and a[2] depends on a[3]. func setDependencies(a []*latest.Artifact, d map[int][]int) { for k, dep := range d { diff --git a/pkg/skaffold/sync/sync.go b/pkg/skaffold/sync/sync.go index a3cad27f3f7..9914b1a52aa 100644 --- a/pkg/skaffold/sync/sync.go +++ b/pkg/skaffold/sync/sync.go @@ -34,6 +34,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/buildpacks" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib" + kosync "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/ko/sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/filemon" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph" @@ -106,6 +107,21 @@ func syncItem(ctx context.Context, a *latest.Artifact, tag string, e filemon.Eve } func inferredSyncItem(ctx context.Context, a *latest.Artifact, tag string, e filemon.Events, cfg docker.Config) (*Item, error) { + // the ko builder doesn't need or use a sync map + if a.KoArtifact != nil { + log.Entry(ctx).Debugf("ko inferred sync %+v", e) + toCopy, toDelete, err := kosync.Infer(ctx, a, e) + if err != nil { + return nil, err + } + return &Item{ + Image: tag, + Artifact: a, + Copy: toCopy, + Delete: toDelete, + }, nil + } + // deleted files are no longer contained in the syncMap, so we need to rebuild if len(e.Deleted) > 0 { return nil, nil diff --git a/pkg/skaffold/sync/sync_test.go b/pkg/skaffold/sync/sync_test.go index bde29a40d1d..5472559dd17 100644 --- a/pkg/skaffold/sync/sync_test.go +++ b/pkg/skaffold/sync/sync_test.go @@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ + package sync import ( @@ -747,6 +748,42 @@ func TestNewSyncItem(t *testing.T) { Delete: nil, }, }, + + // Infer with Ko + { + description: "infer: ko static assets", + artifact: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + KoArtifact: &latest.KoArtifact{}, + }, + ImageName: "test", + Sync: &latest.Sync{ + Infer: []string{"kodata/**/*"}, + }, + }, + builds: []graph.Artifact{{ + ImageName: "test", + Tag: "test:123", + }}, + evt: filemon.Events{ + Added: []string{filepath.Join("kodata", "foo", "bar.html")}, + Modified: []string{ + filepath.Join("kodata", "frob", "baz.js"), + "main.go", + }, + Deleted: []string{filepath.Join("kodata", "corge", "grault.css")}, + }, + expected: &Item{ + Image: "test:123", + Copy: map[string][]string{ + filepath.Join("kodata", "foo", "bar.html"): {"/var/run/ko/foo/bar.html"}, + filepath.Join("kodata", "frob", "baz.js"): {"/var/run/ko/frob/baz.js"}, + }, + Delete: map[string][]string{ + filepath.Join("kodata", "corge", "grault.css"): {"/var/run/ko/corge/grault.css"}, + }, + }, + }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) {