From 2c4de43b75b0598099cfc64ab0501eac4a9ddf98 Mon Sep 17 00:00:00 2001 From: balopat Date: Mon, 1 Apr 2019 18:39:19 -0700 Subject: [PATCH 01/10] fixing non-oneof inline struct handling in schemas --- docs/content/en/schemas/v1alpha1.json | 130 +++++------------- docs/content/en/schemas/v1alpha2.json | 33 ++--- docs/content/en/schemas/v1alpha3.json | 33 ++--- docs/content/en/schemas/v1alpha4.json | 33 ++--- docs/content/en/schemas/v1alpha5.json | 33 ++--- docs/content/en/schemas/v1beta1.json | 33 ++--- docs/content/en/schemas/v1beta2.json | 33 ++--- docs/content/en/schemas/v1beta3.json | 33 ++--- docs/content/en/schemas/v1beta4.json | 33 ++--- docs/content/en/schemas/v1beta5.json | 41 ++---- docs/content/en/schemas/v1beta6.json | 41 ++---- docs/content/en/schemas/v1beta7.json | 41 ++---- docs/content/en/schemas/v1beta8.json | 0 hack/schemas/main.go | 50 +++++-- hack/schemas/main_test.go | 37 +++++ hack/schemas/testdata/inline-anyof/input.go | 34 +++++ .../schemas/testdata/inline-anyof/output.json | 92 +++++++++++++ hack/schemas/testdata/inline/input.go | 37 +++++ hack/schemas/testdata/inline/output.json | 78 +++++++++++ pkg/skaffold/schema/profiles.go | 4 +- testutil/util.go | 9 ++ 21 files changed, 472 insertions(+), 386 deletions(-) delete mode 100755 docs/content/en/schemas/v1beta8.json create mode 100644 hack/schemas/testdata/inline-anyof/input.go create mode 100644 hack/schemas/testdata/inline-anyof/output.json create mode 100644 hack/schemas/testdata/inline/input.go create mode 100644 hack/schemas/testdata/inline/output.json diff --git a/docs/content/en/schemas/v1alpha1.json b/docs/content/en/schemas/v1alpha1.json index 40b63289f49..bb203a8bf85 100755 --- a/docs/content/en/schemas/v1alpha1.json +++ b/docs/content/en/schemas/v1alpha1.json @@ -37,70 +37,30 @@ "x-intellij-html-description": "represents items that need should be built, along with the context in which they should be built." }, "BuildConfig": { - "anyOf": [ - { - "properties": { - "artifacts": { - "items": { - "$ref": "#/definitions/Artifact" - }, - "type": "array" - }, - "tagPolicy": { - "type": "string" - } + "properties": { + "artifacts": { + "items": { + "$ref": "#/definitions/Artifact" }, - "preferredOrder": [ - "artifacts", - "tagPolicy" - ], - "additionalProperties": false + "type": "array" }, - { - "properties": { - "artifacts": { - "items": { - "$ref": "#/definitions/Artifact" - }, - "type": "array" - }, - "local": { - "$ref": "#/definitions/LocalBuild" - }, - "tagPolicy": { - "type": "string" - } - }, - "preferredOrder": [ - "artifacts", - "tagPolicy", - "local" - ], - "additionalProperties": false + "googleCloudBuild": { + "$ref": "#/definitions/GoogleCloudBuild" }, - { - "properties": { - "artifacts": { - "items": { - "$ref": "#/definitions/Artifact" - }, - "type": "array" - }, - "googleCloudBuild": { - "$ref": "#/definitions/GoogleCloudBuild" - }, - "tagPolicy": { - "type": "string" - } - }, - "preferredOrder": [ - "artifacts", - "tagPolicy", - "googleCloudBuild" - ], - "additionalProperties": false + "local": { + "$ref": "#/definitions/LocalBuild" + }, + "tagPolicy": { + "type": "string" } + }, + "preferredOrder": [ + "artifacts", + "tagPolicy", + "local", + "googleCloudBuild" ], + "additionalProperties": false, "description": "contains all the configuration for the build steps", "x-intellij-html-description": "contains all the configuration for the build steps" }, @@ -122,49 +82,23 @@ "x-intellij-html-description": "contains the specific implementation and parameters needed for the build step. Only one field should be populated." }, "DeployConfig": { - "anyOf": [ - { - "properties": { - "name": { - "type": "string" - } - }, - "preferredOrder": [ - "name" - ], - "additionalProperties": false + "properties": { + "helm": { + "$ref": "#/definitions/HelmDeploy" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmDeploy" - }, - "name": { - "type": "string" - } - }, - "preferredOrder": [ - "name", - "helm" - ], - "additionalProperties": false + "kubectl": { + "$ref": "#/definitions/KubectlDeploy" }, - { - "properties": { - "kubectl": { - "$ref": "#/definitions/KubectlDeploy" - }, - "name": { - "type": "string" - } - }, - "preferredOrder": [ - "name", - "kubectl" - ], - "additionalProperties": false + "name": { + "type": "string" } + }, + "preferredOrder": [ + "name", + "helm", + "kubectl" ], + "additionalProperties": false, "description": "contains all the configuration needed by the deploy steps", "x-intellij-html-description": "contains all the configuration needed by the deploy steps" }, diff --git a/docs/content/en/schemas/v1alpha2.json b/docs/content/en/schemas/v1alpha2.json index 5cde3badaa6..2480a0e9929 100755 --- a/docs/content/en/schemas/v1alpha2.json +++ b/docs/content/en/schemas/v1alpha2.json @@ -409,32 +409,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1alpha3.json b/docs/content/en/schemas/v1alpha3.json index 3fe5354f4e4..f705a3d3ae9 100755 --- a/docs/content/en/schemas/v1alpha3.json +++ b/docs/content/en/schemas/v1alpha3.json @@ -409,32 +409,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1alpha4.json b/docs/content/en/schemas/v1alpha4.json index 49d2bab8a1d..4e301980662 100755 --- a/docs/content/en/schemas/v1alpha4.json +++ b/docs/content/en/schemas/v1alpha4.json @@ -499,32 +499,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1alpha5.json b/docs/content/en/schemas/v1alpha5.json index 8f08e24e151..be41d19d2f8 100755 --- a/docs/content/en/schemas/v1alpha5.json +++ b/docs/content/en/schemas/v1alpha5.json @@ -550,32 +550,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta1.json b/docs/content/en/schemas/v1beta1.json index ef80743cf5a..c22998545e2 100755 --- a/docs/content/en/schemas/v1beta1.json +++ b/docs/content/en/schemas/v1beta1.json @@ -507,32 +507,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta2.json b/docs/content/en/schemas/v1beta2.json index 8f94a85f546..324e84256fc 100755 --- a/docs/content/en/schemas/v1beta2.json +++ b/docs/content/en/schemas/v1beta2.json @@ -507,32 +507,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta3.json b/docs/content/en/schemas/v1beta3.json index dba07be6d16..c9bdb8c2dd2 100755 --- a/docs/content/en/schemas/v1beta3.json +++ b/docs/content/en/schemas/v1beta3.json @@ -532,32 +532,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta4.json b/docs/content/en/schemas/v1beta4.json index fe667fd6725..19800221fd0 100755 --- a/docs/content/en/schemas/v1beta4.json +++ b/docs/content/en/schemas/v1beta4.json @@ -553,32 +553,17 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig" - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig" } + }, + "preferredOrder": [ + "fqn", + "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta5.json b/docs/content/en/schemas/v1beta5.json index 558a585bcaf..974f61a36a7 100755 --- a/docs/content/en/schemas/v1beta5.json +++ b/docs/content/en/schemas/v1beta5.json @@ -868,36 +868,21 @@ "additionalProperties": false }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig", - "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig", + "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig", - "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig", + "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." } + }, + "preferredOrder": [ + "fqn", + "helm" ], "description": "adds image configurations to the Helm `values` file.", "x-intellij-html-description": "adds image configurations to the Helm values file." diff --git a/docs/content/en/schemas/v1beta6.json b/docs/content/en/schemas/v1beta6.json index 221a76d5e44..b1296efc895 100755 --- a/docs/content/en/schemas/v1beta6.json +++ b/docs/content/en/schemas/v1beta6.json @@ -919,36 +919,21 @@ "x-intellij-html-description": "describes an image configuration." }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig", - "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig", + "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig", - "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig", + "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." } + }, + "preferredOrder": [ + "fqn", + "helm" ], "description": "adds image configurations to the Helm `values` file.", "x-intellij-html-description": "adds image configurations to the Helm values file." diff --git a/docs/content/en/schemas/v1beta7.json b/docs/content/en/schemas/v1beta7.json index f93c16490f8..54b43ebe067 100755 --- a/docs/content/en/schemas/v1beta7.json +++ b/docs/content/en/schemas/v1beta7.json @@ -1013,36 +1013,21 @@ "x-intellij-html-description": "describes an image configuration." }, "HelmImageStrategy": { - "anyOf": [ - { - "additionalProperties": false - }, - { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig", - "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." - } - }, - "preferredOrder": [ - "fqn" - ], - "additionalProperties": false + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig", + "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." }, - { - "properties": { - "helm": { - "$ref": "#/definitions/HelmConventionConfig", - "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." - } - }, - "preferredOrder": [ - "helm" - ], - "additionalProperties": false + "helm": { + "$ref": "#/definitions/HelmConventionConfig", + "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." } + }, + "preferredOrder": [ + "fqn", + "helm" ], "description": "adds image configurations to the Helm `values` file.", "x-intellij-html-description": "adds image configurations to the Helm values file." diff --git a/docs/content/en/schemas/v1beta8.json b/docs/content/en/schemas/v1beta8.json deleted file mode 100755 index e69de29bb2d..00000000000 diff --git a/hack/schemas/main.go b/hack/schemas/main.go index fde1bd8ca05..522eff59c7b 100644 --- a/hack/schemas/main.go +++ b/hack/schemas/main.go @@ -68,6 +68,9 @@ type Definition struct { HTMLDescription string `json:"x-intellij-html-description,omitempty"` Default interface{} `json:"default,omitempty"` Examples []string `json:"examples,omitempty"` + + inlines []*Definition + tags string } func main() { @@ -145,8 +148,10 @@ func setTypeOrRef(def *Definition, typeName string) { } } -func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string) *Definition { - def := &Definition{} +func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string, tags string) *Definition { + def := &Definition{ + tags: tags, + } switch tt := t.(type) { case *ast.Ident: @@ -172,7 +177,7 @@ func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string) case *ast.ArrayType: def.Type = "array" - def.Items = g.newDefinition("", tt.Elt, "") + def.Items = g.newDefinition("", tt.Elt, "", "") if def.Items.Ref == "" { def.Default = "[]" } @@ -180,14 +185,14 @@ func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string) case *ast.MapType: def.Type = "object" def.Default = "{}" - def.AdditionalProperties = g.newDefinition("", tt.Value, "") + def.AdditionalProperties = g.newDefinition("", tt.Value, "", "") case *ast.StructType: for _, field := range tt.Fields.List { yamlName := yamlFieldName(field) if strings.Contains(field.Tag.Value, "inline") { - def.AnyOf = append(def.AnyOf, &Definition{ + def.inlines = append(def.inlines, &Definition{ Ref: defPrefix + field.Type.(*ast.Ident).Name, }) continue @@ -206,7 +211,7 @@ func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string) } def.PreferredOrder = append(def.PreferredOrder, yamlName) - def.Properties[yamlName] = g.newDefinition(field.Names[0].Name, field.Type, field.Doc.Text()) + def.Properties[yamlName] = g.newDefinition(field.Names[0].Name, field.Type, field.Doc.Text(), field.Tag.Value) def.AdditionalProperties = false } } @@ -251,6 +256,11 @@ func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string) return def } +func isOneOf(definition *Definition) bool { + return len(definition.Properties) > 0 && + strings.Contains(definition.Properties[definition.PreferredOrder[0]].tags, "oneOf=") +} + func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { fset := token.NewFileSet() node, err := parser.ParseFile(fset, inputPath, nil, parser.ParseComments) @@ -275,17 +285,19 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { name := typeSpec.Name.Name preferredOrder = append(preferredOrder, name) - definitions[name] = g.newDefinition(name, typeSpec.Type, declaration.Doc.Text()) + definitions[name] = g.newDefinition(name, typeSpec.Type, declaration.Doc.Text(), "") } } // Inline anyOfs for _, k := range preferredOrder { def := definitions[k] - if len(def.AnyOf) == 0 { + if len(def.inlines) == 0 { continue } + //merge if not anyof + var options []*Definition options = append(options, &Definition{ Properties: def.Properties, @@ -293,10 +305,24 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { AdditionalProperties: false, }) - for _, anyOf := range def.AnyOf { - ref := strings.TrimPrefix(anyOf.Ref, defPrefix) + for _, inlineStruct := range def.inlines { + + ref := strings.TrimPrefix(inlineStruct.Ref, defPrefix) referenced := definitions[ref] + //if not anyof, continue + if !isOneOf(referenced) { + if def.Properties == nil { + def.Properties = make(map[string]*Definition) + } + for k, v := range referenced.Properties { + def.Properties[k] = v + } + def.PreferredOrder = append(def.PreferredOrder, referenced.PreferredOrder...) + def.Required = append(def.Required, referenced.Required...) + continue + } + for _, key := range referenced.PreferredOrder { var preferredOrder []string choice := make(map[string]*Definition) @@ -319,6 +345,10 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { } } + if len(options) == 1 { + continue + } + def.Properties = nil def.PreferredOrder = nil def.AdditionalProperties = nil diff --git a/hack/schemas/main_test.go b/hack/schemas/main_test.go index 187179af5f9..9be2699e1ab 100644 --- a/hack/schemas/main_test.go +++ b/hack/schemas/main_test.go @@ -17,7 +17,12 @@ limitations under the License. package main import ( + "fmt" + "io/ioutil" + "os" "testing" + + "github.com/GoogleContainerTools/skaffold/testutil" ) func TestSchemas(t *testing.T) { @@ -30,3 +35,35 @@ func TestSchemas(t *testing.T) { t.Fatal("json schema files are not up to date. Please run `make generate-schemas` and commit the changes.") } } + +func TestGenerators(t *testing.T) { + tcs := []struct { + name string + }{ + {name: "inline"}, + {name: "inline-anyof"}, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + input := fmt.Sprintf("./testdata/%s/input.go", tc.name) + expectedOutput := fmt.Sprintf("./testdata/%s/output.json", tc.name) + + generator := schemaGenerator{ + strict: false, + } + + actual, err := generator.Apply(input) + testutil.CheckError(t, false, err) + + var expected []byte + if _, err := os.Stat(expectedOutput); err == nil { + var err error + expected, err = ioutil.ReadFile(expectedOutput) + testutil.CheckError(t, false, err) + } + + testutil.CheckMultilineDeepEqual(t, string(expected), string(actual)) + }) + } +} diff --git a/hack/schemas/testdata/inline-anyof/input.go b/hack/schemas/testdata/inline-anyof/input.go new file mode 100644 index 00000000000..6704e87afa8 --- /dev/null +++ b/hack/schemas/testdata/inline-anyof/input.go @@ -0,0 +1,34 @@ +/* +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 latest + +// TestStruct for testing the schema generator. +type TestStruct struct { + // RequiredField should be required + RequiredField string `yaml:"reqField" yamltags:"required"` + InlineOneOfStruct `yaml:"inline"` +} + +//InlineOneOfStruct is embedded inline into TestStruct +type InlineOneOfStruct struct { + + //Field1 should be the first choice + Field1 string `yaml:"field1" yamltags:"oneOf=tag"` + + //Field2 should be the second choice + Field2 string `yaml:"field2" yamltags:"oneOf=tag"` +} diff --git a/hack/schemas/testdata/inline-anyof/output.json b/hack/schemas/testdata/inline-anyof/output.json new file mode 100644 index 00000000000..d2dbcbaee82 --- /dev/null +++ b/hack/schemas/testdata/inline-anyof/output.json @@ -0,0 +1,92 @@ +{ + "type": "object", + "anyOf": [ + { + "$ref": "#/definitions/SkaffoldPipeline" + } + ], + "$schema": "http://json-schema-org/draft-07/schema#", + "definitions": { + "InlineOneOfStruct": { + "properties": { + "field1": { + "type": "string", + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" + }, + "field2": { + "type": "string", + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" + } + }, + "preferredOrder": [ + "field1", + "field2" + ], + "additionalProperties": false, + "description": "embedded inline into TestStruct", + "x-intellij-html-description": "embedded inline into TestStruct" + }, + "TestStruct": { + "required": [ + "reqField" + ], + "anyOf": [ + { + "properties": { + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + } + }, + "preferredOrder": [ + "reqField" + ], + "additionalProperties": false + }, + { + "properties": { + "field1": { + "type": "string", + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" + }, + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + } + }, + "preferredOrder": [ + "reqField", + "field1" + ], + "additionalProperties": false + }, + { + "properties": { + "field2": { + "type": "string", + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" + }, + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + } + }, + "preferredOrder": [ + "reqField", + "field2" + ], + "additionalProperties": false + } + ], + "description": "for testing the schema generator.", + "x-intellij-html-description": "for testing the schema generator." + } + } +} diff --git a/hack/schemas/testdata/inline/input.go b/hack/schemas/testdata/inline/input.go new file mode 100644 index 00000000000..f42738e5d49 --- /dev/null +++ b/hack/schemas/testdata/inline/input.go @@ -0,0 +1,37 @@ +/* +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 latest + +// TestStruct for testing the schema generator. +type TestStruct struct { + // RequiredField should be required + RequiredField string `yaml:"reqField" yamltags:"required"` + InlineStruct `yaml:"inline"` +} + +//InlineOneOfStruct is embedded inline into TestStruct +type InlineStruct struct { + + //Field1 should be the first field + Field1 string `yaml:"field1"` + + //Field2 should be the second field + Field2 string `yaml:"field2"` + + //Field3 should be the third field and listed in required + RequiredField3 string `yaml:"reqField3" yamltags:"required"` +} diff --git a/hack/schemas/testdata/inline/output.json b/hack/schemas/testdata/inline/output.json new file mode 100644 index 00000000000..4d933a8333a --- /dev/null +++ b/hack/schemas/testdata/inline/output.json @@ -0,0 +1,78 @@ +{ + "type": "object", + "anyOf": [ + { + "$ref": "#/definitions/SkaffoldPipeline" + } + ], + "$schema": "http://json-schema-org/draft-07/schema#", + "definitions": { + "InlineStruct": { + "required": [ + "reqField3" + ], + "properties": { + "field1": { + "type": "string", + "description": "should be the first field", + "x-intellij-html-description": "should be the first field" + }, + "field2": { + "type": "string", + "description": "should be the second field", + "x-intellij-html-description": "should be the second field" + }, + "reqField3": { + "type": "string", + "description": "Field3 should be the third field and listed in required", + "x-intellij-html-description": "Field3 should be the third field and listed in required" + } + }, + "preferredOrder": [ + "field1", + "field2", + "reqField3" + ], + "additionalProperties": false, + "description": "InlineOneOfStruct is embedded inline into TestStruct", + "x-intellij-html-description": "InlineOneOfStruct is embedded inline into TestStruct" + }, + "TestStruct": { + "required": [ + "reqField", + "reqField3" + ], + "properties": { + "field1": { + "type": "string", + "description": "should be the first field", + "x-intellij-html-description": "should be the first field" + }, + "field2": { + "type": "string", + "description": "should be the second field", + "x-intellij-html-description": "should be the second field" + }, + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + }, + "reqField3": { + "type": "string", + "description": "Field3 should be the third field and listed in required", + "x-intellij-html-description": "Field3 should be the third field and listed in required" + } + }, + "preferredOrder": [ + "reqField", + "field1", + "field2", + "reqField3" + ], + "additionalProperties": false, + "description": "for testing the schema generator.", + "x-intellij-html-description": "for testing the schema generator." + } + } +} diff --git a/pkg/skaffold/schema/profiles.go b/pkg/skaffold/schema/profiles.go index 27181d357aa..0d1a8340726 100644 --- a/pkg/skaffold/schema/profiles.go +++ b/pkg/skaffold/schema/profiles.go @@ -243,7 +243,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 IsOneOf(t.Field(0)) { return overlayOneOfField(config, profile) } return overlayStructField(config, profile) @@ -265,7 +265,7 @@ func overlayProfileField(config interface{}, profile interface{}) interface{} { } } -func isOneOf(field reflect.StructField) bool { +func IsOneOf(field reflect.StructField) bool { for _, tag := range strings.Split(field.Tag.Get("yamltags"), ",") { tagParts := strings.Split(tag, "=") diff --git a/testutil/util.go b/testutil/util.go index 51d77f9ec3b..7bf004bdc1e 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -46,6 +46,15 @@ func CheckDeepEqual(t *testing.T, expected, actual interface{}) { } } +func CheckMultilineDeepEqual(t *testing.T, expected, actual string) { + t.Helper() + + if diff := cmp.Diff(actual, expected); diff != "" { + t.Errorf("%T differ (-want, +got): \n%s, expected: \n %s actual: \n%s", expected, diff, expected, actual) + return + } +} + func CheckDeepEqualWithOptions(t *testing.T, options cmp.Options, expected, actual interface{}) { t.Helper() if diff := cmp.Diff(actual, expected, options); diff != "" { From e19f26c970b863e53811351f1cc702848d8bbb12 Mon Sep 17 00:00:00 2001 From: balopat Date: Mon, 1 Apr 2019 18:42:12 -0700 Subject: [PATCH 02/10] revert testutil changes --- hack/schemas/main_test.go | 2 +- testutil/util.go | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/hack/schemas/main_test.go b/hack/schemas/main_test.go index 9be2699e1ab..7457d708e41 100644 --- a/hack/schemas/main_test.go +++ b/hack/schemas/main_test.go @@ -63,7 +63,7 @@ func TestGenerators(t *testing.T) { testutil.CheckError(t, false, err) } - testutil.CheckMultilineDeepEqual(t, string(expected), string(actual)) + testutil.CheckDeepEqual(t, string(expected), string(actual)) }) } } diff --git a/testutil/util.go b/testutil/util.go index 7bf004bdc1e..51d77f9ec3b 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -46,15 +46,6 @@ func CheckDeepEqual(t *testing.T, expected, actual interface{}) { } } -func CheckMultilineDeepEqual(t *testing.T, expected, actual string) { - t.Helper() - - if diff := cmp.Diff(actual, expected); diff != "" { - t.Errorf("%T differ (-want, +got): \n%s, expected: \n %s actual: \n%s", expected, diff, expected, actual) - return - } -} - func CheckDeepEqualWithOptions(t *testing.T, options cmp.Options, expected, actual interface{}) { t.Helper() if diff := cmp.Diff(actual, expected, options); diff != "" { From 8e1b4a0a80336cf6ae50c029d2baf190ee6bb89d Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 08:11:15 -0700 Subject: [PATCH 03/10] fix helm image strategy oneOf --- docs/content/en/schemas/v1beta7.json | 41 +++++++++++++++++++--------- pkg/skaffold/schema/latest/config.go | 4 +-- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/docs/content/en/schemas/v1beta7.json b/docs/content/en/schemas/v1beta7.json index 54b43ebe067..f93c16490f8 100755 --- a/docs/content/en/schemas/v1beta7.json +++ b/docs/content/en/schemas/v1beta7.json @@ -1013,21 +1013,36 @@ "x-intellij-html-description": "describes an image configuration." }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig", - "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig", - "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig", + "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig", + "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ], "description": "adds image configurations to the Helm `values` file.", "x-intellij-html-description": "adds image configurations to the Helm values file." diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go index ddb4fec512b..d594756927a 100644 --- a/pkg/skaffold/schema/latest/config.go +++ b/pkg/skaffold/schema/latest/config.go @@ -430,10 +430,10 @@ type HelmImageStrategy struct { // HelmImageConfig describes an image configuration. type HelmImageConfig struct { // HelmFQNConfig is the image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`. - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` // HelmConventionConfig is the image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`. - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig is the image config to use the FullyQualifiedImageName as param to set. From 8e075318da817a758e6e682571062cb1caaba4bb Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 08:19:56 -0700 Subject: [PATCH 04/10] fix helm image strategy oneOf --- docs/content/en/schemas/v1alpha1.json | 130 +++++++++++++++++++------ docs/content/en/schemas/v1alpha2.json | 33 +++++-- docs/content/en/schemas/v1alpha3.json | 33 +++++-- docs/content/en/schemas/v1alpha4.json | 33 +++++-- docs/content/en/schemas/v1alpha5.json | 33 +++++-- docs/content/en/schemas/v1beta1.json | 33 +++++-- docs/content/en/schemas/v1beta2.json | 33 +++++-- docs/content/en/schemas/v1beta3.json | 33 +++++-- docs/content/en/schemas/v1beta4.json | 33 +++++-- docs/content/en/schemas/v1beta5.json | 41 +++++--- docs/content/en/schemas/v1beta6.json | 41 +++++--- pkg/skaffold/schema/v1alpha1/config.go | 8 +- pkg/skaffold/schema/v1alpha2/config.go | 4 +- pkg/skaffold/schema/v1alpha3/config.go | 4 +- pkg/skaffold/schema/v1alpha4/config.go | 4 +- pkg/skaffold/schema/v1alpha5/config.go | 4 +- pkg/skaffold/schema/v1beta1/config.go | 4 +- pkg/skaffold/schema/v1beta2/config.go | 4 +- pkg/skaffold/schema/v1beta3/config.go | 4 +- pkg/skaffold/schema/v1beta4/config.go | 4 +- pkg/skaffold/schema/v1beta5/config.go | 4 +- pkg/skaffold/schema/v1beta6/config.go | 4 +- 22 files changed, 370 insertions(+), 154 deletions(-) diff --git a/docs/content/en/schemas/v1alpha1.json b/docs/content/en/schemas/v1alpha1.json index bb203a8bf85..40b63289f49 100755 --- a/docs/content/en/schemas/v1alpha1.json +++ b/docs/content/en/schemas/v1alpha1.json @@ -37,30 +37,70 @@ "x-intellij-html-description": "represents items that need should be built, along with the context in which they should be built." }, "BuildConfig": { - "properties": { - "artifacts": { - "items": { - "$ref": "#/definitions/Artifact" + "anyOf": [ + { + "properties": { + "artifacts": { + "items": { + "$ref": "#/definitions/Artifact" + }, + "type": "array" + }, + "tagPolicy": { + "type": "string" + } }, - "type": "array" - }, - "googleCloudBuild": { - "$ref": "#/definitions/GoogleCloudBuild" + "preferredOrder": [ + "artifacts", + "tagPolicy" + ], + "additionalProperties": false }, - "local": { - "$ref": "#/definitions/LocalBuild" + { + "properties": { + "artifacts": { + "items": { + "$ref": "#/definitions/Artifact" + }, + "type": "array" + }, + "local": { + "$ref": "#/definitions/LocalBuild" + }, + "tagPolicy": { + "type": "string" + } + }, + "preferredOrder": [ + "artifacts", + "tagPolicy", + "local" + ], + "additionalProperties": false }, - "tagPolicy": { - "type": "string" + { + "properties": { + "artifacts": { + "items": { + "$ref": "#/definitions/Artifact" + }, + "type": "array" + }, + "googleCloudBuild": { + "$ref": "#/definitions/GoogleCloudBuild" + }, + "tagPolicy": { + "type": "string" + } + }, + "preferredOrder": [ + "artifacts", + "tagPolicy", + "googleCloudBuild" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "artifacts", - "tagPolicy", - "local", - "googleCloudBuild" ], - "additionalProperties": false, "description": "contains all the configuration for the build steps", "x-intellij-html-description": "contains all the configuration for the build steps" }, @@ -82,23 +122,49 @@ "x-intellij-html-description": "contains the specific implementation and parameters needed for the build step. Only one field should be populated." }, "DeployConfig": { - "properties": { - "helm": { - "$ref": "#/definitions/HelmDeploy" + "anyOf": [ + { + "properties": { + "name": { + "type": "string" + } + }, + "preferredOrder": [ + "name" + ], + "additionalProperties": false }, - "kubectl": { - "$ref": "#/definitions/KubectlDeploy" + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmDeploy" + }, + "name": { + "type": "string" + } + }, + "preferredOrder": [ + "name", + "helm" + ], + "additionalProperties": false }, - "name": { - "type": "string" + { + "properties": { + "kubectl": { + "$ref": "#/definitions/KubectlDeploy" + }, + "name": { + "type": "string" + } + }, + "preferredOrder": [ + "name", + "kubectl" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "name", - "helm", - "kubectl" ], - "additionalProperties": false, "description": "contains all the configuration needed by the deploy steps", "x-intellij-html-description": "contains all the configuration needed by the deploy steps" }, diff --git a/docs/content/en/schemas/v1alpha2.json b/docs/content/en/schemas/v1alpha2.json index 2480a0e9929..5cde3badaa6 100755 --- a/docs/content/en/schemas/v1alpha2.json +++ b/docs/content/en/schemas/v1alpha2.json @@ -409,17 +409,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1alpha3.json b/docs/content/en/schemas/v1alpha3.json index f705a3d3ae9..3fe5354f4e4 100755 --- a/docs/content/en/schemas/v1alpha3.json +++ b/docs/content/en/schemas/v1alpha3.json @@ -409,17 +409,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1alpha4.json b/docs/content/en/schemas/v1alpha4.json index 4e301980662..49d2bab8a1d 100755 --- a/docs/content/en/schemas/v1alpha4.json +++ b/docs/content/en/schemas/v1alpha4.json @@ -499,17 +499,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1alpha5.json b/docs/content/en/schemas/v1alpha5.json index be41d19d2f8..8f08e24e151 100755 --- a/docs/content/en/schemas/v1alpha5.json +++ b/docs/content/en/schemas/v1alpha5.json @@ -550,17 +550,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta1.json b/docs/content/en/schemas/v1beta1.json index c22998545e2..ef80743cf5a 100755 --- a/docs/content/en/schemas/v1beta1.json +++ b/docs/content/en/schemas/v1beta1.json @@ -507,17 +507,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta2.json b/docs/content/en/schemas/v1beta2.json index 324e84256fc..8f94a85f546 100755 --- a/docs/content/en/schemas/v1beta2.json +++ b/docs/content/en/schemas/v1beta2.json @@ -507,17 +507,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta3.json b/docs/content/en/schemas/v1beta3.json index c9bdb8c2dd2..dba07be6d16 100755 --- a/docs/content/en/schemas/v1beta3.json +++ b/docs/content/en/schemas/v1beta3.json @@ -532,17 +532,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta4.json b/docs/content/en/schemas/v1beta4.json index 19800221fd0..fe667fd6725 100755 --- a/docs/content/en/schemas/v1beta4.json +++ b/docs/content/en/schemas/v1beta4.json @@ -553,17 +553,32 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig" + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig" + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig" + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig" + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ] }, "HelmPackaged": { diff --git a/docs/content/en/schemas/v1beta5.json b/docs/content/en/schemas/v1beta5.json index 974f61a36a7..558a585bcaf 100755 --- a/docs/content/en/schemas/v1beta5.json +++ b/docs/content/en/schemas/v1beta5.json @@ -868,21 +868,36 @@ "additionalProperties": false }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig", - "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig", - "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig", + "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig", + "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ], "description": "adds image configurations to the Helm `values` file.", "x-intellij-html-description": "adds image configurations to the Helm values file." diff --git a/docs/content/en/schemas/v1beta6.json b/docs/content/en/schemas/v1beta6.json index b1296efc895..221a76d5e44 100755 --- a/docs/content/en/schemas/v1beta6.json +++ b/docs/content/en/schemas/v1beta6.json @@ -919,21 +919,36 @@ "x-intellij-html-description": "describes an image configuration." }, "HelmImageStrategy": { - "properties": { - "fqn": { - "$ref": "#/definitions/HelmFQNConfig", - "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." + "anyOf": [ + { + "additionalProperties": false }, - "helm": { - "$ref": "#/definitions/HelmConventionConfig", - "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", - "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." + { + "properties": { + "fqn": { + "$ref": "#/definitions/HelmFQNConfig", + "description": "image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG." + } + }, + "preferredOrder": [ + "fqn" + ], + "additionalProperties": false + }, + { + "properties": { + "helm": { + "$ref": "#/definitions/HelmConventionConfig", + "description": "image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`.", + "x-intellij-html-description": "image configuration uses the syntax IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG." + } + }, + "preferredOrder": [ + "helm" + ], + "additionalProperties": false } - }, - "preferredOrder": [ - "fqn", - "helm" ], "description": "adds image configurations to the Helm `values` file.", "x-intellij-html-description": "adds image configurations to the Helm values file." diff --git a/pkg/skaffold/schema/v1alpha1/config.go b/pkg/skaffold/schema/v1alpha1/config.go index 75c1c7c9d88..764bc3030e9 100644 --- a/pkg/skaffold/schema/v1alpha1/config.go +++ b/pkg/skaffold/schema/v1alpha1/config.go @@ -51,8 +51,8 @@ type BuildConfig 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,omitempty"` - GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild,omitempty"` + LocalBuild *LocalBuild `yaml:"local,omitempty" yamltags:"oneOf=build"` + GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild,omitempty" yamltags:"oneOf=build"` } // LocalBuild contains the fields needed to do a build on the local docker daemon @@ -74,8 +74,8 @@ 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,omitempty"` - KubectlDeploy *KubectlDeploy `yaml:"kubectl,omitempty"` + HelmDeploy *HelmDeploy `yaml:"helm,omitempty" yamltags:"oneOf=deploy"` + KubectlDeploy *KubectlDeploy `yaml:"kubectl,omitempty" yamltags:"oneOf=deploy"` } // KubectlDeploy contains the configuration needed for deploying with `kubectl apply` diff --git a/pkg/skaffold/schema/v1alpha2/config.go b/pkg/skaffold/schema/v1alpha2/config.go index 8cca945e236..13d00f8119b 100644 --- a/pkg/skaffold/schema/v1alpha2/config.go +++ b/pkg/skaffold/schema/v1alpha2/config.go @@ -177,8 +177,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1alpha3/config.go b/pkg/skaffold/schema/v1alpha3/config.go index f471b784a21..58beb41e912 100644 --- a/pkg/skaffold/schema/v1alpha3/config.go +++ b/pkg/skaffold/schema/v1alpha3/config.go @@ -183,8 +183,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1alpha4/config.go b/pkg/skaffold/schema/v1alpha4/config.go index f81c220e5bf..3ac8e4ee5a1 100644 --- a/pkg/skaffold/schema/v1alpha4/config.go +++ b/pkg/skaffold/schema/v1alpha4/config.go @@ -200,8 +200,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1alpha5/config.go b/pkg/skaffold/schema/v1alpha5/config.go index aaa31755fc8..89f5fd5cddf 100644 --- a/pkg/skaffold/schema/v1alpha5/config.go +++ b/pkg/skaffold/schema/v1alpha5/config.go @@ -210,8 +210,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1beta1/config.go b/pkg/skaffold/schema/v1beta1/config.go index 76f9810611c..968320cd6eb 100644 --- a/pkg/skaffold/schema/v1beta1/config.go +++ b/pkg/skaffold/schema/v1beta1/config.go @@ -206,8 +206,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1beta2/config.go b/pkg/skaffold/schema/v1beta2/config.go index 865f4ad09a8..66092345ffe 100644 --- a/pkg/skaffold/schema/v1beta2/config.go +++ b/pkg/skaffold/schema/v1beta2/config.go @@ -207,8 +207,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1beta3/config.go b/pkg/skaffold/schema/v1beta3/config.go index fc9ae06dddb..df12fe88fe9 100644 --- a/pkg/skaffold/schema/v1beta3/config.go +++ b/pkg/skaffold/schema/v1beta3/config.go @@ -216,8 +216,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1beta4/config.go b/pkg/skaffold/schema/v1beta4/config.go index daa3ecf1dfb..5421a31368b 100644 --- a/pkg/skaffold/schema/v1beta4/config.go +++ b/pkg/skaffold/schema/v1beta4/config.go @@ -218,8 +218,8 @@ type HelmImageStrategy struct { } type HelmImageConfig struct { - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig represents image config to use the FullyQualifiedImageName as param to set diff --git a/pkg/skaffold/schema/v1beta5/config.go b/pkg/skaffold/schema/v1beta5/config.go index 057dd717e3e..1e9ade439a8 100644 --- a/pkg/skaffold/schema/v1beta5/config.go +++ b/pkg/skaffold/schema/v1beta5/config.go @@ -422,10 +422,10 @@ type HelmImageStrategy struct { type HelmImageConfig struct { // HelmFQNConfig is the image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`. - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` // HelmConventionConfig is the image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`. - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig is the image config to use the FullyQualifiedImageName as param to set. diff --git a/pkg/skaffold/schema/v1beta6/config.go b/pkg/skaffold/schema/v1beta6/config.go index a0d3b6a102c..7cc303e074b 100644 --- a/pkg/skaffold/schema/v1beta6/config.go +++ b/pkg/skaffold/schema/v1beta6/config.go @@ -443,10 +443,10 @@ type HelmImageStrategy struct { // HelmImageConfig describes an image configuration. type HelmImageConfig struct { // HelmFQNConfig is the image configuration uses the syntax `IMAGE-NAME=IMAGE-REPOSITORY:IMAGE-TAG`. - HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty"` + HelmFQNConfig *HelmFQNConfig `yaml:"fqn,omitempty" yamltags:"oneOf=helmImageStrategy"` // HelmConventionConfig is the image configuration uses the syntax `IMAGE-NAME.repository=IMAGE-REPOSITORY, IMAGE-NAME.tag=IMAGE-TAG`. - HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty"` + HelmConventionConfig *HelmConventionConfig `yaml:"helm,omitempty" yamltags:"oneOf=helmImageStrategy"` } // HelmFQNConfig is the image config to use the FullyQualifiedImageName as param to set. From c753be6d30a2fb67289f1a739acf315e4cefd671 Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 08:21:32 -0700 Subject: [PATCH 05/10] revert profiles change --- pkg/skaffold/schema/profiles.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/schema/profiles.go b/pkg/skaffold/schema/profiles.go index 0d1a8340726..27181d357aa 100644 --- a/pkg/skaffold/schema/profiles.go +++ b/pkg/skaffold/schema/profiles.go @@ -243,7 +243,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 isOneOf(t.Field(0)) { return overlayOneOfField(config, profile) } return overlayStructField(config, profile) @@ -265,7 +265,7 @@ func overlayProfileField(config interface{}, profile interface{}) interface{} { } } -func IsOneOf(field reflect.StructField) bool { +func isOneOf(field reflect.StructField) bool { for _, tag := range strings.Split(field.Tag.Get("yamltags"), ",") { tagParts := strings.Split(tag, "=") From d268e9c637e7e37c2c5542929eae9e618543a240 Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 08:46:32 -0700 Subject: [PATCH 06/10] fix nonzero behavior --- pkg/skaffold/yamltags/tags.go | 3 +++ pkg/skaffold/yamltags/tags_test.go | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/pkg/skaffold/yamltags/tags.go b/pkg/skaffold/yamltags/tags.go index 2dcf26ca77f..e42e99a7a92 100644 --- a/pkg/skaffold/yamltags/tags.go +++ b/pkg/skaffold/yamltags/tags.go @@ -185,6 +185,9 @@ func (oot *OneOfTag) Process(val reflect.Value) error { } func isZeroValue(val reflect.Value) bool { + if val.Kind() == reflect.Invalid { + return true + } zv := reflect.Zero(val.Type()).Interface() return reflect.DeepEqual(zv, val.Interface()) } diff --git a/pkg/skaffold/yamltags/tags_test.go b/pkg/skaffold/yamltags/tags_test.go index 1b2f4fd3f30..2de4ea3d897 100644 --- a/pkg/skaffold/yamltags/tags_test.go +++ b/pkg/skaffold/yamltags/tags_test.go @@ -19,6 +19,8 @@ package yamltags import ( "reflect" "testing" + + "github.com/GoogleContainerTools/skaffold/testutil" ) type otherstruct struct { @@ -217,3 +219,13 @@ func TestOneOf(t *testing.T) { }) } } + +func TestIsZeroValue(t *testing.T) { + testutil.CheckDeepEqual(t, true, isZeroValue(reflect.ValueOf(0))) + testutil.CheckDeepEqual(t, true, isZeroValue(reflect.ValueOf(nil))) + var zeroMap map[string]string + testutil.CheckDeepEqual(t, true, isZeroValue(reflect.ValueOf(zeroMap))) + + nonZeroMap := make(map[string]string) + testutil.CheckDeepEqual(t, false, isZeroValue(reflect.ValueOf(nonZeroMap))) +} From 32b14bd78c7320867b66161e455845eee9aa27df Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 10:58:40 -0700 Subject: [PATCH 07/10] removing comments, adding hybrid scenario --- hack/schemas/main.go | 32 ++-- hack/schemas/main_test.go | 1 + hack/schemas/testdata/inline-hybrid/input.go | 45 ++++++ .../testdata/inline-hybrid/output.json | 149 ++++++++++++++++++ 4 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 hack/schemas/testdata/inline-hybrid/input.go create mode 100644 hack/schemas/testdata/inline-hybrid/output.json diff --git a/hack/schemas/main.go b/hack/schemas/main.go index 522eff59c7b..cbf9de1ba02 100644 --- a/hack/schemas/main.go +++ b/hack/schemas/main.go @@ -289,41 +289,33 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { } } - // Inline anyOfs for _, k := range preferredOrder { def := definitions[k] if len(def.inlines) == 0 { continue } - //merge if not anyof - var options []*Definition - options = append(options, &Definition{ - Properties: def.Properties, - PreferredOrder: def.PreferredOrder, - AdditionalProperties: false, - }) for _, inlineStruct := range def.inlines { ref := strings.TrimPrefix(inlineStruct.Ref, defPrefix) - referenced := definitions[ref] + inlineStructRef := definitions[ref] - //if not anyof, continue - if !isOneOf(referenced) { + //if not anyof, merge & continue + if !isOneOf(inlineStructRef) { if def.Properties == nil { def.Properties = make(map[string]*Definition) } - for k, v := range referenced.Properties { + for k, v := range inlineStructRef.Properties { def.Properties[k] = v } - def.PreferredOrder = append(def.PreferredOrder, referenced.PreferredOrder...) - def.Required = append(def.Required, referenced.Required...) + def.PreferredOrder = append(def.PreferredOrder, inlineStructRef.PreferredOrder...) + def.Required = append(def.Required, inlineStructRef.Required...) continue } - for _, key := range referenced.PreferredOrder { + for _, key := range inlineStructRef.PreferredOrder { var preferredOrder []string choice := make(map[string]*Definition) @@ -335,7 +327,7 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { } preferredOrder = append(preferredOrder, key) - choice[key] = referenced.Properties[key] + choice[key] = inlineStructRef.Properties[key] options = append(options, &Definition{ Properties: choice, @@ -345,10 +337,16 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { } } - if len(options) == 1 { + if len(options) == 0 { continue } + options = append([]*Definition{{ + Properties: def.Properties, + PreferredOrder: def.PreferredOrder, + AdditionalProperties: false, + }}, options...) + def.Properties = nil def.PreferredOrder = nil def.AdditionalProperties = nil diff --git a/hack/schemas/main_test.go b/hack/schemas/main_test.go index 7457d708e41..b1334471979 100644 --- a/hack/schemas/main_test.go +++ b/hack/schemas/main_test.go @@ -42,6 +42,7 @@ func TestGenerators(t *testing.T) { }{ {name: "inline"}, {name: "inline-anyof"}, + {name: "inline-hybrid"}, } for _, tc := range tcs { diff --git a/hack/schemas/testdata/inline-hybrid/input.go b/hack/schemas/testdata/inline-hybrid/input.go new file mode 100644 index 00000000000..d7cb0cd2606 --- /dev/null +++ b/hack/schemas/testdata/inline-hybrid/input.go @@ -0,0 +1,45 @@ +/* +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 latest + +// TestStruct for testing the schema generator. +type TestStruct struct { + // RequiredField should be required + RequiredField string `yaml:"reqField" yamltags:"required"` + InlineOneOfStruct `yaml:"inline"` + InlineOneOfStructAnyOf `yaml:"inline"` +} + +//InlineOneOfStruct is embedded inline into TestStruct +type InlineOneOfStruct struct { + + //Field1 should be the first choice + Field1 string `yaml:"f1"` + + //Field2 should be the second choice + Field2 string `yaml:"f2"` +} + +//InlineOneOfStruct is embedded inline into TestStruct +type InlineOneOfStructAnyOf struct { + + //Field1 should be the first choice + Choice1 string `yaml:"choice1" yamltags:"oneOf=tag"` + + //Field2 should be the second choice + Choice2 string `yaml:"choice2" yamltags:"oneOf=tag"` +} diff --git a/hack/schemas/testdata/inline-hybrid/output.json b/hack/schemas/testdata/inline-hybrid/output.json new file mode 100644 index 00000000000..949e4a1d58f --- /dev/null +++ b/hack/schemas/testdata/inline-hybrid/output.json @@ -0,0 +1,149 @@ +{ + "type": "object", + "anyOf": [ + { + "$ref": "#/definitions/SkaffoldPipeline" + } + ], + "$schema": "http://json-schema-org/draft-07/schema#", + "definitions": { + "InlineOneOfStruct": { + "properties": { + "f1": { + "type": "string", + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" + }, + "f2": { + "type": "string", + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" + } + }, + "preferredOrder": [ + "f1", + "f2" + ], + "additionalProperties": false, + "description": "embedded inline into TestStruct", + "x-intellij-html-description": "embedded inline into TestStruct" + }, + "InlineOneOfStructAnyOf": { + "properties": { + "choice1": { + "type": "string", + "description": "Field1 should be the first choice", + "x-intellij-html-description": "Field1 should be the first choice" + }, + "choice2": { + "type": "string", + "description": "Field2 should be the second choice", + "x-intellij-html-description": "Field2 should be the second choice" + } + }, + "preferredOrder": [ + "choice1", + "choice2" + ], + "additionalProperties": false, + "description": "InlineOneOfStruct is embedded inline into TestStruct", + "x-intellij-html-description": "InlineOneOfStruct is embedded inline into TestStruct" + }, + "TestStruct": { + "required": [ + "reqField" + ], + "anyOf": [ + { + "properties": { + "f1": { + "type": "string", + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" + }, + "f2": { + "type": "string", + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" + }, + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + } + }, + "preferredOrder": [ + "reqField", + "f1", + "f2" + ], + "additionalProperties": false + }, + { + "properties": { + "choice1": { + "type": "string", + "description": "Field1 should be the first choice", + "x-intellij-html-description": "Field1 should be the first choice" + }, + "f1": { + "type": "string", + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" + }, + "f2": { + "type": "string", + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" + }, + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + } + }, + "preferredOrder": [ + "reqField", + "f1", + "f2", + "choice1" + ], + "additionalProperties": false + }, + { + "properties": { + "choice2": { + "type": "string", + "description": "Field2 should be the second choice", + "x-intellij-html-description": "Field2 should be the second choice" + }, + "f1": { + "type": "string", + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" + }, + "f2": { + "type": "string", + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" + }, + "reqField": { + "type": "string", + "description": "should be required", + "x-intellij-html-description": "should be required" + } + }, + "preferredOrder": [ + "reqField", + "f1", + "f2", + "choice2" + ], + "additionalProperties": false + } + ], + "description": "for testing the schema generator.", + "x-intellij-html-description": "for testing the schema generator." + } + } +} From fbf0d9d26b0a86054f08cf82c2ad7334edc13c1b Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 12:06:53 -0700 Subject: [PATCH 08/10] fixing naming of tags --- hack/schemas/testdata/inline-anyof/input.go | 4 ++-- hack/schemas/testdata/inline-hybrid/input.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hack/schemas/testdata/inline-anyof/input.go b/hack/schemas/testdata/inline-anyof/input.go index 6704e87afa8..a2f05d25694 100644 --- a/hack/schemas/testdata/inline-anyof/input.go +++ b/hack/schemas/testdata/inline-anyof/input.go @@ -27,8 +27,8 @@ type TestStruct struct { type InlineOneOfStruct struct { //Field1 should be the first choice - Field1 string `yaml:"field1" yamltags:"oneOf=tag"` + Field1 string `yaml:"field1" yamltags:"oneOf=fooBar"` //Field2 should be the second choice - Field2 string `yaml:"field2" yamltags:"oneOf=tag"` + Field2 string `yaml:"field2" yamltags:"oneOf=fooBar"` } diff --git a/hack/schemas/testdata/inline-hybrid/input.go b/hack/schemas/testdata/inline-hybrid/input.go index d7cb0cd2606..e6f91d28370 100644 --- a/hack/schemas/testdata/inline-hybrid/input.go +++ b/hack/schemas/testdata/inline-hybrid/input.go @@ -38,8 +38,8 @@ type InlineOneOfStruct struct { type InlineOneOfStructAnyOf struct { //Field1 should be the first choice - Choice1 string `yaml:"choice1" yamltags:"oneOf=tag"` + Choice1 string `yaml:"choice1" yamltags:"oneOf=fooBar"` //Field2 should be the second choice - Choice2 string `yaml:"choice2" yamltags:"oneOf=tag"` + Choice2 string `yaml:"choice2" yamltags:"oneOf=fooBar"` } From 7f857fbc982c0275faa98bf12e151abef6a46385 Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 12:48:30 -0700 Subject: [PATCH 09/10] fixing some comments --- hack/schemas/main_test.go | 7 ++++++- hack/schemas/testdata/inline-hybrid/input.go | 6 +++--- .../testdata/inline-hybrid/output.json | 20 +++++++++---------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/hack/schemas/main_test.go b/hack/schemas/main_test.go index b1334471979..20664e45a5b 100644 --- a/hack/schemas/main_test.go +++ b/hack/schemas/main_test.go @@ -22,6 +22,8 @@ import ( "os" "testing" + "github.com/google/go-cmp/cmp" + "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -64,7 +66,10 @@ func TestGenerators(t *testing.T) { testutil.CheckError(t, false, err) } - testutil.CheckDeepEqual(t, string(expected), string(actual)) + if diff := cmp.Diff(string(actual), string(expected)); diff != "" { + t.Errorf("%T differ (-got, +want): %s\n actual:\n%s", string(expected), diff, string(actual)) + return + } }) } } diff --git a/hack/schemas/testdata/inline-hybrid/input.go b/hack/schemas/testdata/inline-hybrid/input.go index e6f91d28370..9afaea56dd0 100644 --- a/hack/schemas/testdata/inline-hybrid/input.go +++ b/hack/schemas/testdata/inline-hybrid/input.go @@ -34,12 +34,12 @@ type InlineOneOfStruct struct { Field2 string `yaml:"f2"` } -//InlineOneOfStruct is embedded inline into TestStruct +//InlineOneOfStructAnyOf is embedded inline into TestStruct type InlineOneOfStructAnyOf struct { - //Field1 should be the first choice + //Choice1 should be the first choice Choice1 string `yaml:"choice1" yamltags:"oneOf=fooBar"` - //Field2 should be the second choice + //Choice2 should be the second choice Choice2 string `yaml:"choice2" yamltags:"oneOf=fooBar"` } diff --git a/hack/schemas/testdata/inline-hybrid/output.json b/hack/schemas/testdata/inline-hybrid/output.json index 949e4a1d58f..a94fea83b2a 100644 --- a/hack/schemas/testdata/inline-hybrid/output.json +++ b/hack/schemas/testdata/inline-hybrid/output.json @@ -32,13 +32,13 @@ "properties": { "choice1": { "type": "string", - "description": "Field1 should be the first choice", - "x-intellij-html-description": "Field1 should be the first choice" + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" }, "choice2": { "type": "string", - "description": "Field2 should be the second choice", - "x-intellij-html-description": "Field2 should be the second choice" + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" } }, "preferredOrder": [ @@ -46,8 +46,8 @@ "choice2" ], "additionalProperties": false, - "description": "InlineOneOfStruct is embedded inline into TestStruct", - "x-intellij-html-description": "InlineOneOfStruct is embedded inline into TestStruct" + "description": "embedded inline into TestStruct", + "x-intellij-html-description": "embedded inline into TestStruct" }, "TestStruct": { "required": [ @@ -83,8 +83,8 @@ "properties": { "choice1": { "type": "string", - "description": "Field1 should be the first choice", - "x-intellij-html-description": "Field1 should be the first choice" + "description": "should be the first choice", + "x-intellij-html-description": "should be the first choice" }, "f1": { "type": "string", @@ -114,8 +114,8 @@ "properties": { "choice2": { "type": "string", - "description": "Field2 should be the second choice", - "x-intellij-html-description": "Field2 should be the second choice" + "description": "should be the second choice", + "x-intellij-html-description": "should be the second choice" }, "f1": { "type": "string", From 670b22f08e302311c369c1fa5e8bebeadbd2e234 Mon Sep 17 00:00:00 2001 From: balopat Date: Tue, 2 Apr 2019 13:02:50 -0700 Subject: [PATCH 10/10] nick's comments --- hack/schemas/main.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hack/schemas/main.go b/hack/schemas/main.go index cbf9de1ba02..f56c91f4551 100644 --- a/hack/schemas/main.go +++ b/hack/schemas/main.go @@ -298,14 +298,13 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) { var options []*Definition for _, inlineStruct := range def.inlines { - ref := strings.TrimPrefix(inlineStruct.Ref, defPrefix) inlineStructRef := definitions[ref] - //if not anyof, merge & continue + // if not anyof, merge & continue if !isOneOf(inlineStructRef) { if def.Properties == nil { - def.Properties = make(map[string]*Definition) + def.Properties = make(map[string]*Definition, len(inlineStructRef.Properties)) } for k, v := range inlineStructRef.Properties { def.Properties[k] = v