Feature Request: expanded glob support for sync

[bryanlarsen]

AFAICT, the cool new sync feature uses path/filepath#Glob for it's globbing. That function is very limited: no support for matching the separator (aka recursive matching), no support for 0 or 1 matches, et cetera. Would it be possible to use a more featureful globbing package?

[r2d4]

The sync feature should respect the same exact syntax as the Dockerfile I think.

We already parse the dockerfile COPY and ADD glob patterns in https://github.com/GoogleContainerTools/skaffold/blob/master/pkg/skaffold/docker/parse.go#L213-L257

I'm pretty sure applying that code to the sync patterns should work.

Somewhere along the lines util.ExpandPathsGlob was used there, but we should have one central glob matcher (for kubectl manifest list, sync list, and Dockerfile COPY/ADD parsing)

[r2d4]

That would give us https://docs.docker.com/engine/reference/builder/#copy style matching.

[bryanlarsen]

According to your linked docs, that's also only filepath.Match. I've currently got ten subdirectories in my source directory, so to synchronize both .ts & .tsx from those 10 and the root I'd need 22 rules, afaict. #1161 is limiting my testing of that. That's not a huge deal, but if anybody creates a new directory and forgets to add that to skaffold.yaml, they might be confused for a while...

[pehlert]

This might be a slightly different approach to it, but I think it's the only feasible approach for deeply nested projects. What do you think? #1266

[balopat]

The sync feature should respect the same exact syntax as the Dockerfile I think.

@r2d4 can you elaborate why is this?

[priyawadhwa]

Fixed by #1266

[jsdevtom]

@radityasurya

so it is not possible to pass multiple patterns?

I believe the cause of this is this function:

skaffold/pkg/skaffold/sync/sync.go

Lines 178 to 189 in 0bd43a4

	func segregateSyncMaps(syncMap map[string]string) (tripleStarPattern, doubleStarPattern map[string]string) {
	tripleStarPattern = make(map[string]string)
	doubleStarPattern = make(map[string]string)
	for p, dst := range syncMap {
	if strings.Contains(p, "***") {
	tripleStarPattern[p] = dst
	} else {
	doubleStarPattern[p] = dst
	}
	}
	return
	}

Notice how it returns only one tripleStarPattern. @priyawadhwa Shouldn't it be possible to determine multiple tripleStartPatterns? Like this:

build:
  local:
    push: false
  artifacts:
    - image: image_name
      context: .
      docker:
        dockerfile: ./client/Dockerfile.dev
      sync:
        '***/*.ts': .
        '***/*.scss': .
        '***/*.html': .

[jsdevtom]

For anyone else strugling with this issue, this work around (syncing all the things) works for me:

      sync:
        '***/*': .

[priyawadhwa]

Hey @jsdevtom I wasn't able to reproduce this bug, could you provide the repo you're using, or a sample repo? For me, multiple triple star patterns are working as expected, and from the code it looks like multiple triple star patterns can be added to the map.

[jsdevtom]

@priyawadhwa This is definitely reproducible on a project I am working on (but am not willing to share). But when I try it with the react-reload example project, I am unable to reproduce, I will see if I can narrow the problem down on the project I am working on.

[jsdevtom]

please feel free to close until I am able to reproduce