From defda6aa9246c2bf57a755c821edc6070101f643 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Tue, 7 Jan 2020 17:56:40 -0500 Subject: [PATCH 1/5] Add common jib sync code for processing syncmap Converts json output from jib maven/gradle command to a syncmap that can be processed within skaffold --- pkg/skaffold/build/jib/sync.go | 94 +++++++++++++++++++ pkg/skaffold/build/jib/sync_test.go | 134 ++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+) create mode 100644 pkg/skaffold/build/jib/sync.go create mode 100644 pkg/skaffold/build/jib/sync_test.go diff --git a/pkg/skaffold/build/jib/sync.go b/pkg/skaffold/build/jib/sync.go new file mode 100644 index 00000000000..ca1fa247e0b --- /dev/null +++ b/pkg/skaffold/build/jib/sync.go @@ -0,0 +1,94 @@ +/* +Copyright 2019 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 jib + +import ( + "bytes" + "encoding/json" + "os" + "os/exec" + "regexp" + "time" + + "github.com/pkg/errors" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" +) + +type SyncMap map[string]SyncEntry + +type SyncEntry struct { + Dest []string + FileTime time.Time + IsDirect bool +} + +type JSONSyncMap struct { + Direct []JSONSyncEntry `json:"direct"` + Generated []JSONSyncEntry `json:"generated"` +} + +type JSONSyncEntry struct { + Src string `json:"src"` + Dest string `json:"dest"` +} + +func getSyncMapFromSystem(cmd *exec.Cmd) (*SyncMap, error) { + jsm := JSONSyncMap{} + stdout, err := util.RunCmdOut(cmd) + if err != nil { + return nil, errors.Wrap(err, "failed to get Jib sync map") + } + + // To parse the output, search for "BEGIN JIB JSON", then unmarshal the next line into the pathMap struct. + // Syncmap is transitioning to "BEGIN JIB JSON: SYNCMAP/1" starting in jib 2.0.0 + // perhaps this feature should only be included from 2.0.0 onwards? And we generally avoid this? + matches := regexp.MustCompile(`BEGIN JIB JSON(?:: SYNCMAP/1)?\r?\n({.*})`).FindSubmatch(stdout) + if len(matches) == 0 { + return nil, errors.New("failed to get Jib Sync data") + } + + line := bytes.Replace(matches[1], []byte(`\`), []byte(`\\`), -1) + if err := json.Unmarshal(line, &jsm); err != nil { + return nil, errors.WithStack(err) + } + + sm := make(SyncMap) + for _, de := range jsm.Direct { + info, err := os.Stat(de.Src) + if err != nil { + return nil, errors.Wrap(err, "could not obtain file mod time") + } + sm[de.Src] = SyncEntry{ + Dest: []string{de.Dest}, + FileTime: info.ModTime(), + IsDirect: true, + } + } + for _, ge := range jsm.Generated { + info, err := os.Stat(ge.Src) + if err != nil { + return nil, errors.Wrap(err, "could not obtain file mod time") + } + sm[ge.Src] = SyncEntry{ + Dest: []string{ge.Dest}, + FileTime: info.ModTime(), + IsDirect: false, + } + } + return &sm, nil +} diff --git a/pkg/skaffold/build/jib/sync_test.go b/pkg/skaffold/build/jib/sync_test.go new file mode 100644 index 00000000000..230df4809bb --- /dev/null +++ b/pkg/skaffold/build/jib/sync_test.go @@ -0,0 +1,134 @@ +/* +Copyright 2019 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 jib + +import ( + "fmt" + "os" + "os/exec" + "testing" + "time" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestGetSyncMapFromSystem(t *testing.T) { + tmpDir, cleanup := testutil.NewTempDir(t) + defer cleanup() + + tmpDir.Touch("dep1", "dir/dep2") + dep1 := tmpDir.Path("dep1") + dep2 := tmpDir.Path("dir/dep2") + + dep1Time := getFileTime(dep1, t) + dep2Time := getFileTime(dep2, t) + + dep1Target := "/target/dep1" + dep2Target := "/target/anotherDir/dep2" + + tests := []struct { + description string + stdout string + shouldErr bool + expected *SyncMap + }{ + { + description: "empty", + stdout: "", + shouldErr: true, + expected: nil, + }, + { + description: "old style marker", + stdout: "BEGIN JIB JSON\n{}", + shouldErr: false, + expected: &SyncMap{}, + }, + { + description: "bad marker", + stdout: "BEGIN JIB JSON: BAD/1\n{}", + shouldErr: true, + expected: nil, + }, + { + description: "direct only", + stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + + fmt.Sprintf("{\"direct\":[{\"src\":\"%s\",\"dest\":\"%s\"}]}", dep1, dep1Target), + shouldErr: false, + expected: &SyncMap{ + dep1: SyncEntry{ + []string{dep1Target}, + dep1Time, + true, + }, + }, + }, + { + description: "generated only", + stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + + fmt.Sprintf("{\"generated\":[{\"src\":\"%s\",\"dest\":\"%s\"}]}", dep1, dep1Target), + shouldErr: false, + expected: &SyncMap{ + dep1: SyncEntry{ + []string{dep1Target}, + dep1Time, + false, + }, + }, + }, + { + description: "generated and direct", + stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + + fmt.Sprintf("{\"direct\":[{\"src\":\"%s\",\"dest\":\"%s\"}],\"generated\":[{\"src\":\"%s\",\"dest\":\"%s\"}]}", dep1, dep1Target, dep2, dep2Target), + shouldErr: false, + expected: &SyncMap{ + dep1: SyncEntry{ + []string{dep1Target}, + dep1Time, + true, + }, + dep2: SyncEntry{ + Dest: []string{dep2Target}, + FileTime: dep2Time, + IsDirect: false, + }, + }, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + t.Override(&util.DefaultExecCommand, testutil.CmdRunOut( + "ignored", + test.stdout, + )) + + results, err := getSyncMapFromSystem(&exec.Cmd{Args: []string{"ignored"}}) + + t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, results) + }) + } +} + +func getFileTime(file string, t *testing.T) time.Time { + info, err := os.Stat(file) + if err != nil { + t.Fatalf("Failed to stat %s", file) + return time.Time{} + } + return info.ModTime() +} From a90057fccc47bbeacadb6a434eaddf36e7672fcd Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Mon, 13 Jan 2020 13:00:14 -0500 Subject: [PATCH 2/5] Use 2020 for copyright --- pkg/skaffold/build/jib/sync.go | 2 +- pkg/skaffold/build/jib/sync_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/build/jib/sync.go b/pkg/skaffold/build/jib/sync.go index ca1fa247e0b..dba8978f664 100644 --- a/pkg/skaffold/build/jib/sync.go +++ b/pkg/skaffold/build/jib/sync.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Skaffold Authors +Copyright 2020 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. diff --git a/pkg/skaffold/build/jib/sync_test.go b/pkg/skaffold/build/jib/sync_test.go index 230df4809bb..e21cb1441d6 100644 --- a/pkg/skaffold/build/jib/sync_test.go +++ b/pkg/skaffold/build/jib/sync_test.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Skaffold Authors +Copyright 2020 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. From 136e3b9804c148a8a80bdedd675cae7f5d120937 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Wed, 15 Jan 2020 17:41:42 -0500 Subject: [PATCH 3/5] cleanup --- pkg/skaffold/build/jib/sync.go | 33 ++++++++++++++--------------- pkg/skaffold/build/jib/sync_test.go | 6 +++--- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/pkg/skaffold/build/jib/sync.go b/pkg/skaffold/build/jib/sync.go index dba8978f664..b5f9411a6eb 100644 --- a/pkg/skaffold/build/jib/sync.go +++ b/pkg/skaffold/build/jib/sync.go @@ -68,27 +68,26 @@ func getSyncMapFromSystem(cmd *exec.Cmd) (*SyncMap, error) { } sm := make(SyncMap) - for _, de := range jsm.Direct { - info, err := os.Stat(de.Src) - if err != nil { - return nil, errors.Wrap(err, "could not obtain file mod time") - } - sm[de.Src] = SyncEntry{ - Dest: []string{de.Dest}, - FileTime: info.ModTime(), - IsDirect: true, - } + if err := sm.addEntries(jsm.Direct, true); err != nil { + return nil, errors.WithStack(err) } - for _, ge := range jsm.Generated { - info, err := os.Stat(ge.Src) + if err := sm.addEntries(jsm.Generated, false); err != nil { + return nil, errors.WithStack(err) + } + return &sm, nil +} + +func (sm SyncMap) addEntries(entries []JSONSyncEntry, direct bool) error { + for _, entry := range entries { + info, err := os.Stat(entry.Src) if err != nil { - return nil, errors.Wrap(err, "could not obtain file mod time") + return errors.Wrapf(err, "could not obtain file mod time for %s", entry.Src) } - sm[ge.Src] = SyncEntry{ - Dest: []string{ge.Dest}, + sm[entry.Src] = SyncEntry{ + Dest: []string{entry.Dest}, FileTime: info.ModTime(), - IsDirect: false, + IsDirect: direct, } } - return &sm, nil + return nil } diff --git a/pkg/skaffold/build/jib/sync_test.go b/pkg/skaffold/build/jib/sync_test.go index e21cb1441d6..31efedccf09 100644 --- a/pkg/skaffold/build/jib/sync_test.go +++ b/pkg/skaffold/build/jib/sync_test.go @@ -68,7 +68,7 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "direct only", stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + - fmt.Sprintf("{\"direct\":[{\"src\":\"%s\",\"dest\":\"%s\"}]}", dep1, dep1Target), + fmt.Sprintf(`{"direct":[{"src":"%s","dest":"%s"}]}`, dep1, dep1Target), shouldErr: false, expected: &SyncMap{ dep1: SyncEntry{ @@ -81,7 +81,7 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "generated only", stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + - fmt.Sprintf("{\"generated\":[{\"src\":\"%s\",\"dest\":\"%s\"}]}", dep1, dep1Target), + fmt.Sprintf(`{"generated":[{"src":"%s","dest":"%s"}]}`, dep1, dep1Target), shouldErr: false, expected: &SyncMap{ dep1: SyncEntry{ @@ -94,7 +94,7 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "generated and direct", stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + - fmt.Sprintf("{\"direct\":[{\"src\":\"%s\",\"dest\":\"%s\"}],\"generated\":[{\"src\":\"%s\",\"dest\":\"%s\"}]}", dep1, dep1Target, dep2, dep2Target), + fmt.Sprintf(`{"direct":[{"src":"%s","dest":"%s"}],"generated":[{"src":"%s","dest":"%s"}]}"`, dep1, dep1Target, dep2, dep2Target), shouldErr: false, expected: &SyncMap{ dep1: SyncEntry{ From b599e1c3e3e063de95c939e77c3e06f150c9b153 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Fri, 17 Jan 2020 12:54:18 -0500 Subject: [PATCH 4/5] remove-unnecessary-backslashing --- pkg/skaffold/build/jib/sync.go | 4 +--- pkg/skaffold/build/jib/sync_test.go | 13 ++++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/skaffold/build/jib/sync.go b/pkg/skaffold/build/jib/sync.go index b5f9411a6eb..d0f7887ff92 100644 --- a/pkg/skaffold/build/jib/sync.go +++ b/pkg/skaffold/build/jib/sync.go @@ -17,7 +17,6 @@ limitations under the License. package jib import ( - "bytes" "encoding/json" "os" "os/exec" @@ -62,8 +61,7 @@ func getSyncMapFromSystem(cmd *exec.Cmd) (*SyncMap, error) { return nil, errors.New("failed to get Jib Sync data") } - line := bytes.Replace(matches[1], []byte(`\`), []byte(`\\`), -1) - if err := json.Unmarshal(line, &jsm); err != nil { + if err := json.Unmarshal(matches[1], &jsm); err != nil { return nil, errors.WithStack(err) } diff --git a/pkg/skaffold/build/jib/sync_test.go b/pkg/skaffold/build/jib/sync_test.go index 31efedccf09..5ec22f920b8 100644 --- a/pkg/skaffold/build/jib/sync_test.go +++ b/pkg/skaffold/build/jib/sync_test.go @@ -68,7 +68,7 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "direct only", stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + - fmt.Sprintf(`{"direct":[{"src":"%s","dest":"%s"}]}`, dep1, dep1Target), + fmt.Sprintf(`{"direct":[{"src":"%s","dest":"%s"}]}`, escapeBackslashes(dep1), dep1Target), shouldErr: false, expected: &SyncMap{ dep1: SyncEntry{ @@ -81,7 +81,7 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "generated only", stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + - fmt.Sprintf(`{"generated":[{"src":"%s","dest":"%s"}]}`, dep1, dep1Target), + fmt.Sprintf(`{"generated":[{"src":"%s","dest":"%s"}]}`, escapeBackslashes(dep1), dep1Target), shouldErr: false, expected: &SyncMap{ dep1: SyncEntry{ @@ -94,7 +94,7 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "generated and direct", stdout: "BEGIN JIB JSON: SYNCMAP/1\n" + - fmt.Sprintf(`{"direct":[{"src":"%s","dest":"%s"}],"generated":[{"src":"%s","dest":"%s"}]}"`, dep1, dep1Target, dep2, dep2Target), + fmt.Sprintf(`{"direct":[{"src":"%s","dest":"%s"}],"generated":[{"src":"%s","dest":"%s"}]}"`, escapeBackslashes(dep1), dep1Target, escapeBackslashes(dep2), dep2Target), shouldErr: false, expected: &SyncMap{ dep1: SyncEntry{ @@ -132,3 +132,10 @@ func getFileTime(file string, t *testing.T) time.Time { } return info.ModTime() } + +// for paths that contain "\", they must be escaped in json strings +func escapeBackslashes(path string) string { + // currently don't do anything so I can verify windows tests fail + return path + //return strings.Replace(path, `\`, `\\`, -1) +} From 9667f3ae3636f617be7dc33bee18c654456f9954 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Fri, 17 Jan 2020 12:59:52 -0500 Subject: [PATCH 5/5] fix windows paths in tests --- pkg/skaffold/build/jib/sync_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/skaffold/build/jib/sync_test.go b/pkg/skaffold/build/jib/sync_test.go index 5ec22f920b8..fc0eaa54538 100644 --- a/pkg/skaffold/build/jib/sync_test.go +++ b/pkg/skaffold/build/jib/sync_test.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "os/exec" + "strings" "testing" "time" @@ -135,7 +136,5 @@ func getFileTime(file string, t *testing.T) time.Time { // for paths that contain "\", they must be escaped in json strings func escapeBackslashes(path string) string { - // currently don't do anything so I can verify windows tests fail - return path - //return strings.Replace(path, `\`, `\\`, -1) + return strings.Replace(path, `\`, `\\`, -1) }