diff --git a/pkg/skaffold/config/config_test.go b/pkg/skaffold/config/config_test.go index 97479fef951..88bfcdd7429 100644 --- a/pkg/skaffold/config/config_test.go +++ b/pkg/skaffold/config/config_test.go @@ -41,6 +41,20 @@ build: deploy: kubectl: {} ` + // This config has two tag policies set. + invalidConfig = ` +apiVersion: skaffold/v1alpha2 +kind: Config +build: + tagPolicy: + sha256: {} + gitCommit: {} + artifacts: + - imageName: example +deploy: + name: example +` + completeConfig = ` apiVersion: skaffold/v1alpha2 kind: Config @@ -151,6 +165,11 @@ func TestParseConfig(t *testing.T) { config: badConfig, shouldErr: true, }, + { + description: "two taggers defined", + config: invalidConfig, + shouldErr: true, + }, } for _, test := range tests { diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go index eeaeadf705a..687c5550490 100644 --- a/pkg/skaffold/config/util.go +++ b/pkg/skaffold/config/util.go @@ -22,6 +22,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha1" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/yamltags" ) // Versions is an ordered list of all schema versions. @@ -45,6 +46,9 @@ func GetConfig(contents []byte, useDefault bool) (util.VersionedConfig, error) { err := cfg.Parse(contents, useDefault) if cfg.GetVersion() == version { // Versions are same hence propagate the parse error. + if err := yamltags.ProcessStruct(cfg); err != nil { + return nil, err + } return cfg, err } } diff --git a/pkg/skaffold/schema/util/util.go b/pkg/skaffold/schema/util/util.go index dc009f71d03..68cc5090ffd 100644 --- a/pkg/skaffold/schema/util/util.go +++ b/pkg/skaffold/schema/util/util.go @@ -32,7 +32,9 @@ type Config interface { func IsOneOf(field reflect.StructField) bool { for _, tag := range strings.Split(field.Tag.Get("yamltags"), ",") { - if tag == "oneOf" { + tagParts := strings.Split(tag, "=") + + if tagParts[0] == "oneOf" { return true } } diff --git a/pkg/skaffold/schema/v1alpha2/config.go b/pkg/skaffold/schema/v1alpha2/config.go index dba849ad670..b263a8ceff2 100644 --- a/pkg/skaffold/schema/v1alpha2/config.go +++ b/pkg/skaffold/schema/v1alpha2/config.go @@ -45,10 +45,10 @@ type BuildConfig struct { // TagPolicy contains all the configuration for the tagging step type TagPolicy struct { - GitTagger *GitTagger `yaml:"gitCommit" yamltags:"oneOf"` - ShaTagger *ShaTagger `yaml:"sha256" yamltags:"oneOf"` - EnvTemplateTagger *EnvTemplateTagger `yaml:"envTemplate" yamltags:"oneOf"` - DateTimeTagger *DateTimeTagger `yaml:"dateTime" yamltags:"oneOf"` + GitTagger *GitTagger `yaml:"gitCommit" yamltags:"oneOf=tag"` + ShaTagger *ShaTagger `yaml:"sha256" yamltags:"oneOf=tag"` + EnvTemplateTagger *EnvTemplateTagger `yaml:"envTemplate" yamltags:"oneOf=tag"` + DateTimeTagger *DateTimeTagger `yaml:"dateTime" yamltags:"oneOf=tag"` } // ShaTagger contains the configuration for the SHA tagger. @@ -71,9 +71,9 @@ type DateTimeTagger struct { // BuildType contains the specific implementation and parameters needed // for the build step. Only one field should be populated. type BuildType struct { - LocalBuild *LocalBuild `yaml:"local" yamltags:"oneOf"` - GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild" yamltags:"oneOf"` - KanikoBuild *KanikoBuild `yaml:"kaniko" yamltags:"oneOf"` + LocalBuild *LocalBuild `yaml:"local" yamltags:"oneOf=build"` + GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild" yamltags:"oneOf=build"` + KanikoBuild *KanikoBuild `yaml:"kaniko" yamltags:"oneOf=build"` } // LocalBuild contains the fields needed to do a build on the local docker daemon @@ -112,9 +112,9 @@ type DeployConfig struct { // DeployType contains the specific implementation and parameters needed // for the deploy step. Only one field should be populated. type DeployType struct { - HelmDeploy *HelmDeploy `yaml:"helm" yamltags:"oneOf"` - KubectlDeploy *KubectlDeploy `yaml:"kubectl" yamltags:"oneOf"` - KustomizeDeploy *KustomizeDeploy `yaml:"kustomize" yamltags:"oneOf"` + HelmDeploy *HelmDeploy `yaml:"helm" yamltags:"oneOf=deploy"` + KubectlDeploy *KubectlDeploy `yaml:"kubectl" yamltags:"oneOf=deploy"` + KustomizeDeploy *KustomizeDeploy `yaml:"kustomize" yamltags:"oneOf=deploy"` } // KubectlDeploy contains the configuration needed for deploying with `kubectl apply` @@ -202,8 +202,8 @@ type Profile struct { } type ArtifactType struct { - DockerArtifact *DockerArtifact `yaml:"docker" yamltags:"oneOf"` - BazelArtifact *BazelArtifact `yaml:"bazel" yamltags:"oneOf"` + DockerArtifact *DockerArtifact `yaml:"docker" yamltags:"oneOf=artifact"` + BazelArtifact *BazelArtifact `yaml:"bazel" yamltags:"oneOf=artifact"` } type DockerArtifact struct { diff --git a/pkg/skaffold/yamltags/tags.go b/pkg/skaffold/yamltags/tags.go new file mode 100644 index 00000000000..20822790397 --- /dev/null +++ b/pkg/skaffold/yamltags/tags.go @@ -0,0 +1,189 @@ +/* +Copyright 2018 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 yamltags + +import ( + "errors" + "fmt" + "reflect" + "strconv" + "strings" +) + +// ProcessStruct validates and processes the provided pointer to a struct. +func ProcessStruct(s interface{}) error { + parentStruct := reflect.ValueOf(s).Elem() + t := parentStruct.Type() + + // Loop through the fields on the struct, looking for tags. + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + val := parentStruct.Field(i) + field := parentStruct.Type().Field(i) + if tags, ok := f.Tag.Lookup("yamltags"); ok { + if err := ProcessTags(tags, val, parentStruct, field); err != nil { + return err + } + } + // Recurse down the struct + if val.Kind() == reflect.Struct { + if err := ProcessStruct(val.Addr().Interface()); err != nil { + return err + } + } + } + return nil +} + +func ProcessTags(yamltags string, val reflect.Value, parentStruct reflect.Value, field reflect.StructField) error { + tags := strings.Split(yamltags, ",") + for _, tag := range tags { + tagParts := strings.Split(tag, "=") + var yt YamlTag + switch tagParts[0] { + case "required": + yt = &RequiredTag{} + case "default": + yt = &DefaultTag{} + case "oneOf": + yt = &OneOfTag{ + Field: field, + Parent: parentStruct, + } + } + if err := yt.Load(tagParts); err != nil { + return err + } + if err := yt.Process(val); err != nil { + return err + } + } + return nil +} + +type YamlTag interface { + Load([]string) error + Process(reflect.Value) error +} + +type RequiredTag struct { +} + +func (rt *RequiredTag) Load(s []string) error { + return nil +} + +func (rt *RequiredTag) Process(val reflect.Value) error { + if isZeroValue(val) { + return errors.New("required value not set") + } + return nil +} + +type DefaultTag struct { + dv string +} + +func (dt *DefaultTag) Load(s []string) error { + if len(s) != 2 { + return fmt.Errorf("invalid default tag: %v, expected key=value", s) + } + dt.dv = s[1] + return nil +} + +func (dt *DefaultTag) Process(val reflect.Value) error { + if !isZeroValue(val) { + return nil + } + + switch val.Kind() { + case reflect.Int, reflect.Int16, reflect.Int32, reflect.Int64: + i, err := strconv.ParseInt(dt.dv, 0, 0) + if err != nil { + return err + } + val.SetInt(i) + case reflect.String: + val.SetString(dt.dv) + } + return nil +} + +// A program can have many structs, that each have many oneOfSets +// each oneOfSet is a map of a set name to the list of fields that belong to that set +// only one field in that list can have a non-zero value. + +var allOneOfs map[string]map[string][]string + +func getOneOfSetsForStruct(structName string) map[string][]string { + _, ok := allOneOfs[structName] + if !ok { + allOneOfs[structName] = map[string][]string{} + } + return allOneOfs[structName] +} + +type OneOfTag struct { + Field reflect.StructField + Parent reflect.Value + oneOfSets map[string][]string + setName string +} + +func (oot *OneOfTag) Load(s []string) error { + if len(s) != 2 { + return fmt.Errorf("invalid default struct tag: %v, expected key=value", s) + } + oot.setName = s[1] + + // Fetch the right oneOfSet for the struct. + structName := oot.Parent.Type().Name() + oot.oneOfSets = getOneOfSetsForStruct(structName) + + // Add this field to the oneOfSet + oot.oneOfSets[oot.setName] = append(oot.oneOfSets[oot.setName], oot.Field.Name) + return nil +} + +func (oot *OneOfTag) Process(val reflect.Value) error { + if isZeroValue(val) { + return nil + } + + // This must exist because process is always called after Load. + oneOfSet := oot.oneOfSets[oot.setName] + for _, otherField := range oneOfSet { + if otherField == oot.Field.Name { + continue + } + field := oot.Parent.FieldByName(otherField) + if !isZeroValue(field) { + return fmt.Errorf("only one element in set %s can be set. got %s and %s", oot.setName, otherField, oot.Field.Name) + } + } + return nil +} + +func isZeroValue(val reflect.Value) bool { + zv := reflect.Zero(val.Type()).Interface() + return reflect.DeepEqual(zv, val.Interface()) +} + +func init() { + allOneOfs = make(map[string]map[string][]string) +} diff --git a/pkg/skaffold/yamltags/tags_test.go b/pkg/skaffold/yamltags/tags_test.go new file mode 100644 index 00000000000..058761eda0c --- /dev/null +++ b/pkg/skaffold/yamltags/tags_test.go @@ -0,0 +1,219 @@ +/* +Copyright 2018 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 yamltags + +import ( + "reflect" + "testing" +) + +type otherstruct struct { + A int `yamltags:"required"` +} + +type required struct { + A string `yamltags:"required"` + B int `yamltags:"required"` + C otherstruct +} + +func TestProcessStructRequired(t *testing.T) { + type args struct { + s interface{} + } + + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "missing all", + args: args{ + s: &required{}, + }, + wantErr: true, + }, + { + name: "all set", + args: args{ + s: &required{ + A: "hey", + B: 3, + C: otherstruct{ + A: 1, + }, + }, + }, + wantErr: false, + }, + { + name: "missng some", + args: args{ + s: &required{ + A: "hey", + C: otherstruct{ + A: 1, + }, + }, + }, + wantErr: true, + }, + { + name: "missng nested", + args: args{ + s: &required{ + A: "hey", + B: 3, + }, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := ProcessStruct(tt.args.s); (err != nil) != tt.wantErr { + t.Errorf("ProcessStruct() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +type defaultTags struct { + A string `yamltags:"default=foo"` + B int `yamltags:"default=3"` +} + +func TestProcessStructDefault(t *testing.T) { + type args struct { + s interface{} + } + + tests := []struct { + name string + args args + want interface{} + wantErr bool + }{ + { + name: "missing all", + args: args{ + s: &defaultTags{}, + }, + want: &defaultTags{A: "foo", B: 3}, + wantErr: false, + }, + { + name: "all set", + args: args{ + s: &defaultTags{A: "yo", B: 1}, + }, + want: &defaultTags{A: "yo", B: 1}, + wantErr: false, + }, + { + name: "some set", + args: args{ + s: &defaultTags{A: "yo"}, + }, + want: &defaultTags{A: "yo", B: 3}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := ProcessStruct(tt.args.s); (err != nil) != tt.wantErr { + t.Errorf("ProcessStruct() error = %v, wantErr %v", err, tt.wantErr) + } + if !reflect.DeepEqual(tt.args.s, tt.want) { + t.Errorf("Got %v, want %v", tt.args.s, tt.want) + } + }) + } +} + +type oneOfStruct struct { + A string `yamltags:"oneOf=set1"` + B string `yamltags:"oneOf=set1"` + C int `yamltags:"oneOf=set2"` + D *nested `yamltags:"oneOf=set2"` + E nested `yamltags:"oneOf=set2"` +} + +type nested struct { + F string +} + +func TestOneOf(t *testing.T) { + type args struct { + s interface{} + } + + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "only one", + args: args{ + s: &oneOfStruct{ + A: "foo", + C: 3, + }, + }, + wantErr: false, + }, + { + name: "too many in one set", + args: args{ + s: &oneOfStruct{ + A: "foo", + B: "baz", + C: 3, + }}, + wantErr: true, + }, + { + name: "too many pointers set", + args: args{ + s: &oneOfStruct{ + D: &nested{F: "foo"}, + E: nested{F: "foo"}, + }}, + wantErr: true, + }, + { + name: "too many in both sets", + args: args{ + s: &oneOfStruct{ + A: "foo", + B: "baz", + C: 3, + D: &nested{F: "foo"}, + }}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := ProcessStruct(tt.args.s); (err != nil) != tt.wantErr { + t.Errorf("ProcessStruct() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}