Improve syntax for artifact.sync config [1/3] (#1847)

* Improved pipeline config for artifact.sync

Currently, the pipeline config is a map of local glob pattern to destination directory.
This scheme has been extended with some magic sequences, making it difficult to understand.
This commit converts the sync map into a list of sync rules. All sync rules are consulted to determine the destination paths.

This prepares for further changes to the sync logic. Also see #1844
* Update sync spec config according to design proposal

- Wrap sync-rules under key 'sync.manual'
- Config changes:
- from -> src
- to -> dest
- flatten option removed and warn during schema upgrade if an incompatible pattern is migrated to the new schema version

* Add validation for sync rules
* Migrate sync config to new schema version
* Update filesync documentation

Extract example into sample snippet in order to have it tested.

* Run validation on doc examples
* Review comments: add test case and clarify example in docs
* Improve doc and error message wording

Signed-off-by: Cornelius Weig <[email protected]>

Author: corneliusweig

docs/content/en/docs/how-tos/filesync/_index.md

@@ -10,40 +10,38 @@ This page discusses how to set up file sync for files that don't require full re
 File sync is alpha and may change between releases.
 {{< /alert >}}
 
-Skaffold supports copying changed files to a deployed containers so as to avoid the need to
-rebuild, redeploy, and restart the corresponding pod.  The file copying is enabled
-by adding a `sync` section with _sync rules_ to the `artifact` in the `skaffold.yaml`.
-
-The following example will cause any changes to JavaScript files under the _context_ directory
-to be copied to the deployed container into the container's `WORKDIR`.
-
-```yaml
-apiVersion: skaffold/v1beta8
-kind: Config
-build:
-  artifacts:
-  - image: gcr.io/k8s-skaffold/node-example
-    context: node
-    sync:
-      '.filebaserc': .
-      '*.html': static
-      '**/*.png': assets
-      '***/*.md': content
-```
-A double-asterisk (`**/`) applies recursively to all subdirectories but flattens the result,
-stripping the subdirectory structure.
-A triple-asterisk (`***/`) applies recursively to all subdirectories but retains
-the subdirectory structure.  
-
-Under the hood, Skaffold monitors and creates a tar file with changed files that match
-the sync rules.  This tar file is sent and extracted on the corresponding containers. 
-
-### Limitations
+Skaffold supports copying changed files to a deployed container so as to avoid the need to rebuild, redeploy, and restart the corresponding pod.
+The file copying is enabled by adding a `sync` section with _sync rules_ to the `artifact` in the `skaffold.yaml`.
+Under the hood, Skaffold creates a tar file with changed files that match the sync rules.
+This tar file is sent to and extracted on the corresponding containers. 
+
+### Manual sync mode
+
+A manual sync rule must specify the `src` and `dest` field.
+The `src` field is a glob pattern to match files relative to the artifact _context_ directory, which may contain `**` to match nested files.
+The `dest` field is the absolute or relative destination path in the container.
+If the destination is a relative path, an absolute path will be inferred by prepending the path with the container's `WORKDIR`.
+By default, matched files are transplanted with their whole directory hierarchy below the artifact context directory onto the destination.
+The optional `strip` field can cut off some levels from the directory hierarchy.
+The following example showcases manual filesync:
+
+{{% readfile file="samples/filesync/filesync.yaml" %}}
+
+- The first rule synchronizes the file `.filebaserc` to the `/etc` folder in the container.
+- The second rule synchronizes all `html` files in the `static-html` folder into the `<WORKDIR>/static` folder in the container.
+  Note that this pattern does not match files in sub-folders below `static-html` (e.g. `static-html/a.html` but not `static-html/sub/a.html`).
+- The third rule synchronizes all `png` files from any sub-folder into the `assets` folder on the container.
+  For example, `img.png` ↷ `assets/img.png` or `sub/img.png` ↷ `assets/sub/img.png`.
+- The last rule synchronizes all `md` files below the `content/en` directory into the `content` folder on the container.
+  The `strip` directive ensures that only the directory hierarchy below `content/en` is re-created at the destination.
+  For example, `content/en/index.md` ↷ `content/index.md` or `content/en/sub/index.md` ↷ `content/sub/index.md`.
+
+Currently, there is only manual filesync mode, but a mode with destination inference is already in the making.
+
+## Limitations
 
 File sync has some limitations:
 
   - File sync can only update files that can be modified by the container's configured User ID.
   - File sync requires the `tar` command to be available in the container.
   - Only local source files can be synchronized: files created by the builder will not be copied.
-
-{{% todo 1076 %}}

docs/content/en/samples/filesync/filesync.yaml

@@ -0,0 +1,22 @@
+build:
+  artifacts:
+    - image: gcr.io/k8s-skaffold/node-example
+      context: node
+      sync:
+        manual:
+          # sync a single file into the `/etc` folder
+          - src: '.filebaserc'
+            dest: /etc
+
+          # sync files directly below `static-html` into `static/`
+          - src: 'static-html/*.html'
+            dest: static
+
+          # sync any `png` file into the assets folder
+          - src: '**/*.png'
+            dest: assets
+
+          # sync all `md` files from `content/en` into `content`
+          - src: 'content/en/**/*.md'
+            dest: content
+            strip: 'content/en/'

docs/content/en/schemas/v1beta10.json

@@ -65,16 +65,9 @@
               ]
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -106,16 +99,9 @@
               ]
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -148,16 +134,9 @@
               ]
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -190,16 +169,9 @@
               "x-intellij-html-description": "<em>alpha</em> builds images using the <a href=\"https://github.com/GoogleContainerTools/jib/tree/master/jib-maven-plugin\">Jib plugin for Maven</a>."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -232,16 +204,9 @@
               "x-intellij-html-description": "<em>alpha</em> builds images using the <a href=\"https://github.com/GoogleContainerTools/jib/tree/master/jib-gradle-plugin\">Jib plugin for Gradle</a>."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -274,16 +239,9 @@
               "x-intellij-html-description": "<em>alpha</em> builds images using <a href=\"https://github.com/GoogleContainerTools/kaniko\">kaniko</a>."
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -316,16 +274,9 @@
               ]
             },
             "sync": {
-              "additionalProperties": {
-                "type": "string"
-              },
-              "type": "object",
-              "description": "*alpha* local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified. This is a mapping of local files to sync to remote folders.",
-              "default": "{}",
-              "examples": [
-                "{\"*.py\": \".\", \"css/**/*.css\": \"app/css\"}"
-              ]
+              "$ref": "#/definitions/Sync",
+              "description": "*alpha* local files synced to pods instead of triggering an image build when modified.",
+              "x-intellij-html-description": "<em>alpha</em> local files synced to pods instead of triggering an image build when modified."
             }
           },
           "preferredOrder": [
@@ -1670,6 +1621,64 @@
       "description": "holds the fields parsed from the Skaffold configuration file (skaffold.yaml).",
       "x-intellij-html-description": "holds the fields parsed from the Skaffold configuration file (skaffold.yaml)."
     },
