Skip to content

Commit

Permalink
Merge pull request #3906 from balopat/disabling_jib_gradle_init_by_de…
Browse files Browse the repository at this point in the history 
…fault

disable jib gradle in skaffold init by default
  • Loading branch information
@balopat
balopat committed Apr 2, 2020
2 parents e25d6d7 + ebca3b0 commit 3ac3212
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 6 deletions.
4 changes: 4 additions & 0 deletions cmd/skaffold/app/cmd/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ var (
force bool
analyze bool
enableJibInit bool
enableJibGradleInit bool
enableBuildpacksInit bool
enableNewInitFormat bool
enableManifestGeneration bool
Expand Down Expand Up @@ -65,6 +66,8 @@ func NewCmdInit() *cobra.Command {
f.MarkHidden("XXenableNewInitFormat")
f.BoolVar(&enableJibInit, "XXenableJibInit", false, "")
f.MarkHidden("XXenableJibInit")
f.BoolVar(&enableJibGradleInit, "XXenableJibGradleInit", false, "")
f.MarkHidden("XXenableJibGradleInit")
f.BoolVar(&enableBuildpacksInit, "XXenableBuildpacksInit", false, "")
f.MarkHidden("XXenableBuildpacksInit")
f.StringVar(&buildpacksBuilder, "XXdefaultBuildpacksBuilder", "heroku/buildpacks", "")
Expand All @@ -85,6 +88,7 @@ func doInit(ctx context.Context, out io.Writer) error {
Force: force,
Analyze: analyze,
EnableJibInit: enableJibInit,
EnableJibGradleInit: enableJibGradleInit,
EnableBuildpacksInit: enableBuildpacksInit,
EnableNewInitFormat: enableNewInitFormat || enableBuildpacksInit || enableJibInit,
EnableManifestGeneration: enableManifestGeneration,
Expand Down
3 changes: 3 additions & 0 deletions cmd/skaffold/app/cmd/init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ func TestFlagsToConfigVersion(t *testing.T) {
Force: false,
Analyze: false,
EnableJibInit: false,
EnableJibGradleInit: false,
EnableBuildpacksInit: false,
EnableNewInitFormat: false,
BuildpacksBuilder: "heroku/buildpacks",
Expand All @@ -72,6 +73,7 @@ func TestFlagsToConfigVersion(t *testing.T) {
"--force",
"--analyze",
"--XXenableJibInit",
"--XXenableJibGradleInit",
"--XXenableBuildpacksInit",
"--XXenableNewInitFormat",
"--XXdefaultBuildpacksBuilder", "buildpacks/builder",
Expand All @@ -85,6 +87,7 @@ func TestFlagsToConfigVersion(t *testing.T) {
Force: true,
Analyze: true,
EnableJibInit: true,
EnableJibGradleInit: true,
EnableBuildpacksInit: true,
EnableNewInitFormat: true,
BuildpacksBuilder: "buildpacks/builder",
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/jib/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type jibJSON struct {
}

// validate checks if a file is a valid Jib configuration. Returns the list of Config objects corresponding to each Jib project built by the file, or nil if Jib is not configured.
func validate(path string) []ArtifactConfig {
func validate(path string, enableGradleAnalysis bool) []ArtifactConfig {
// Determine whether maven or gradle
var builderType PluginType
var executable, wrapper, taskName, searchString, consoleFlag string
Expand All @@ -96,7 +96,7 @@ func validate(path string) []ArtifactConfig {
searchString = "<artifactId>jib-maven-plugin</artifactId>"
taskName = "jib:_skaffold-init"
consoleFlag = "--batch-mode"
case strings.HasSuffix(path, "build.gradle"), strings.HasSuffix(path, "build.gradle.kts"):
case enableGradleAnalysis && (strings.HasSuffix(path, "build.gradle") || strings.HasSuffix(path, "build.gradle.kts")):
builderType = JibGradle
executable = "gradle"
wrapper = "gradlew"
Expand Down
19 changes: 18 additions & 1 deletion pkg/skaffold/build/jib/init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestValidate(t *testing.T) {
var tests = []struct {
description string
path string
enableGradle bool
fileContents string
command string
stdout string
Expand All @@ -37,17 +38,20 @@ func TestValidate(t *testing.T) {
{
description: "not a jib file",
path: "path/to/something.txt",
enableGradle: true,
expectedConfig: nil,
},
{
description: "jib string not found",
path: "path/to/build.gradle",
enableGradle: true,
fileContents: "not a useful string",
expectedConfig: nil,
},
{
description: "jib string found but not configured",
path: "path/to/build.gradle",
enableGradle: true,
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: "error",
Expand All @@ -56,6 +60,7 @@ func TestValidate(t *testing.T) {
{
description: "jib gradle single project",
path: "path/to/build.gradle",
enableGradle: true,
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: `BEGIN JIB JSON
Expand All @@ -68,6 +73,7 @@ func TestValidate(t *testing.T) {
{
description: "jib gradle-kotlin single project",
path: "path/to/build.gradle.kts",
enableGradle: true,
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: `BEGIN JIB JSON
Expand All @@ -79,6 +85,7 @@ func TestValidate(t *testing.T) {
},
{
description: "jib gradle multi-project",
enableGradle: true,
path: "path/to/build.gradle",
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q --console=plain",
Expand All @@ -93,9 +100,19 @@ BEGIN JIB JSON
{BuilderName: PluginName(JibGradle), File: "path/to/build.gradle", Project: "project2"},
},
},
{
description: "jib gradle disabled",
path: "path/to/build.gradle",
enableGradle: false,
fileContents: "com.google.cloud.tools.jib",
command: "",
stdout: ``,
expectedConfig: nil,
},
{
description: "jib maven single module",
path: "path/to/pom.xml",
enableGradle: true,
fileContents: "<artifactId>jib-maven-plugin</artifactId>",
command: "mvn jib:_skaffold-init -q --batch-mode",
stdout: `BEGIN JIB JSON
Expand Down Expand Up @@ -133,7 +150,7 @@ BEGIN JIB JSON
test.stdout,
))

validated := Validate(tmpDir.Path(test.path))
validated := Validate(tmpDir.Path(test.path), test.enableGradle)

t.CheckDeepEqual(test.expectedConfig, validated)
})
Expand Down
1 change: 1 addition & 0 deletions pkg/skaffold/initializer/analyze/analyze.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func NewAnalyzer(c config.Config) *ProjectAnalysis {
builderAnalyzer: &builderAnalyzer{
findBuilders: !c.SkipBuild,
enableJibInit: c.EnableJibInit,
enableJibGradleInit: c.EnableJibGradleInit,
enableBuildpacksInit: c.EnableBuildpacksInit,
buildpacksBuilder: c.BuildpacksBuilder,
},
Expand Down
25 changes: 23 additions & 2 deletions pkg/skaffold/initializer/analyze/analyze_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func TestAnalyze(t *testing.T) {
Force: false,
EnableBuildpacksInit: true,
EnableJibInit: true,
EnableJibGradleInit: true,
},
expectedConfigs: []string{
"k8pod.yml",
Expand Down Expand Up @@ -153,6 +154,7 @@ func TestAnalyze(t *testing.T) {
Force: false,
EnableBuildpacksInit: false,
EnableJibInit: true,
EnableJibGradleInit: true,
},
expectedConfigs: []string{
"k8pod.yml",
Expand Down Expand Up @@ -180,6 +182,7 @@ func TestAnalyze(t *testing.T) {
Force: false,
EnableBuildpacksInit: false,
EnableJibInit: true,
EnableJibGradleInit: true,
},
expectedConfigs: []string{
"k8pod.yml",
Expand All @@ -192,6 +195,24 @@ func TestAnalyze(t *testing.T) {
},
shouldErr: false,
},
{
description: "should skip jib gradle",
filesWithContents: map[string]string{
"build.gradle": emptyFile,
"pom.xml": emptyFile,
},
config: initconfig.Config{
Force: false,
EnableBuildpacksInit: false,
EnableJibInit: true,
EnableJibGradleInit: false,
},
expectedConfigs: nil,
expectedPaths: []string{
"pom.xml",
},
shouldErr: false,
},
{
description: "should skip hidden dir",
filesWithContents: map[string]string{
Expand Down Expand Up @@ -348,8 +369,8 @@ func fakeValidateDockerfile(path string) bool {
return strings.Contains(strings.ToLower(path), "dockerfile")
}

func fakeValidateJibConfig(path string) []jib.ArtifactConfig {
if strings.HasSuffix(path, "build.gradle") {
func fakeValidateJibConfig(path string, enableGradle bool) []jib.ArtifactConfig {
if strings.HasSuffix(path, "build.gradle") && enableGradle {
return []jib.ArtifactConfig{{BuilderName: jib.PluginName(jib.JibGradle), File: path}}
}
if strings.HasSuffix(path, "pom.xml") {
Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/initializer/analyze/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
type builderAnalyzer struct {
directoryAnalyzer
enableJibInit bool
enableJibGradleInit bool
enableBuildpacksInit bool
findBuilders bool
buildpacksBuilder string
Expand Down Expand Up @@ -62,7 +63,7 @@ func (a *builderAnalyzer) detectBuilders(path string, detectJib bool) ([]build.I
// TODO: Remove backwards compatibility if statement (not entire block)
if a.enableJibInit && detectJib {
// Check for jib
if builders := jib.Validate(path); builders != nil {
if builders := jib.Validate(path, a.enableJibGradleInit); builders != nil {
results := make([]build.InitBuilder, len(builders))
for i := range builders {
results[i] = builders[i]
Expand Down
1 change: 1 addition & 0 deletions pkg/skaffold/initializer/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Config struct {
Force bool
Analyze bool
EnableJibInit bool // TODO: Remove this parameter
EnableJibGradleInit bool
EnableBuildpacksInit bool
EnableNewInitFormat bool
EnableManifestGeneration bool
Expand Down

0 comments on commit 3ac3212

Please sign in to comment.