Validate generated schemas in generator script (#6385)

* fix build --push=false for missing kubeconfig * Fixed log ordering per PR comment * Added validation to schema generation * Added validation to schema generation * Updated validate function * Refactor validate method * Updated error logic * Fix linter error
GoogleContainerTools · Aug 9, 2021 · 005dc31 · 005dc31
1 parent 1734f56
commit 005dc31
Show file tree

Hide file tree

Showing 7 changed files with 233 additions and 7 deletions.
diff --git a/docs/content/en/schemas/v2beta19.json b/docs/content/en/schemas/v2beta19.json
@@ -914,6 +914,29 @@
       "description": "describes a dependency on another skaffold configuration.",
       "x-intellij-html-description": "describes a dependency on another skaffold configuration."
     },
+    "ContainerHook": {
+      "required": [
+        "command"
+      ],
+      "properties": {
+        "command": {
+          "items": {
+            "type": "string"
+          },
+          "type": "array",
+          "description": "command to execute.",
+          "x-intellij-html-description": "command to execute.",
+          "default": "[]"
+        }
+      },
+      "preferredOrder": [
+        "command"
+      ],
+      "additionalProperties": false,
+      "type": "object",
+      "description": "describes a lifecycle hook definition to execute on a container. The container name is inferred from the scope in which this hook is defined.",
+      "x-intellij-html-description": "describes a lifecycle hook definition to execute on a container. The container name is inferred from the scope in which this hook is defined."
+    },
     "CustomArtifact": {
       "properties": {
         "buildCommand": {

diff --git a/hack/schemas/main.go b/hack/schemas/main.go
@@ -32,6 +32,7 @@ import (
 	"sync"
 
 	blackfriday "github.com/russross/blackfriday/v2"
+	"github.com/xeipuuv/gojsonschema"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
 )
@@ -474,7 +475,23 @@ func (g *schemaGenerator) Apply(inputPath string) ([]byte, error) {
 		Definitions: definitions,
 	}
 
-	return toJSON(schema)
+	buf, err := toJSON(schema)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := validate(buf); err != nil {
+		return nil, fmt.Errorf("invalid schema generated: %v", err.Error())
+	}
+
+	return buf, nil
+}
+
+// Validate generated schema
+func validate(data []byte) error {
+	schemaLoader := gojsonschema.NewBytesLoader(data)
+	_, err := gojsonschema.NewSchema(schemaLoader)
+	return err
 }
 
 // Make sure HTML description are not encoded

diff --git a/hack/schemas/main_test.go b/hack/schemas/main_test.go
@@ -23,7 +23,6 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/xeipuuv/gojsonschema"
 
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
@@ -46,6 +45,7 @@ func TestGenerators(t *testing.T) {
 		{name: "inline"},
 		{name: "inline-anyof"},
 		{name: "inline-hybrid"},
+		{name: "inline-skiptrim"},
 		{name: "integer"},
 	}
 	for _, test := range tests {
@@ -65,14 +65,35 @@ func TestGenerators(t *testing.T) {
 
 			expected = bytes.ReplaceAll(expected, []byte("\r\n"), []byte("\n"))
 
-			schemaLoader := gojsonschema.NewBytesLoader(actual)
-			_, err = gojsonschema.NewSchema(schemaLoader)
-			t.CheckNoError(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
 			}
 		})
 	}
 }