+    "Sync": {
+      "properties": {
+        "manual": {
+          "items": {
+            "$ref": "#/definitions/SyncRule"
+          },
+          "type": "array",
+          "description": "manual sync rules indicating the source and destination.",
+          "x-intellij-html-description": "manual sync rules indicating the source and destination."
+        }
+      },
+      "preferredOrder": [
+        "manual"
+      ],
+      "additionalProperties": false,
+      "description": "*alpha* specifies what files to sync into the container. This is a list of sync rules indicating the intent to sync for source files.",
+      "x-intellij-html-description": "<em>alpha</em> specifies what files to sync into the container. This is a list of sync rules indicating the intent to sync for source files."
+    },
+    "SyncRule": {
+      "required": [
+        "src",
+        "dest"
+      ],
+      "properties": {
+        "dest": {
+          "type": "string",
+          "description": "destination path in the container where the files should be synced to.",
+          "x-intellij-html-description": "destination path in the container where the files should be synced to.",
+          "examples": [
+            "\"app/\""
+          ]
+        },
+        "src": {
+          "type": "string",
+          "description": "a glob pattern to match local paths against.",
+          "x-intellij-html-description": "a glob pattern to match local paths against.",
+          "examples": [
+            "\"css/**/*.css\""
+          ]
+        },
+        "strip": {
+          "type": "string",
+          "description": "specifies the path prefix to remove from the source path when transplanting the files into the destination folder.",
+          "x-intellij-html-description": "specifies the path prefix to remove from the source path when transplanting the files into the destination folder.",
+          "examples": [
+            "\"css/\""
+          ]
+        }
+      },
+      "preferredOrder": [
+        "src",
+        "dest",
+        "strip"
+      ],
+      "additionalProperties": false,
+      "description": "specifies which local files to sync to remote folders.",
+      "x-intellij-html-description": "specifies which local files to sync to remote folders."
+    },
     "TagPolicy": {
       "properties": {
         "dateTime": {

examples/hot-reload/skaffold.yaml

@@ -1,15 +1,21 @@
-apiVersion: skaffold/v1beta9
+apiVersion: skaffold/v1beta10
 kind: Config
 build:
   artifacts:
   - image: gcr.io/k8s-skaffold/node-example
     context: node
     sync:
-      '**/*.js': .
+      manual:
+      - src: 'src/**/*.js'
+        dest: .
+        strip: src/
   - image: gcr.io/k8s-skaffold/python-reload
     context: python
     sync:
-      '**/*.py': .
+      manual:
+      - src: 'src/**/*.py'
+        dest: .
+        strip: src/
 deploy:
   kubectl:
     manifests:

examples/nodejs/skaffold.yaml

@@ -1,13 +1,15 @@
-apiVersion: skaffold/v1beta9
+apiVersion: skaffold/v1beta10
 kind: Config
 build:
   artifacts:
   - image: gcr.io/k8s-skaffold/node-example
     context: backend
     sync:
-      # Sync all the javascript files that are in the src folder 
-      # with the container src folder
-      'src/***/*.js': src/
+      manual:
+        # Sync all the javascript files that are in the src folder
+        # with the container src folder
+        - src: 'src/**/*.js'
+          dest: .
 deploy:
   kubectl:
     manifests:

integration/examples/hot-reload/skaffold.yaml

@@ -5,11 +5,17 @@ build:
   - image: gcr.io/k8s-skaffold/node-example
     context: node
     sync:
-      '**/*.js': .
+      manual:
+      - src: 'src/**/*.js'
+        dest: .
+        strip: src/
   - image: gcr.io/k8s-skaffold/python-reload
     context: python
     sync:
-      '**/*.py': .
+      manual:
+      - src: 'src/**/*.py'
+        dest: .
+        strip: src/
 deploy:
   kubectl:
     manifests:

integration/examples/nodejs/skaffold.yaml

@@ -8,9 +8,11 @@ build:
       buildArgs:
         SCRIPT: "{{.SCRIPT}}"
     sync:
-      # Sync all the javascript files that are in the src folder 
+      manual:
+      # Sync all the javascript files that are in the src folder
       # with the container src folder
-      'src/***/*.js': src/
+      - src: 'src/**/*.js'
+        dest: .
 deploy:
   kubectl:
     manifests:

integration/testdata/file-sync/skaffold.yaml

@@ -7,7 +7,9 @@ build:
   - image: gcr.io/k8s-skaffold/test-file-sync
     context: .
     sync:
-      '**/foo*' : /test
+      manual:
+      - src: '**/foo*'
+        dest: /test
 deploy:
  kubectl:
    manifests:

pkg/skaffold/runner/dev_test.go

@@ -346,8 +346,8 @@ func TestDevSync(t *testing.T) {
 			err := runner.Dev(context.Background(), ioutil.Discard, []*latest.Artifact{
 				{
 					ImageName: "img1",
-					Sync: map[string]string{
-						"file1": "file1",
+					Sync: &latest.Sync{
+						Manual: []*latest.SyncRule{{Src: "file1", Dest: "file1"}},
 					},
 				},
 				{