Handle simple glob patterns when upgrading the sync patterns

Also simple glob patterns are flattened at the destination. The translation to the new sync config was not handled correctly for cases such as: - public/*: /app/public - public/index.html: /app/public Signed-off-by: Cornelius Weig <[email protected]>
GoogleContainerTools · Jun 18, 2019 · 503ac0e · 503ac0e
1 parent 355fae5
commit 503ac0e
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 10 deletions.
diff --git a/docs/content/en/schemas/v1beta11.json b/docs/content/en/schemas/v1beta11.json
@@ -1674,8 +1674,8 @@
         },
         "src": {
           "type": "string",
-          "description": "a glob pattern to match local paths against.",
-          "x-intellij-html-description": "a glob pattern to match local paths against.",
+          "description": "a glob pattern to match local paths against. Directories should be delimited by `/` on all platforms.",
+          "x-intellij-html-description": "a glob pattern to match local paths against. Directories should be delimited by <code>/</code> on all platforms.",
           "examples": [
             "\"css/**/*.css\""
           ]

diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go
@@ -498,6 +498,7 @@ type Sync struct {
 // SyncRule specifies which local files to sync to remote folders.
 type SyncRule struct {
 	// Src is a glob pattern to match local paths against.
+	// Directories should be delimited by `/` on all platforms.
 	// For example: `"css/**/*.css"`.
 	Src string `yaml:"src,omitempty" yamltags:"required"`
 

diff --git a/pkg/skaffold/schema/v1beta9/upgrade.go b/pkg/skaffold/schema/v1beta9/upgrade.go
@@ -17,6 +17,7 @@ limitations under the License.
 package v1beta9
 
 import (
+	"regexp"
 	"strings"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
@@ -30,6 +31,11 @@ const (
 	incompatibleSyncWarning = `The semantics of sync has changed, the folder structure is no longer flattened but preserved (see https://skaffold.dev/docs/how-tos/filesync/). The likely impacted patterns in your skaffold yaml are: %s`
 )
 
+var (
+	// compatibleSimplePattern have a directory prefix without stars and a basename with at most one star.
+	compatibleSimplePattern = regexp.MustCompile(`^([^*]*/)?([^*/]*\*[^*/]*|[^*/]+)$`)
+)
+
 // Upgrade upgrades a configuration to the next version.
 // Config changes from v1beta9 to v1beta10
 // 1. Additions:
@@ -94,17 +100,25 @@ func (config *SkaffoldConfig) convertSyncRules() [][]*next.SyncRule {
 		newRules := make([]*next.SyncRule, 0, len(a.Sync))
 		for src, dest := range a.Sync {
 			var syncRule *next.SyncRule
-			if strings.Contains(src, "***") {
+			switch {
+			case compatibleSimplePattern.MatchString(src):
+				syncRule = &next.SyncRule{
+					Src:   src,
+					Dest:  dest,
+					Strip: compatibleSimplePattern.FindStringSubmatch(src)[1],
+				}
+			case strings.Contains(src, "***"):
 				syncRule = &next.SyncRule{
 					Src:   strings.Replace(src, "***", "**", -1),
 					Dest:  dest,
 					Strip: strings.Split(src, "***")[0],
 				}
-			} else {
-				// only patterns with '**' are incompatible
-				if strings.Contains(src, "**") {
-					incompatiblePatterns = append(incompatiblePatterns, src)
-				}
+			default:
+				// Incompatible patterns contain `**` or glob directories.
+				// Such patterns flatten the content at the destination which
+				// cannot be reproduced with the current config. For example:
+				// `/app/**/subdir/*.html`, `/app/*/*.html`
+				incompatiblePatterns = append(incompatiblePatterns, src)
 				syncRule = &next.SyncRule{
 					Src:  src,
 					Dest: dest,

diff --git a/pkg/skaffold/schema/v1beta9/upgrade_test.go b/pkg/skaffold/schema/v1beta9/upgrade_test.go
@@ -140,10 +140,33 @@ apiVersion: skaffold/v1beta9
 kind: Config
 build:
   artifacts:
+  - image: gcr.io/no-star-1
+    sync:
+      '/public/A/B/a.html': /www
+  - image: gcr.io/no-star-2
+    sync:
+      '/b.html': /www
+  - image: gcr.io/no-star-3
+    sync:
+      'c.html': /www
+  - image: gcr.io/no-star-4
+    sync:
+      'public/A/d.html': /www
+  - image: gcr.io/single-star-1
+    sync:
+      'public/*': /app
+  - image: gcr.io/single-star-2
+    sync:
+      'main*.js': /app
+  - image: gcr.io/single-star-3
+    sync:
+      '/public/b/*.js': /app
+  - image: gcr.io/single-star-4
+    sync:
+      '/c/prefix-*': /app
   - image: gcr.io/k8s-skaffold/node-example
     sync:
       '**/*.js': .
-  - image: gcr.io/k8s-skaffold/leeroy
   - image: gcr.io/k8s-skaffold/react-reload
     sync:
       'src/***/*.js': app/
@@ -158,12 +181,57 @@ apiVersion: skaffold/v1beta10
 kind: Config
 build:
   artifacts:
+  - image: gcr.io/no-star-1
+    sync:
+      manual:
+      - src: /public/A/B/a.html
+        dest: /www
+        strip: /public/A/B/
+  - image: gcr.io/no-star-2
+    sync:
+      manual:
+      - src: /b.html
+        dest: /www
+        strip: /
+  - image: gcr.io/no-star-3
+    sync:
+      manual:
+      - src: c.html
+        dest: /www
+  - image: gcr.io/no-star-4
+    sync:
+      manual:
+      - src: public/A/d.html
+        dest: /www
+        strip: public/A/
+  - image: gcr.io/single-star-1
+    sync:
+      manual:
+      - src: 'public/*'
+        dest: /app
+        strip: public/
+  - image: gcr.io/single-star-2
+    sync:
+      manual:
+      - src: 'main*.js'
+        dest: /app
+  - image: gcr.io/single-star-3
+    sync:
+      manual:
+      - src: '/public/b/*.js'
+        dest: /app
+        strip: /public/b/
+  - image: gcr.io/single-star-4
+    sync:
+      manual:
+      - src: '/c/prefix-*'
+        dest: /app
+        strip: /c/
   - image: gcr.io/k8s-skaffold/node-example
     sync:
       manual:
       - src: '**/*.js'
         dest: .
-  - image: gcr.io/k8s-skaffold/leeroy
   - image: gcr.io/k8s-skaffold/react-reload
     sync:
       manual: