Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(*): replace jsonschema validation with kube-openapi #5166

Merged
merged 6 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/kumactl/cmd/apply/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ spec:
default:
action: foo
`,
err: `YAML contains invalid resource: from.0.default.action: from.0.default.action must be one of the following: "ALLOW", "DENY", "ALLOW_WITH_SHADOW_DENY", "DENY_WITH_SHADOW_ALLOW"`,
err: `YAML contains invalid resource: spec: from[0].default.action in body should be one of [ALLOW DENY ALLOW_WITH_SHADOW_DENY DENY_WITH_SHADOW_ALLOW]`,
}),
)
})
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ require (
github.com/spf13/viper v1.13.0
github.com/spiffe/go-spiffe v0.0.0-20190820222348-6adcf1eecbcc
github.com/testcontainers/testcontainers-go v0.14.0
github.com/xeipuuv/gojsonschema v1.2.0
go.uber.org/multierr v1.8.0
go.uber.org/zap v1.23.0
golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4
Expand All @@ -69,6 +68,7 @@ require (
k8s.io/apiextensions-apiserver v0.25.3
k8s.io/apimachinery v0.25.3
k8s.io/client-go v0.25.3
k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1
k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed
sigs.k8s.io/controller-runtime v0.13.0
sigs.k8s.io/controller-tools v0.10.0
Expand Down Expand Up @@ -179,6 +179,7 @@ require (
github.com/vishvananda/netns v0.0.0-20211101163701-50045581ed74 // indirect
github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
github.com/xeipuuv/gojsonschema v1.2.0 // indirect
go.opencensus.io v0.23.0 // indirect
go.uber.org/atomic v1.10.0 // indirect
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa // indirect
Expand All @@ -194,7 +195,6 @@ require (
gotest.tools/v3 v3.3.0 // indirect
k8s.io/component-base v0.25.3 // indirect
k8s.io/klog/v2 v2.80.0 // indirect
k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,21 @@ import (
_ "embed"
"fmt"

"github.com/xeipuuv/gojsonschema"
"github.com/ghodss/yaml"
"k8s.io/kube-openapi/pkg/validation/spec"

"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/plugins/policies/validation"
)

//go:embed schema.yaml
var rawSchema []byte
var schema *gojsonschema.JSONLoader
var schema = &spec.Schema{}

func init() {
sch, err := validation.YamlToJsonSchemaLoader(rawSchema)
if err != nil {
if err := yaml.Unmarshal(rawSchema, schema); err != nil {
panic(err)
}
schema = sch
}

const (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,21 @@ import (
_ "embed"
"fmt"

"github.com/xeipuuv/gojsonschema"
"github.com/ghodss/yaml"
"k8s.io/kube-openapi/pkg/validation/spec"

"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/plugins/policies/validation"
)

//go:embed schema.yaml
var rawSchema []byte
var schema *gojsonschema.JSONLoader
var schema = &spec.Schema{}

func init() {
sch, err := validation.YamlToJsonSchemaLoader(rawSchema)
if err != nil {
if err := yaml.Unmarshal(rawSchema, schema); err != nil {
panic(err)
}
schema = sch
}

const (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,21 @@ import (
_ "embed"
"fmt"

"github.com/xeipuuv/gojsonschema"
"github.com/ghodss/yaml"
"k8s.io/kube-openapi/pkg/validation/spec"

"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/plugins/policies/validation"
)

//go:embed schema.yaml
var rawSchema []byte
var schema *gojsonschema.JSONLoader
var schema = &spec.Schema{}

func init() {
sch, err := validation.YamlToJsonSchemaLoader(rawSchema)
if err != nil {
if err := yaml.Unmarshal(rawSchema, schema); err != nil {
panic(err)
}
schema = sch
}

const (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,21 @@ import (
_ "embed"
"fmt"

"github.com/xeipuuv/gojsonschema"
"github.com/ghodss/yaml"
"k8s.io/kube-openapi/pkg/validation/spec"

"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/plugins/policies/validation"
)

//go:embed schema.yaml
var rawSchema []byte
var schema *gojsonschema.JSONLoader
var schema = &spec.Schema{}

func init() {
sch, err := validation.YamlToJsonSchemaLoader(rawSchema)
if err != nil {
if err := yaml.Unmarshal(rawSchema, schema); err != nil {
panic(err)
}
schema = sch
}

const (
Expand Down
46 changes: 12 additions & 34 deletions pkg/plugins/policies/validation/schema.go
Original file line number Diff line number Diff line change
@@ -1,50 +1,28 @@
package validation

import (
"github.com/ghodss/yaml"
"github.com/xeipuuv/gojsonschema"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
"k8s.io/kube-openapi/pkg/validation/spec"
"k8s.io/kube-openapi/pkg/validation/strfmt"
"k8s.io/kube-openapi/pkg/validation/validate"

core_model "github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/core/validators"
)

func ValidateSchema(message proto.Message, schema *gojsonschema.JSONLoader) error {
json, err := protojson.Marshal(message)
if err != nil {
return err
}

documentLoader := gojsonschema.NewBytesLoader(json)
result, err := gojsonschema.Validate(*schema, documentLoader)
func ValidateSchema(rspec core_model.ResourceSpec, schema *spec.Schema) error {
var rootSchema *spec.Schema = nil
var root = ""
validator := validate.NewSchemaValidator(schema, rootSchema, root, strfmt.Default)

if err != nil {
return err
}

if result.Valid() {
res := validator.Validate(rspec)
if res.IsValid() {
return nil
} else {
return mapSchemaToValidatorErrors(result.Errors())
}
}

func mapSchemaToValidatorErrors(errors []gojsonschema.ResultError) error {
var verr validators.ValidationError

for _, err := range errors {
verr.AddViolation(err.Field(), err.Description())
for _, err := range res.Errors {
verr.AddViolation("spec", err.Error())
}

return &verr
}

func YamlToJsonSchemaLoader(rawSchema []byte) (*gojsonschema.JSONLoader, error) {
json, err := yaml.YAMLToJSON(rawSchema)
if err != nil {
return nil, err
}
loader := gojsonschema.NewStringLoader(string(json))

return &loader, nil
}
49 changes: 34 additions & 15 deletions pkg/plugins/policies/validation/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,60 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

meshtrace_proto "github.com/kumahq/kuma/pkg/plugins/policies/meshtrafficpermission/api/v1alpha1"
core_model "github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/core/resources/registry"
_ "github.com/kumahq/kuma/pkg/plugins/policies"
meshtrace_proto "github.com/kumahq/kuma/pkg/plugins/policies/meshtrace/api/v1alpha1"
meshtrafficpermissions_proto "github.com/kumahq/kuma/pkg/plugins/policies/meshtrafficpermission/api/v1alpha1"
"github.com/kumahq/kuma/pkg/util/proto"
)

var _ = Describe("plugins validation", func() {
Describe("ValidateResourceSchema()", func() {
type testCase struct {
inputYaml string
resourceType core_model.ResourceType
expected string
}
DescribeTable("valid MeshTrafficPermission should pass validation",
func(resourceYaml string) {
mtp := meshtrace_proto.NewMeshTrafficPermissionResource()
err := proto.FromYAML([]byte(resourceYaml), mtp.Spec)
func(given testCase) {
obj, err := registry.Global().NewObject(given.resourceType)
Expect(err).To(Not(HaveOccurred()))
verr := mtp.Validate()

err = proto.FromYAML([]byte(given.inputYaml), obj.GetSpec())
Expect(err).To(Not(HaveOccurred()))

verr := obj.(core_model.ResourceValidator).Validate()
Expect(verr).To(BeNil())
},
Entry("valid MeshTrafficPermission", `
Entry("valid MeshTrafficPermission", testCase{
resourceType: meshtrafficpermissions_proto.MeshTrafficPermissionType,
inputYaml: `
targetRef:
kind: Mesh
from:
- targetRef:
kind: Mesh
default:
action: ALLOW
`),
`,
}),
Entry("valid MeshTrace", testCase{
resourceType: meshtrace_proto.MeshTraceType,
inputYaml: `
targetRef:
kind: Mesh
default:
backends: []
tags: null
`,
}),
)

type testCase struct {
inputYaml string
expected string
}

DescribeTable("should validate schema and return as accurate errors as possible",
func(given testCase) {
// and
mtp := meshtrace_proto.NewMeshTrafficPermissionResource()
mtp := meshtrafficpermissions_proto.NewMeshTrafficPermissionResource()
err := proto.FromYAML([]byte(given.inputYaml), mtp.Spec)
Expect(err).To(Not(HaveOccurred()))
verr := mtp.Validate()
Expand All @@ -61,8 +80,8 @@ from:
`,
expected: `
violations:
- field: from.0.default.action
message: 'from.0.default.action must be one of the following: "ALLOW", "DENY", "ALLOW_WITH_SHADOW_DENY", "DENY_WITH_SHADOW_ALLOW"'`,
- field: spec
message: from[0].default.action in body should be one of [ALLOW DENY ALLOW_WITH_SHADOW_DENY DENY_WITH_SHADOW_ALLOW]`,
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved
}),
)
})
Expand Down
9 changes: 4 additions & 5 deletions tools/policy-gen/protoc-gen-kumapolicy/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,21 @@ import (
_ "embed"
"fmt"

"github.com/xeipuuv/gojsonschema"
"github.com/ghodss/yaml"
"k8s.io/kube-openapi/pkg/validation/spec"

"github.com/kumahq/kuma/pkg/core/resources/model"
"github.com/kumahq/kuma/pkg/plugins/policies/validation"
)

//_DELETE_GO_EMBED_WORKAROUND_go:embed schema.yaml
var rawSchema []byte
var schema *gojsonschema.JSONLoader
var schema = &spec.Schema{}

func init() {
sch, err := validation.YamlToJsonSchemaLoader(rawSchema)
if err != nil {
if err := yaml.Unmarshal(rawSchema, schema); err != nil {
panic(err)
}
schema = sch
}

{{range .Resources}}
Expand Down