diff --git a/pkg/skaffold/build/jib/gradle.go b/pkg/skaffold/build/jib/gradle.go index 3dab1ec035d..741f5eb0568 100644 --- a/pkg/skaffold/build/jib/gradle.go +++ b/pkg/skaffold/build/jib/gradle.go @@ -38,6 +38,7 @@ var ( // Skaffold-Jib depends on functionality introduced with Jib-Gradle 1.4.0 const MinimumJibGradleVersion = "1.4.0" +const MinimumJibGradleVersionForSync = "2.0.0" // GradleCommand stores Gradle executable and wrapper name var GradleCommand = util.CommandWrapper{Executable: "gradle", Wrapper: "gradlew"} @@ -87,18 +88,18 @@ func getDependenciesGradle(ctx context.Context, workspace string, a *latest.JibA } func getCommandGradle(ctx context.Context, workspace string, a *latest.JibArtifact) exec.Cmd { - args := append(gradleArgsFunc(a, "_jibSkaffoldFilesV2"), "-q") + args := append(gradleArgsFunc(a, "_jibSkaffoldFilesV2", MinimumJibGradleVersion), "-q") return GradleCommand.CreateCommand(ctx, workspace, args) } func getSyncMapCommandGradle(ctx context.Context, workspace string, a *latest.JibArtifact) *exec.Cmd { - cmd := GradleCommand.CreateCommand(ctx, workspace, gradleBuildArgsFunc("_jibSkaffoldSyncMap", a, true)) + cmd := GradleCommand.CreateCommand(ctx, workspace, gradleBuildArgsFunc("_jibSkaffoldSyncMap", a, true, MinimumJibMavenVersionForSync)) return &cmd } // GenerateGradleBuildArgs generates the arguments to Gradle for building the project as an image. func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifact, skipTests bool, insecureRegistries map[string]bool) []string { - args := gradleBuildArgsFunc(task, a, skipTests) + args := gradleBuildArgsFunc(task, a, skipTests, MinimumJibGradleVersion) if insecure, err := isOnInsecureRegistry(imageName, insecureRegistries); err == nil && insecure { // jib doesn't support marking specific registries as insecure args = append(args, "-Djib.allowInsecureRegistries=true") @@ -109,11 +110,11 @@ func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifac } // Do not use directly, use gradleBuildArgsFunc -func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests bool) []string { +func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string { // disable jib's rich progress footer; we could use `--console=plain` // but it also disables colour which can be helpful args := []string{"-Djib.console=plain"} - args = append(args, gradleArgsFunc(a, task)...) + args = append(args, gradleArgsFunc(a, task, minimumVersion)...) if skipTests { args = append(args, "-x", "test") @@ -123,8 +124,8 @@ func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests bool) []strin } // Do not use directly, use gradleArgsFunc -func gradleArgs(a *latest.JibArtifact, task string) []string { - args := []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion} +func gradleArgs(a *latest.JibArtifact, task string, minimumVersion string) []string { + args := []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + minimumVersion} if a.Project == "" { return append(args, ":"+task) } diff --git a/pkg/skaffold/build/jib/gradle_test.go b/pkg/skaffold/build/jib/gradle_test.go index 5ed5663875a..6600fb8f9bd 100644 --- a/pkg/skaffold/build/jib/gradle_test.go +++ b/pkg/skaffold/build/jib/gradle_test.go @@ -71,7 +71,7 @@ func TestBuildJibGradleToDocker(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.NewTempDir().Touch("build.gradle").Chdir() - t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake) + t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion)) t.Override(&util.DefaultExecCommand, test.commands) api := (&testutil.FakeAPIClient{}).Add("img:tag", "imageID") localDocker := docker.NewLocalDaemon(api, nil, false, nil) @@ -130,7 +130,7 @@ func TestBuildJibGradleToRegistry(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.NewTempDir().Touch("build.gradle").Chdir() - t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake) + t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion)) t.Override(&util.DefaultExecCommand, test.commands) t.Override(&docker.RemoteDigest, func(identifier string, _ map[string]bool) (string, error) { if identifier == "img:tag" { @@ -219,7 +219,9 @@ func TestGetDependenciesGradle(t *testing.T) { )) // Change build file mod time - os.Chtimes(build, test.modTime, test.modTime) + if err := os.Chtimes(build, test.modTime, test.modTime); err != nil { + t.Fatal(err) + } deps, err := getDependenciesGradle(ctx, tmpDir.Root(), &latest.JibArtifact{Project: "gradle-test"}) if test.err != nil { @@ -313,7 +315,7 @@ func TestGetSyncMapCommandGradle(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake) + t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersionForSync)) cmd := getSyncMapCommandGradle(ctx, test.workspace, &test.jibArtifact) expectedCmd := test.expectedCmd(test.workspace) t.CheckDeepEqual(expectedCmd.Path, cmd.Path) @@ -340,7 +342,7 @@ func TestGenerateGradleBuildArgs(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake) + t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion)) command := GenerateGradleBuildArgs("testTask", test.image, &test.in, test.skipTests, test.insecureRegistries) t.CheckDeepEqual(test.out, command) }) @@ -356,16 +358,16 @@ func TestGradleArgs(t *testing.T) { { description: "single module", jibArtifact: latest.JibArtifact{}, - expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":testTask"}, + expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=test-version", ":testTask"}, }, { description: "multi module", jibArtifact: latest.JibArtifact{Project: "module"}, - expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":module:testTask"}, + expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=test-version", ":module:testTask"}, }, } for _, test := range tests { - args := gradleArgs(&test.jibArtifact, "testTask") + args := gradleArgs(&test.jibArtifact, "testTask", "test-version") testutil.CheckDeepEqual(t, test.expected, args) } } @@ -416,29 +418,36 @@ func TestGradleBuildArgs(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&gradleArgsFunc, gradleArgsFuncFake) - args := gradleBuildArgs("testTask", &test.jibArtifact, test.skipTests) + t.Override(&gradleArgsFunc, getGradleArgsFuncFake(t, "test-version")) + args := gradleBuildArgs("testTask", &test.jibArtifact, test.skipTests, "test-version") t.CheckDeepEqual(test.expected, args) }) } } -func gradleArgsFuncFake(a *latest.JibArtifact, task string) []string { - if a.Project == "" { - return []string{"fake-gradleArgs-for-" + task} +func getGradleArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(*latest.JibArtifact, string, string) []string { + return func(a *latest.JibArtifact, task string, minimumVersion string) []string { + t.CheckDeepEqual(expectedMinimumVersion, minimumVersion) + if a.Project == "" { + return []string{"fake-gradleArgs-for-" + task} + } + return []string{"fake-gradleArgs-for-" + a.Project + "-for-" + task} } - return []string{"fake-gradleArgs-for-" + a.Project + "-for-" + task} } // check that parameters are actually passed though -func gradleBuildArgsFuncFake(task string, a *latest.JibArtifact, skipTests bool) []string { - testString := "" - if skipTests { - testString = "-skipTests" - } - - if a.Project == "" { - return []string{"fake-gradleBuildArgs-for-" + task + testString} +func getGradleBuildArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(string, *latest.JibArtifact, bool, string) []string { + return func(task string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string { + t.CheckDeepEqual(expectedMinimumVersion, minimumVersion) + + testString := "" + if skipTests { + testString = "-skipTests" + } + + if a.Project == "" { + return []string{"fake-gradleBuildArgs-for-" + task + testString} + } + return []string{"fake-gradleBuildArgs-for-" + a.Project + "-for-" + task + testString} } - return []string{"fake-gradleBuildArgs-for-" + a.Project + "-for-" + task + testString} } diff --git a/pkg/skaffold/build/jib/maven.go b/pkg/skaffold/build/jib/maven.go index 01ffc71f920..f3ec2287f0f 100644 --- a/pkg/skaffold/build/jib/maven.go +++ b/pkg/skaffold/build/jib/maven.go @@ -37,6 +37,7 @@ var ( // Skaffold-Jib depends on functionality introduced with Jib-Maven 1.4.0 const MinimumJibMavenVersion = "1.4.0" +const MinimumJibMavenVersionForSync = "2.0.0" // MavenCommand stores Maven executable and wrapper name var MavenCommand = util.CommandWrapper{Executable: "mvn", Wrapper: "mvnw"} @@ -85,20 +86,20 @@ func getDependenciesMaven(ctx context.Context, workspace string, a *latest.JibAr } func getCommandMaven(ctx context.Context, workspace string, a *latest.JibArtifact) exec.Cmd { - args := mavenArgsFunc(a) + args := mavenArgsFunc(a, MinimumJibMavenVersion) args = append(args, "jib:_skaffold-files-v2", "--quiet") return MavenCommand.CreateCommand(ctx, workspace, args) } func getSyncMapCommandMaven(ctx context.Context, workspace string, a *latest.JibArtifact) *exec.Cmd { - cmd := MavenCommand.CreateCommand(ctx, workspace, mavenBuildArgsFunc("_skaffold-sync-map", a, true)) + cmd := MavenCommand.CreateCommand(ctx, workspace, mavenBuildArgsFunc("_skaffold-sync-map", a, true, MinimumJibMavenVersionForSync)) return &cmd } // GenerateMavenBuildArgs generates the arguments to Maven for building the project as an image. func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact, skipTests bool, insecureRegistries map[string]bool) []string { - args := mavenBuildArgsFunc(goal, a, skipTests) + args := mavenBuildArgsFunc(goal, a, skipTests, MinimumJibMavenVersion) if insecure, err := isOnInsecureRegistry(imageName, insecureRegistries); err == nil && insecure { // jib doesn't support marking specific registries as insecure args = append(args, "-Djib.allowInsecureRegistries=true") @@ -109,11 +110,11 @@ func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact } // Do not use directly, use mavenBuildArgsFunc -func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests bool) []string { +func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string { // disable jib's rich progress footer on builds; we could use --batch-mode // but it also disables colour which can be helpful args := []string{"-Djib.console=plain"} - args = append(args, mavenArgsFunc(a)...) + args = append(args, mavenArgsFunc(a, minimumVersion)...) if skipTests { args = append(args, "-DskipTests=true") @@ -130,8 +131,8 @@ func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests bool) []string } // Do not use directly, use mavenArgsFunc -func mavenArgs(a *latest.JibArtifact) []string { - args := []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion} +func mavenArgs(a *latest.JibArtifact, minimumVersion string) []string { + args := []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + minimumVersion} args = append(args, a.Flags...) if a.Project == "" { diff --git a/pkg/skaffold/build/jib/maven_test.go b/pkg/skaffold/build/jib/maven_test.go index a172306176a..3e8b20ccb1e 100644 --- a/pkg/skaffold/build/jib/maven_test.go +++ b/pkg/skaffold/build/jib/maven_test.go @@ -70,7 +70,7 @@ func TestBuildJibMavenToDocker(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake) + t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion)) t.NewTempDir().Touch("pom.xml").Chdir() t.Override(&util.DefaultExecCommand, test.commands) api := (&testutil.FakeAPIClient{}).Add("img:tag", "imageID") @@ -104,16 +104,12 @@ func TestBuildJibMavenToRegistry(t *testing.T) { { description: "build", artifact: &latest.JibArtifact{}, - commands: testutil.CmdRun( - "mvn fake-mavenBuildArgs-for-build -Dimage=img:tag", - ), + commands: testutil.CmdRun("mvn fake-mavenBuildArgs-for-build -Dimage=img:tag"), }, { description: "build with module", artifact: &latest.JibArtifact{Project: "module"}, - commands: testutil.CmdRun( - "mvn fake-mavenBuildArgs-for-module-for-build -Dimage=img:tag", - ), + commands: testutil.CmdRun("mvn fake-mavenBuildArgs-for-module-for-build -Dimage=img:tag"), }, { description: "fail build", @@ -129,7 +125,7 @@ func TestBuildJibMavenToRegistry(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake) + t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion)) t.NewTempDir().Touch("pom.xml").Chdir() t.Override(&util.DefaultExecCommand, test.commands) t.Override(&docker.RemoteDigest, func(identifier string, _ map[string]bool) (string, error) { @@ -219,7 +215,9 @@ func TestGetDependenciesMaven(t *testing.T) { )) // Change build file mod time - os.Chtimes(build, test.modTime, test.modTime) + if err := os.Chtimes(build, test.modTime, test.modTime); err != nil { + t.Fatal(err) + } deps, err := getDependenciesMaven(ctx, tmpDir.Root(), &latest.JibArtifact{Project: "maven-test"}) if test.err != nil { @@ -266,7 +264,7 @@ func TestGetCommandMaven(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&mavenArgsFunc, mavenArgsFuncFake) + t.Override(&mavenArgsFunc, getMavenArgsFuncFake(t, MinimumJibMavenVersion)) tmpDir := t.NewTempDir(). Touch(test.filesInWorkspace...) @@ -305,7 +303,7 @@ func TestGetSyncMapCommandMaven(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake) + t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersionForSync)) cmd := getSyncMapCommandMaven(ctx, test.workspace, &test.jibArtifact) expectedCmd := test.expectedCmd(test.workspace) t.CheckDeepEqual(expectedCmd.Path, cmd.Path) @@ -332,7 +330,7 @@ func TestGenerateMavenBuildArgs(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake) + t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion)) args := GenerateMavenBuildArgs("test-goal", test.image, &test.a, test.skipTests, test.insecureRegistries) t.CheckDeepEqual(test.out, args) }) @@ -373,8 +371,8 @@ func TestMavenBuildArgs(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&mavenArgsFunc, mavenArgsFuncFake) - args := mavenBuildArgs("test-goal", &test.jibArtifact, test.skipTests) + t.Override(&mavenArgsFunc, getMavenArgsFuncFake(t, "test-version")) + args := mavenBuildArgs("test-goal", &test.jibArtifact, test.skipTests, "test-version") t.CheckDeepEqual(test.expected, args) }) } @@ -389,19 +387,19 @@ func TestMavenArgs(t *testing.T) { { description: "single module", jibArtifact: latest.JibArtifact{}, - expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--non-recursive"}, + expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--non-recursive"}, }, { description: "single module with extra flags", jibArtifact: latest.JibArtifact{ Flags: []string{"--flag1", "--flag2"}, }, - expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--flag1", "--flag2", "--non-recursive"}, + expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--flag1", "--flag2", "--non-recursive"}, }, { description: "multi module", jibArtifact: latest.JibArtifact{Project: "module"}, - expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--projects", "module", "--also-make"}, + expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--projects", "module", "--also-make"}, }, { description: "multi module with extra falgs", @@ -409,31 +407,36 @@ func TestMavenArgs(t *testing.T) { Project: "module", Flags: []string{"--flag1", "--flag2"}, }, - expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--flag1", "--flag2", "--projects", "module", "--also-make"}, + expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--flag1", "--flag2", "--projects", "module", "--also-make"}, }, } for _, test := range tests { - args := mavenArgs(&test.jibArtifact) + args := mavenArgs(&test.jibArtifact, "test-version") testutil.CheckDeepEqual(t, test.expected, args) } } -func mavenArgsFuncFake(a *latest.JibArtifact) []string { - if a.Project == "" { - return []string{"fake-mavenArgs"} +func getMavenArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(*latest.JibArtifact, string) []string { + return func(a *latest.JibArtifact, minimumVersion string) []string { + t.CheckDeepEqual(expectedMinimumVersion, minimumVersion) + if a.Project == "" { + return []string{"fake-mavenArgs"} + } + return []string{"fake-mavenArgs-for-" + a.Project} } - return []string{"fake-mavenArgs-for-" + a.Project} } -// check that parameters are actually passed though -func mavenBuildArgsFuncFake(goal string, a *latest.JibArtifact, skipTests bool) []string { - testString := "" - if skipTests { - testString = "-skipTests" - } - - if a.Project == "" { - return []string{"fake-mavenBuildArgs-for-" + goal + testString} +func getMavenBuildArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(string, *latest.JibArtifact, bool, string) []string { + return func(goal string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string { + t.CheckDeepEqual(expectedMinimumVersion, minimumVersion) + testString := "" + if skipTests { + testString = "-skipTests" + } + + if a.Project == "" { + return []string{"fake-mavenBuildArgs-for-" + goal + testString} + } + return []string{"fake-mavenBuildArgs-for-" + a.Project + "-for-" + goal + testString} } - return []string{"fake-mavenBuildArgs-for-" + a.Project + "-for-" + goal + testString} } diff --git a/pkg/skaffold/build/jib/sync.go b/pkg/skaffold/build/jib/sync.go index 13b0622a00d..ef9e6db1028 100644 --- a/pkg/skaffold/build/jib/sync.go +++ b/pkg/skaffold/build/jib/sync.go @@ -186,10 +186,7 @@ func getSyncMapFromSystem(cmd *exec.Cmd) (*SyncMap, error) { 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) + 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") } diff --git a/pkg/skaffold/build/jib/sync_test.go b/pkg/skaffold/build/jib/sync_test.go index 6f8aee4db76..4dde4659dc6 100644 --- a/pkg/skaffold/build/jib/sync_test.go +++ b/pkg/skaffold/build/jib/sync_test.go @@ -60,8 +60,8 @@ func TestGetSyncMapFromSystem(t *testing.T) { { description: "old style marker", stdout: "BEGIN JIB JSON\n{}", - shouldErr: false, - expected: &SyncMap{}, + shouldErr: true, + expected: nil, }, { description: "bad marker",