+
+func TestGeneratorErrors(t *testing.T) {
+	tests := []struct {
+		name          string
+		shouldErr     bool
+		expectedError string
+	}{
+		{name: "invalid-schema", shouldErr: true, expectedError: "Object has no key 'InlineStruct'"},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.name, func(t *testutil.T) {
+			input := filepath.Join("testdata", test.name, "input.go")
+
+			generator := schemaGenerator{
+				strict: false,
+			}
+
+			_, err := generator.Apply(input)
+			t.CheckError(test.shouldErr, err)
+			if test.expectedError != "" {
+				t.CheckErrorContains(test.expectedError, err)
+			}
+		})
+	}
+}
diff --git a/hack/schemas/testdata/inline-skiptrim/input.go b/hack/schemas/testdata/inline-skiptrim/input.go
@@ -0,0 +1,42 @@
+/*
+Copyright 2021 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"`
+
+	// AnotherField has reference to InlineStruct
+	AnotherField *InlineStruct `yaml:"anotherField"`
+}
+
+// AnotherTestStruct for testing the schema s generator.
+type AnotherTestStruct struct {
+	InlineStruct `yaml:"inline" yamltags:"skipTrim"`
+}
+
+// InlineStruct is embedded inline into TestStruct
+type InlineStruct struct {
+
+	// Field1 should be the first choice
+	Field1 string `yaml:"f1"`
+
+	// Field2 should be the second choice
+	Field2 string `yaml:"f2"`
+}
diff --git a/hack/schemas/testdata/inline-skiptrim/output.json b/hack/schemas/testdata/inline-skiptrim/output.json
@@ -0,0 +1,79 @@
+{
+  "type": "object",
+  "anyOf": [
+    {
+      "$ref": "#/definitions/TestStruct"
+    }
+  ],
+  "$schema": "http://json-schema-org/draft-07/schema#",
+  "definitions": {
+    "AnotherTestStruct": {
+      "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"
+      ],
+      "type": "object",
+      "description": "for testing the schema s generator.",
+      "x-intellij-html-description": "for testing the schema s generator."
+    },
+    "InlineStruct": {
+      "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,
+      "type": "object",
+      "description": "embedded inline into TestStruct",
+      "x-intellij-html-description": "embedded inline into TestStruct"
+    },
+    "TestStruct": {
+      "required": [
+        "reqField"
+      ],
+      "properties": {
+        "anotherField": {
+          "$ref": "#/definitions/InlineStruct",
+          "description": "has reference to InlineStruct",
+          "x-intellij-html-description": "has reference to InlineStruct"
+        },
+        "reqField": {
+          "type": "string",
+          "description": "should be required",
+          "x-intellij-html-description": "should be required"
+        }
+      },
+      "preferredOrder": [
+        "reqField",
+        "anotherField"
+      ],
+      "additionalProperties": false,
+      "type": "object",
+      "description": "for testing the schema generator.",
+      "x-intellij-html-description": "for testing the schema generator."
+    }
+  }
+}
diff --git a/hack/schemas/testdata/invalid-schema/input.go b/hack/schemas/testdata/invalid-schema/input.go
@@ -0,0 +1,44 @@
+/*
+Copyright 2021 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"`
+
+	// AnotherField has reference to InlineStruct
+	AnotherField *InlineStruct `yaml:"anotherField"`
+}
+
+// AnotherTestStruct for testing the schema s generator.
+type AnotherTestStruct struct {
+	// Not adding yamltags:"skipTrim" causes InlineStruct to be removed from definitions
+	// This breaks the TestStruct reference above
+	InlineStruct `yaml:"inline"`
+}
+
+// InlineStruct is embedded inline into TestStruct
+type InlineStruct struct {
+
+	// Field1 should be the first choice
+	Field1 string `yaml:"f1"`
+
+	// Field2 should be the second choice
+	Field2 string `yaml:"f2"`
+}
diff --git a/pkg/skaffold/schema/v2beta19/config.go b/pkg/skaffold/schema/v2beta19/config.go
@@ -1381,7 +1381,7 @@ type ContainerHook struct {
 // NamedContainerHook describes a lifecycle hook definition to execute on a named container.
 type NamedContainerHook struct {
 	// ContainerHook describes a lifecycle hook definition to execute on a container.
-	ContainerHook `yaml:",inline"`
+	ContainerHook `yaml:",inline" yamltags:"skipTrim"`
 	// PodName is the name of the pod to execute the command in.
 	PodName string `yaml:"podName" yamltags:"required"`
 	// ContainerName is the name of the container to execute the command in.