Validate pipeline config

cmd/skaffold/app/cmd/runner.go

@@ -24,6 +24,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/validation"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/update"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/pkg/errors"
@@ -43,15 +44,18 @@ func newRunner(opts *config.SkaffoldOptions) (*runner.SkaffoldRunner, *latest.Sk
 
 	config := parsed.(*latest.SkaffoldPipeline)
 
-	err = schema.ApplyProfiles(config, opts)
-	if err != nil {
+	if err = schema.ApplyProfiles(config, opts); err != nil {
 		return nil, nil, errors.Wrap(err, "applying profiles")
 	}
 
 	if err := defaults.Set(config); err != nil {
 		return nil, nil, errors.Wrap(err, "setting default values")
 	}
 
+	if err := validation.ValidateSchema(config); err != nil {
+		return nil, nil, errors.Wrap(err, "invalid skaffold config")
+	}
+
 	defaultRepo, err := configutil.GetDefaultRepo(opts.DefaultRepo)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "getting default repo")

pkg/skaffold/schema/profiles.go

@@ -25,6 +25,7 @@ import (
 	cfg "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
 	yamlpatch "github.com/krishicks/yaml-patch"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -243,7 +244,7 @@ func overlayProfileField(config interface{}, profile interface{}) interface{} {
 	switch v.Kind() {
 	case reflect.Struct:
 		// check the first field of the struct for a oneOf yamltag.
-		if isOneOf(t.Field(0)) {
+		if util.IsOneOfField(t.Field(0)) {
 			return overlayOneOfField(config, profile)
 		}
 		return overlayStructField(config, profile)
@@ -264,14 +265,3 @@ func overlayProfileField(config interface{}, profile interface{}) interface{} {
 		return config
 	}
 }
-
-func isOneOf(field reflect.StructField) bool {
-	for _, tag := range strings.Split(field.Tag.Get("yamltags"), ",") {
-		tagParts := strings.Split(tag, "=")
-
-		if tagParts[0] == "oneOf" {
-			return true
-		}
-	}
-	return false
-}

pkg/skaffold/schema/util/util.go

@@ -18,15 +18,18 @@ package util
 
 import (
 	"encoding/json"
+	"reflect"
+	"strings"
 
-	yaml "gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v2"
 )
 
 type VersionedConfig interface {
 	GetVersion() string
 	Upgrade() (VersionedConfig, error)
 }
 
+// HelmOverrides is a helper struct to aid with json serialization of map[string]interface{}
 type HelmOverrides struct {
 	Values map[string]interface{} `yaml:",inline"`
 }
@@ -50,3 +53,15 @@ func (h *HelmOverrides) UnmarshalJSON(text []byte) error {
 	}
 	return yaml.Unmarshal([]byte(in.Yaml), h)
 }
+
+// IsOneOfField checks if a field is tagged with oneOf
+func IsOneOfField(field reflect.StructField) bool {
+	for _, tag := range strings.Split(field.Tag.Get("yamltags"), ",") {
+		tagParts := strings.Split(tag, "=")
+
+		if tagParts[0] == "oneOf" {
+			return true
+		}
+	}
+	return false
+}

pkg/skaffold/schema/validation/validation.go

@@ -0,0 +1,105 @@
+/*
+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 validation
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
+	"github.com/sirupsen/logrus"
+)
+
+// ValidateSchema checks if the Skaffold pipeline is valid and returns all encountered errors as a concatenated string
+func ValidateSchema(config *latest.SkaffoldPipeline) error {
+	errs := validateOneOf(config)
+	if errs == nil {
+		return nil
+	}
+	var messages []string
+	for _, err := range errs {
+		messages = append(messages, err.Error())
+	}
+	return fmt.Errorf(strings.Join(messages, " | "))
+}
+
+// validateOneOf recursively visits all fields in the config and collects errors for oneOf conflicts
+func validateOneOf(config interface{}) []error {
+	v := reflect.ValueOf(config) // the config itself
+	t := reflect.TypeOf(config)  // the type of the config, used for getting struct field types
+	logrus.Debugf("validating oneOf on %s", t.Name())
+
+	switch v.Kind() {
+	case reflect.Struct:
+		var errs []error
+
+		// fields marked with oneOf should only be set once
+		if t.NumField() > 1 && util.IsOneOfField(t.Field(0)) {
+			var given []string
+			for i := 0; i < t.NumField(); i++ {
+				zero := reflect.Zero(v.Field(i).Type())
+				if util.IsOneOfField(t.Field(i)) && v.Field(i).Interface() != zero.Interface() {
+					given = append(given, yamlName(t.Field(i)))
+				}
+			}
+			if len(given) > 1 {
+				err := fmt.Errorf("only one of %s may be set", given)
+				errs = append(errs, err)
+			}
+		}
+
+		// also check all fields of the current struct
+		for i := 0; i < t.NumField(); i++ {
+			if fieldErrs := validateOneOf(v.Field(i).Interface()); fieldErrs != nil {
+				errs = append(errs, fieldErrs...)
+			}
+		}
+
+		return errs
+
+	case reflect.Slice:
+		// for slices check each element
+		if v.Len() == 0 {
+			return nil
+		}
+		var errs []error
+		for i := 0; i < v.Len(); i++ {
+			if elemErrs := validateOneOf(v.Index(i).Interface()); elemErrs != nil {
+				errs = append(errs, elemErrs...)
+			}
+		}
+		return errs
+
+	case reflect.Ptr:
+		// for pointers check the referenced value
+		if v.IsNil() {
+			return nil
+		}
+		return validateOneOf(v.Elem().Interface())
+
+	default:
+		// other values are fine
+		return nil
+	}
+}
+
+// yamlName retrieves the field name in the yaml
+func yamlName(field reflect.StructField) string {
+	return strings.Split(field.Tag.Get("yaml"), ",")[0]
+}

pkg/skaffold/schema/validation/validation_test.go

@@ -0,0 +1,130 @@
+/*
+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 validation
+
+import (
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+var (
+	cfgWithErrors = &latest.SkaffoldPipeline{
+		Build: latest.BuildConfig{
+			Artifacts: []*latest.Artifact{
+				{
+					ArtifactType: latest.ArtifactType{
+						DockerArtifact: &latest.DockerArtifact{},
+						BazelArtifact:  &latest.BazelArtifact{},
+					},
+				},
+				{
+					ArtifactType: latest.ArtifactType{
+						BazelArtifact:  &latest.BazelArtifact{},
+						KanikoArtifact: &latest.KanikoArtifact{},
+					},
+				},
+			},
+		},
+		Deploy: latest.DeployConfig{
+			DeployType: latest.DeployType{
+				HelmDeploy:    &latest.HelmDeploy{},
+				KubectlDeploy: &latest.KubectlDeploy{},
+			},
+		},
+	}
+)
+
+func TestValidateSchema(t *testing.T) {
+	err := ValidateSchema(cfgWithErrors)
+	testutil.CheckError(t, true, err)
+
+	err = ValidateSchema(&latest.SkaffoldPipeline{})
+	testutil.CheckError(t, false, err)
+}
+
+func TestValidateOneOf(t *testing.T) {
+	tests := []struct {
+		name      string
+		input     interface{}
+		shouldErr bool
+		expected  []string
+	}{
+		{
+			name: "only one field set",
+			input: &latest.BuildType{
+				Cluster: &latest.ClusterDetails{},
+			},
+			shouldErr: false,
+		},
+		{
+			name: "two colliding buildTypes",
+			input: &latest.BuildType{
+				GoogleCloudBuild: &latest.GoogleCloudBuild{},
+				Cluster:          &latest.ClusterDetails{},
+			},
+			shouldErr: true,
+			expected:  []string{"googleCloudBuild cluster"},
+		},
+		{
+			name: "deployType with two fields",
+			input: &latest.DeployType{
+				HelmDeploy:    &latest.HelmDeploy{},
+				KubectlDeploy: &latest.KubectlDeploy{},
+			},
+			shouldErr: true,
+			expected:  []string{"helm kubectl"},
+		},
+		{
+			name: "deployType with three fields",
+			input: &latest.DeployType{
+				HelmDeploy:      &latest.HelmDeploy{},
+				KubectlDeploy:   &latest.KubectlDeploy{},
+				KustomizeDeploy: &latest.KustomizeDeploy{},
+			},
+			shouldErr: true,
+			expected:  []string{"helm kubectl kustomize"},
+		},
+		{
+			name:      "empty struct should not fail",
+			input:     &latest.GitTagger{},
+			shouldErr: false,
+		},
+		{
+			name:      "full Skaffold pipeline",
+			input:     cfgWithErrors,
+			shouldErr: true,
+			expected:  []string{"docker bazel", "bazel kaniko", "helm kubectl"},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual := validateOneOf(test.input)
+
+			if test.shouldErr {
+				testutil.CheckDeepEqual(t, len(test.expected), len(actual))
+				for i, message := range test.expected {
+					testutil.CheckContains(t, message, actual[i].Error())
+				}
+			} else {
+				testutil.CheckDeepEqual(t, []error(nil), actual)
+			}
+		})
+	}
+}