fixing non-oneof inline struct handling in schemas

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,22 +177,22 @@ 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 = "[]"
 		}
 
 	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,29 +285,36 @@ 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
 		}
 
 		var options []*Definition
-		options = append(options, &Definition{
-			Properties:           def.Properties,
-			PreferredOrder:       def.PreferredOrder,
-			AdditionalProperties: false,
-		})
 
-		for _, anyOf := range def.AnyOf {
-			ref := strings.TrimPrefix(anyOf.Ref, defPrefix)
-			referenced := definitions[ref]
+		for _, inlineStruct := range def.inlines {
+			ref := strings.TrimPrefix(inlineStruct.Ref, defPrefix)
+			inlineStructRef := definitions[ref]
 
-			for _, key := range referenced.PreferredOrder {
+			// if not anyof, merge & continue
+			if !isOneOf(inlineStructRef) {
+				if def.Properties == nil {
+					def.Properties = make(map[string]*Definition, len(inlineStructRef.Properties))
+				}
+				for k, v := range inlineStructRef.Properties {
+					def.Properties[k] = v
+				}
+				def.PreferredOrder = append(def.PreferredOrder, inlineStructRef.PreferredOrder...)
+				def.Required = append(def.Required, inlineStructRef.Required...)
+				continue
+			}
+
+			for _, key := range inlineStructRef.PreferredOrder {
 				var preferredOrder []string
 				choice := make(map[string]*Definition)
 
@@ -309,7 +326,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,
@@ -319,6 +336,16 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) {
 			}
 		}
 
+		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

hack/schemas/main_test.go

@@ -17,7 +17,14 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
+	"io/ioutil"
+	"os"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+
+	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 func TestSchemas(t *testing.T) {
@@ -30,3 +37,39 @@ 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"},
+		{name: "inline-hybrid"},
+	}
+
+	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)
+			}
+
+			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
+			}
+		})
+	}
+}

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=fooBar"`
+
+	//Field2 should be the second choice
+	Field2 string `yaml:"field2" yamltags:"oneOf=fooBar"`
+}

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."
+    }
+  }
+}

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"`
+}
+
+//InlineOneOfStructAnyOf is embedded inline into TestStruct
+type InlineOneOfStructAnyOf struct {
+
+	//Choice1 should be the first choice
+	Choice1 string `yaml:"choice1" yamltags:"oneOf=fooBar"`
+
+	//Choice2 should be the second choice
+	Choice2 string `yaml:"choice2" yamltags:"oneOf=fooBar"`
+}