Skip to content

Allow configuring jib plugin type #2964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Nov 6, 2019
8 changes: 7 additions & 1 deletion docs/content/en/schemas/v1beta14.json
Original file line number Diff line number Diff line change
Expand Up @@ -1219,11 +1219,17 @@
"type": "string",
"description": "selects which sub-project to build for multi-module builds.",
"x-intellij-html-description": "selects which sub-project to build for multi-module builds."
},
"type": {
"type": "string",
"description": "the Jib builder type (internal: see jib.PluginType)",
"x-intellij-html-description": "the Jib builder type (internal: see jib.PluginType)"
}
},
"preferredOrder": [
"project",
"args"
"args",
"type"
],
"additionalProperties": false,
"description": "*alpha* builds images using the [Jib plugins for Maven and Gradle](https://github.com/GoogleContainerTools/jib/).",
Expand Down
8 changes: 7 additions & 1 deletion docs/content/en/schemas/v1beta15.json
Original file line number Diff line number Diff line change
Expand Up @@ -1229,11 +1229,17 @@
"type": "string",
"description": "selects which sub-project to build for multi-module builds.",
"x-intellij-html-description": "selects which sub-project to build for multi-module builds."
},
"type": {
"type": "string",
"description": "the Jib builder type; normally determined automatically. Valid types are: `maven`: for Maven. `gradle`: for Gradle.",
"x-intellij-html-description": "the Jib builder type; normally determined automatically. Valid types are: <code>maven</code>: for Maven. <code>gradle</code>: for Gradle."
}
},
"preferredOrder": [
"project",
"args"
"args",
"type"
],
"additionalProperties": false,
"description": "*alpha* builds images using the [Jib plugins for Maven and Gradle](https://github.com/GoogleContainerTools/jib/).",
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/gcb/jib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestJibMavenBuildSpec(t *testing.T) {
testutil.Run(t, test.description, func(t *testutil.T) {
artifact := &latest.Artifact{
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{Type: int(jib.JibMaven)},
JibArtifact: &latest.JibArtifact{Type: string(jib.JibMaven)},
},
}

Expand Down Expand Up @@ -91,7 +91,7 @@ func TestJibGradleBuildSpec(t *testing.T) {
testutil.Run(t, test.description, func(t *testutil.T) {
artifact := &latest.Artifact{
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{Type: int(jib.JibGradle)},
JibArtifact: &latest.JibArtifact{Type: string(jib.JibGradle)},
},
}

Expand Down
12 changes: 5 additions & 7 deletions pkg/skaffold/jib/jib.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"time"

Expand All @@ -43,12 +42,11 @@ const (
)

// PluginType defines the different supported Jib plugins.
type PluginType int
type PluginType string

const (
// use `iota+1` so that 0 is an invalid value
JibMaven PluginType = iota + 1
JibGradle
JibMaven PluginType = "maven"
JibGradle PluginType = "gradle"
)

// IsKnown checks that the num value is a known value (vs 0 or an unknown value).
Expand All @@ -68,7 +66,7 @@ func PluginName(t PluginType) string {
case JibGradle:
return "Jib Gradle Plugin"
}
panic("Unknown Jib Plugin Type: " + strconv.Itoa(int(t)))
panic("Unknown Jib Plugin Type: " + string(t))
}

// filesLists contains cached build/input dependencies
Expand Down Expand Up @@ -125,7 +123,7 @@ func DeterminePluginType(workspace string, artifact *latest.JibArtifact) (Plugin
if util.IsFile(filepath.Join(workspace, "pom.xml")) || util.IsDir(filepath.Join(workspace, ".mvn")) {
return JibMaven, nil
}
return -1, errors.Errorf("Unable to determine Jib plugin type for %s", workspace)
return "", errors.Errorf("Unable to determine Jib plugin type for %s", workspace)
}

// getDependencies returns a list of files to watch for changes to rebuild
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/jib/jib_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ func (j Jib) CreateArtifact(manifestImage string) *latest.Artifact {
}

if j.BuilderName == PluginName(JibMaven) {
jibMaven := &latest.JibArtifact{Type: int(JibMaven)}
jibMaven := &latest.JibArtifact{Type: string(JibMaven)}
if j.Project != "" {
jibMaven.Project = j.Project
}
jibMaven.Flags = []string{"-Dimage=" + manifestImage}
a.ArtifactType = latest.ArtifactType{JibArtifact: jibMaven}

} else if j.BuilderName == PluginName(JibGradle) {
jibGradle := &latest.JibArtifact{Type: int(JibGradle)}
jibGradle := &latest.JibArtifact{Type: string(JibGradle)}
if j.Project != "" {
jibGradle.Project = j.Project
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/skaffold/jib/jib_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func TestCreateArtifact(t *testing.T) {
ImageName: "different-image",
Workspace: filepath.Join("path", "to"),
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{Project: "project", Flags: []string{"-Dimage=different-image"}, Type: int(JibGradle)},
JibArtifact: &latest.JibArtifact{Project: "project", Flags: []string{"-Dimage=different-image"}, Type: string(JibGradle)},
},
},
},
Expand All @@ -184,7 +184,7 @@ func TestCreateArtifact(t *testing.T) {
ImageName: "different-image",
Workspace: filepath.Join("path", "to"),
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{Flags: []string{"-Dimage=different-image"}, Type: int(JibGradle)},
JibArtifact: &latest.JibArtifact{Flags: []string{"-Dimage=different-image"}, Type: string(JibGradle)},
},
},
},
Expand All @@ -195,7 +195,7 @@ func TestCreateArtifact(t *testing.T) {
expectedArtifact: latest.Artifact{
ImageName: "different-image",
Workspace: filepath.Join("path", "to"),
ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{Project: "project", Flags: []string{"-Dimage=different-image"}, Type: int(JibMaven)}},
ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{Project: "project", Flags: []string{"-Dimage=different-image"}, Type: string(JibMaven)}},
},
},
{
Expand All @@ -206,7 +206,7 @@ func TestCreateArtifact(t *testing.T) {
ImageName: "different-image",
Workspace: filepath.Join("path", "to"),
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{Flags: []string{"-Dimage=different-image"}, Type: int(JibMaven)},
JibArtifact: &latest.JibArtifact{Flags: []string{"-Dimage=different-image"}, Type: string(JibMaven)},
},
},
},
Expand All @@ -216,7 +216,7 @@ func TestCreateArtifact(t *testing.T) {
manifestImage: "different-image",
expectedArtifact: latest.Artifact{
ImageName: "different-image",
ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{Flags: []string{"-Dimage=different-image"}, Type: int(JibGradle)}},
ArtifactType: latest.ArtifactType{JibArtifact: &latest.JibArtifact{Flags: []string{"-Dimage=different-image"}, Type: string(JibGradle)}},
},
},
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/jib/jib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func TestDeterminePluginType(t *testing.T) {
shouldErr bool
PluginType PluginType
}{
{"empty", []string{}, nil, true, PluginType(-1)},
{"empty", []string{}, nil, true, PluginType("")},
{"gradle-2", []string{"gradle.properties"}, nil, false, JibGradle},
{"gradle-3", []string{"gradlew"}, nil, false, JibGradle},
{"gradle-4", []string{"gradlew.bat"}, nil, false, JibGradle},
Expand All @@ -181,8 +181,8 @@ func TestDeterminePluginType(t *testing.T) {
{"maven-1", []string{"pom.xml"}, nil, false, JibMaven},
{"maven-2", []string{".mvn/maven.config"}, nil, false, JibMaven},
{"maven-3", []string{".mvn/extensions.xml"}, nil, false, JibMaven},
{"gradle override", []string{"pom.xml"}, &latest.JibArtifact{Type: int(JibGradle)}, false, JibGradle},
{"maven override", []string{"build.gradle"}, &latest.JibArtifact{Type: int(JibMaven)}, false, JibMaven},
{"gradle override", []string{"pom.xml"}, &latest.JibArtifact{Type: string(JibGradle)}, false, JibGradle},
{"maven override", []string{"build.gradle"}, &latest.JibArtifact{Type: string(JibMaven)}, false, JibMaven},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
Expand Down
6 changes: 4 additions & 2 deletions pkg/skaffold/schema/latest/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,8 @@ type JibArtifact struct {
// For example: `["--no-build-cache"]`.
Flags []string `yaml:"args,omitempty"`

// Type the Jib builder type (internal: see jib.PluginType)
Type int `yaml:"-"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine as it actually helps making the upgrade process to not break when the detection mechanism doesn't work. It is a conservative change. I will use admin rights to merge this PR.

// Type the Jib builder type; normally determined automatically. Valid types are:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Type the Jib builder type; normally determined automatically. Valid types are:
// Type the Jib builder type; if not set then it is detected automatically. Valid types are:

// `maven`: for Maven.
// `gradle`: for Gradle.
Type string `yaml:"type,omitempty"`
}
2 changes: 2 additions & 0 deletions pkg/skaffold/schema/v1beta13/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ func upgradeOnePipeline(oldPipeline, newPipeline interface{}) error {
newBuild.Artifacts[i].JibArtifact = &next.JibArtifact{
Project: a.JibMavenArtifact.Module,
Flags: flags,
Type: "maven",
}
case a.JibGradleArtifact != nil:
newBuild.Artifacts[i].JibArtifact = &next.JibArtifact{
Project: a.JibGradleArtifact.Project,
Flags: a.JibGradleArtifact.Flags,
Type: "gradle",
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/skaffold/schema/v1beta13/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,11 @@ build:
jib:
args: ['-v', '--activate-profiles', 'prof']
project: dir
type: maven
- image: gcr.io/k8s-skaffold/jib-gradle
jib:
args: ['-v']
type: gradle
googleCloudBuild:
projectId: test-project
test:
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/schema/v1beta14/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -771,5 +771,5 @@ type JibArtifact struct {
Flags []string `yaml:"args,omitempty"`

// Type the Jib builder type (internal: see jib.PluginType)
Type int `yaml:"-"`
Type string `yaml:"type,omitempty"`
}
16 changes: 16 additions & 0 deletions pkg/skaffold/schema/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func Process(config *latest.SkaffoldConfig) error {
errs = append(errs, validateCustomDependencies(config.Build.Artifacts)...)
errs = append(errs, validateSyncRules(config.Build.Artifacts)...)
errs = append(errs, validatePortForwardResources(config.PortForward)...)
errs = append(errs, validateJibPluginTypes(config.Build.Artifacts)...)

if len(errs) == 0 {
return nil
Expand Down Expand Up @@ -187,3 +188,18 @@ func validatePortForwardResources(pfrs []*latest.PortForwardResource) []error {
}
return errs
}

// validateJibPluginTypes makes sure that jib type is one of `maven`, or `gradle` if set.
func validateJibPluginTypes(artifacts []*latest.Artifact) (errs []error) {
for _, a := range artifacts {
if a.JibArtifact == nil || a.JibArtifact.Type == "" {
continue
}
t := strings.ToLower(a.JibArtifact.Type)
if t == "maven" || t == "gradle" {
continue
}
errs = append(errs, fmt.Errorf("artifact %s has invalid plugin type '%s'", a.ImageName, t))
}
return
}
103 changes: 103 additions & 0 deletions pkg/skaffold/schema/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -613,3 +613,106 @@ func TestValidateImageNames(t *testing.T) {
})
}
}

func TestValidateJibPluginType(t *testing.T) {
tests := []struct {
description string
artifacts []*latest.Artifact
shouldErr bool
}{
{
description: "no type",
artifacts: []*latest.Artifact{
{
ImageName: "image/jib",
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{},
},
},
},
},
{
description: "maven",
artifacts: []*latest.Artifact{
{
ImageName: "image/jib",
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{
Type: "maven",
},
},
},
},
},
{
description: "gradle",
artifacts: []*latest.Artifact{
{
ImageName: "image/jib",
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{
Type: "gradle",
},
},
},
},
},
{
description: "empty",
artifacts: []*latest.Artifact{
{
ImageName: "image/jib",
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{
Type: "",
},
},
},
},
},
{
description: "cAsE inSenSiTiVe",
artifacts: []*latest.Artifact{
{
ImageName: "image/jib",
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{
Type: "gRaDlE",
},
},
},
},
},
{
description: "invalid type",
shouldErr: true,
artifacts: []*latest.Artifact{
{
ImageName: "image/jib",
ArtifactType: latest.ArtifactType{
JibArtifact: &latest.JibArtifact{
Type: "invalid",
},
},
},
},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
// disable yamltags validation
t.Override(&validateYamltags, func(interface{}) error { return nil })

err := Process(
&latest.SkaffoldConfig{
Pipeline: latest.Pipeline{
Build: latest.BuildConfig{
Artifacts: test.artifacts,
},
},
})

t.CheckError(test.shouldErr, err)
})
}
}