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

chore(bootstrap): improve validator policy bootstrap #5014

Merged
merged 2 commits into from
Sep 20, 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 mk/docs.mk
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ docs/install/markdown: ## Generate CLI reference in markdown format
.PHONY: docs/install/manpages
docs/install/manpages: DESTDIR:=$(DESTDIR)/manpages
docs/install/manpages: ## Generate CLI reference in man(1) format
@DESTDIR=$(DESTDIR) FORMAT=man $(GO_RUN) $(TOOLS_DIR)/tools/docs/generate.go
@DESTDIR=$(DESTDIR) FORMAT=man $(GO_RUN) $(TOOLS_DIR)/docs/generate.go

.PHONY: docs/install/resources
docs/install/resources: DESTDIR:=$(DESTDIR)/resources
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,23 @@ message DoNothingPolicy {
message To {
// TargetRef is a reference to the resource that represents a group of
// destinations.
kuma.common.v1alpha1.TargetRef targetRef = 1;
kuma.common.v1alpha1.TargetRef targetRef = 1 [ (doc.required) = true ];

// Default is a configuration specific to the group of destinations
// referenced in 'targetRef'
Conf default = 2;
Conf default = 2 [ (doc.required) = true ];
}

repeated To to = 2;

message From {
// TargetRef is a reference to the resource that represents a group of
// clients.
kuma.common.v1alpha1.TargetRef targetRef = 1;
kuma.common.v1alpha1.TargetRef targetRef = 1 [ (doc.required) = true ];
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved

// Default is a configuration specific to the group of clients referenced in
// 'targetRef'
Conf default = 2;
Conf default = 2 [ (doc.required) = true ];
}

repeated From from = 3;
Expand Down
90 changes: 48 additions & 42 deletions pkg/plugins/policies/donothingpolicy/api/v1alpha1/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,57 +9,63 @@ import (
func (r *DoNothingPolicyResource) validate() error {
var verr validators.ValidationError
path := validators.RootedAt("spec")

targetRefErr := matcher_validators.ValidateTargetRef(path.Field("targetRef"), r.Spec.GetTargetRef(), &matcher_validators.ValidateTargetRefOpts{
verr.AddErrorAt(path.Field("targetRef"), validateTop(r.Spec.GetTargetRef()))
if len(r.Spec.GetTo()) == 0 && len(r.Spec.GetFrom()) == 0 {
verr.AddViolationAt(path, "at least one of 'from', 'to' has to be defined")
}
verr.AddErrorAt(path, validateTo(r.Spec.GetTo()))
verr.AddErrorAt(path, validateFrom(r.Spec.GetFrom()))
return verr.OrNil()
}
func validateTop(targetRef *common_proto.TargetRef) validators.ValidationError {
targetRefErr := matcher_validators.ValidateTargetRef(targetRef, &matcher_validators.ValidateTargetRefOpts{
SupportedKinds: []common_proto.TargetRef_Kind{
// TODO add supported TargetRef kinds for this policy
},
})
verr.AddError("", targetRefErr)

from := path.Field("from")
if len(r.Spec.GetFrom()) == 0 {
verr.AddViolationAt(from, "cannot be empty")
} else {
for idx, fromItem := range r.Spec.GetFrom() {
targetRefErr := matcher_validators.ValidateTargetRef(from.Index(idx).Field("targetRef"), fromItem.GetTargetRef(), &matcher_validators.ValidateTargetRefOpts{
SupportedKinds: []common_proto.TargetRef_Kind{
// TODO add supported TargetRef for 'from' item
},
})
verr.AddError("", targetRefErr)
return targetRefErr
}
func validateFrom(from []*DoNothingPolicy_From) validators.ValidationError {
var verr validators.ValidationError
for idx, fromItem := range from {
path := validators.RootedAt("from").Index(idx)
verr.AddErrorAt(path.Field("targetRef"), matcher_validators.ValidateTargetRef(fromItem.GetTargetRef(), &matcher_validators.ValidateTargetRefOpts{
SupportedKinds: []common_proto.TargetRef_Kind{
// TODO add supported TargetRef for 'from' item
},
}))

defaultField := from.Index(idx).Field("default")
if fromItem.GetDefault() == nil {
verr.AddViolationAt(defaultField, "cannot be nil")
} else {
// TODO add default conf validation
verr.AddViolationAt(defaultField, "")
}
defaultField := path.Field("default")
if fromItem.GetDefault() == nil {
verr.AddViolationAt(defaultField, "must be defined")
} else {
verr.AddErrorAt(defaultField, validateDefault(fromItem.Default))
}
}
return verr
}
func validateTo(to []*DoNothingPolicy_To) validators.ValidationError {
var verr validators.ValidationError
for idx, toItem := range to {
path := validators.RootedAt("to").Index(idx)
verr.AddErrorAt(path.Field("targetRef"), matcher_validators.ValidateTargetRef(toItem.GetTargetRef(), &matcher_validators.ValidateTargetRefOpts{
SupportedKinds: []common_proto.TargetRef_Kind{
// TODO add supported TargetRef for 'to' item
},
}))

to := path.Field("to")
if len(r.Spec.GetTo()) == 0 {
verr.AddViolationAt(to, "cannot be empty")
} else {
for idx, toItem := range r.Spec.GetTo() {
targetRefErr := matcher_validators.ValidateTargetRef(from.Index(idx).Field("targetRef"), toItem.GetTargetRef(), &matcher_validators.ValidateTargetRefOpts{
SupportedKinds: []common_proto.TargetRef_Kind{
// TODO add supported TargetRef for 'to' item
},
})
verr.AddError("", targetRefErr)

defaultField := to.Index(idx).Field("default")
if toItem.GetDefault() == nil {
verr.AddViolationAt(defaultField, "cannot be nil")
} else {
// TODO add default conf validation
verr.AddViolationAt(defaultField, "")
}
defaultField := path.Field("default")
if toItem.GetDefault() == nil {
verr.AddViolationAt(defaultField, "must be defined")
} else {
verr.AddErrorAt(defaultField, validateDefault(toItem.Default))
}
}
return verr
}

return verr.OrNil()
func validateDefault(conf *DoNothingPolicy_Conf) validators.ValidationError {
var verr validators.ValidationError
// TODO add default conf validation
return verr
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1alpha1

import (
"github.com/kumahq/kuma/pkg/core"
core_plugins "github.com/kumahq/kuma/pkg/core/plugins"
core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
core_xds "github.com/kumahq/kuma/pkg/core/xds"
Expand All @@ -10,6 +11,7 @@ import (
)

var _ core_plugins.PolicyPlugin = &plugin{}
var log = core.Log.WithName("DoNothingPolicy")

type plugin struct {
}
Expand All @@ -23,5 +25,6 @@ func (p plugin) MatchedPolicies(dataplane *core_mesh.DataplaneResource, resource
}

func (p plugin) Apply(rs *core_xds.ResourceSet, ctx xds_context.Context, proxy *core_xds.Proxy) error {
panic("implement me")
log.Info("apply is not implemented")
return nil
}
27 changes: 13 additions & 14 deletions pkg/plugins/policies/matchers/validators/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ type ValidateTargetRefOpts struct {
}

func ValidateTargetRef(
path validators.PathBuilder,
ref *common_proto.TargetRef,
opts *ValidateTargetRefOpts,
) validators.ValidationError {
Expand All @@ -22,50 +21,50 @@ func ValidateTargetRef(
return verr
}
if !contains(opts.SupportedKinds, ref.GetKindEnum()) {
verr.AddViolationAt(path.Field("kind"), "value is not supported")
verr.AddViolation("kind", "value is not supported")
} else {
switch ref.GetKindEnum() {
case common_proto.TargetRef_Mesh:
if len(ref.Tags) != 0 {
verr.AddViolationAt(path.Field("tags"), fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_Mesh))
verr.AddViolation("tags", fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_Mesh))
}
if len(ref.Mesh) != 0 {
verr.AddViolationAt(path.Field("mesh"), fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_Mesh))
verr.AddViolation("mesh", fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_Mesh))
}
case common_proto.TargetRef_MeshSubset:
if len(ref.Mesh) != 0 {
verr.AddViolationAt(path.Field("mesh"), fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshSubset))
verr.AddViolation("mesh", fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshSubset))
}
if ref.Tags != nil && len(ref.Tags) == 0 {
verr.AddViolationAt(path.Field("tags"), "cannot be empty")
verr.AddViolation("tags", "cannot be empty")
}
case common_proto.TargetRef_MeshService:
if len(ref.Tags) != 0 {
verr.AddViolationAt(path.Field("tags"), fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshService))
verr.AddViolation("tags", fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshService))
}
if len(ref.Name) == 0 {
verr.AddViolationAt(path.Field("name"), "cannot be empty")
verr.AddViolation("name", "cannot be empty")
}
case common_proto.TargetRef_MeshServiceSubset:
if len(ref.Name) == 0 {
verr.AddViolationAt(path.Field("name"), "cannot be empty")
verr.AddViolation("name", "cannot be empty")
}
if ref.Tags != nil && len(ref.Tags) == 0 {
verr.AddViolationAt(path.Field("tags"), "cannot be empty")
verr.AddViolation("tags", "cannot be empty")
}
case common_proto.TargetRef_MeshGatewayRoute:
if len(ref.Name) == 0 {
verr.AddViolationAt(path.Field("name"), "cannot be empty")
verr.AddViolation("name", "cannot be empty")
}
if len(ref.Mesh) != 0 {
verr.AddViolationAt(path.Field("mesh"), fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshGatewayRoute))
verr.AddViolation("mesh", fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshGatewayRoute))
}
case common_proto.TargetRef_MeshHTTPRoute:
if len(ref.Name) == 0 {
verr.AddViolationAt(path.Field("name"), "cannot be empty")
verr.AddViolation("name", "cannot be empty")
}
if len(ref.Mesh) != 0 {
verr.AddViolationAt(path.Field("mesh"), fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshHTTPRoute))
verr.AddViolation("mesh", fmt.Sprintf("could not be set with kind %v", common_proto.TargetRef_MeshHTTPRoute))
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/plugins/policies/matchers/validators/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ var _ = Describe("TargetRef Validator", func() {
Expect(err).ToNot(HaveOccurred())

// when
validationErr := matcher_validators.ValidateTargetRef(validators.RootedAt("targetRef"), targetRef, given.opts)
validationErr := validators.ValidationError{}
validationErr.AddError("targetRef", matcher_validators.ValidateTargetRef(targetRef, given.opts))

// then
Expect(validationErr.OrNil()).To(BeNil())
Expand Down Expand Up @@ -181,7 +182,8 @@ name: backend-http-route
Expect(err).ToNot(HaveOccurred())

// when
validationErr := matcher_validators.ValidateTargetRef(validators.RootedAt("targetRef"), targetRef, given.opts)
validationErr := validators.ValidationError{}
validationErr.AddError("targetRef", matcher_validators.ValidateTargetRef(targetRef, given.opts))
// and
actual, err := yaml.Marshal(validationErr)

Expand Down
Loading