Skip to content

Commit

Permalink
mesh: add validation for the new pbmesh resources
Browse files Browse the repository at this point in the history
  • Loading branch information
rboyer committed Aug 11, 2023
1 parent 66bcaa3 commit c4ed456
Show file tree
Hide file tree
Showing 15 changed files with 2,223 additions and 11 deletions.
47 changes: 46 additions & 1 deletion internal/mesh/internal/types/computed_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package types

import (
"github.com/hashicorp/go-multierror"

"github.com/hashicorp/consul/internal/resource"
pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1"
"github.com/hashicorp/consul/proto-public/pbresource"
Expand All @@ -27,6 +29,49 @@ func RegisterComputedRoutes(r resource.Registry) {
r.Register(resource.Registration{
Type: ComputedRoutesV1Alpha1Type,
Proto: &pbmesh.ComputedRoutes{},
Validate: nil,
Validate: ValidateComputedRoutes,
})
}

func ValidateComputedRoutes(res *pbresource.Resource) error {
var config pbmesh.ComputedRoutes

if err := res.Data.UnmarshalTo(&config); err != nil {
return resource.NewErrDataParse(&config, err)
}

var merr error

if len(config.PortedConfigs) == 0 {
merr = multierror.Append(merr, resource.ErrInvalidField{
Name: "ported_configs",
Wrapped: resource.ErrEmpty,
})
}

// TODO(rb): do more elaborate validation

for port, pmc := range config.PortedConfigs {
wrapErr := func(err error) error {
return resource.ErrInvalidMapValue{
Map: "ported_configs",
Key: port,
Wrapped: err,
}
}
if pmc.Config == nil {
merr = multierror.Append(merr, wrapErr(resource.ErrInvalidField{
Name: "config",
Wrapped: resource.ErrEmpty,
}))
}
if len(pmc.Targets) == 0 {
merr = multierror.Append(merr, wrapErr(resource.ErrInvalidField{
Name: "targets",
Wrapped: resource.ErrEmpty,
}))
}
}

return merr
}
79 changes: 79 additions & 0 deletions internal/mesh/internal/types/computed_routes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package types

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/hashicorp/consul/internal/resource/resourcetest"
pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1"
"github.com/hashicorp/consul/proto/private/prototest"
"github.com/hashicorp/consul/sdk/testutil"
)

func TestValidateComputedRoutes(t *testing.T) {
type testcase struct {
routes *pbmesh.ComputedRoutes
expectErr string
}

run := func(t *testing.T, tc testcase) {
res := resourcetest.Resource(ComputedRoutesType, "api").
WithData(t, tc.routes).
Build()

err := ValidateComputedRoutes(res)

// Verify that validate didn't actually change the object.
got := resourcetest.MustDecode[pbmesh.ComputedRoutes, *pbmesh.ComputedRoutes](t, res)
prototest.AssertDeepEqual(t, tc.routes, got.Data)

if tc.expectErr == "" {
require.NoError(t, err)
} else {
testutil.RequireErrorContains(t, err, tc.expectErr)
}
}

cases := map[string]testcase{
"empty": {
routes: &pbmesh.ComputedRoutes{},
expectErr: `invalid "ported_configs" field: cannot be empty`,
},
"empty targets": {
routes: &pbmesh.ComputedRoutes{
PortedConfigs: map[string]*pbmesh.ComputedPortRoutes{
"http": {
Config: &pbmesh.ComputedPortRoutes_Tcp{
Tcp: &pbmesh.InterpretedTCPRoute{},
},
},
},
},
expectErr: `invalid value of key "http" within ported_configs: invalid "targets" field: cannot be empty`,
},
"valid": {
routes: &pbmesh.ComputedRoutes{
PortedConfigs: map[string]*pbmesh.ComputedPortRoutes{
"http": {
Config: &pbmesh.ComputedPortRoutes_Tcp{
Tcp: &pbmesh.InterpretedTCPRoute{},
},
Targets: map[string]*pbmesh.BackendTargetDetails{
"foo": {},
},
},
},
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
run(t, tc)
})
}
}
142 changes: 141 additions & 1 deletion internal/mesh/internal/types/destination_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
package types

import (
"errors"
"fmt"

"github.com/hashicorp/go-multierror"

"github.com/hashicorp/consul/internal/resource"
pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1"
"github.com/hashicorp/consul/proto-public/pbresource"
Expand All @@ -27,6 +32,141 @@ func RegisterDestinationPolicy(r resource.Registry) {
r.Register(resource.Registration{
Type: DestinationPolicyV1Alpha1Type,
Proto: &pbmesh.DestinationPolicy{},
Validate: nil,
Validate: ValidateDestinationPolicy,
})
}

func ValidateDestinationPolicy(res *pbresource.Resource) error {
var policy pbmesh.DestinationPolicy

if err := res.Data.UnmarshalTo(&policy); err != nil {
return resource.NewErrDataParse(&policy, err)
}

var merr error

if len(policy.PortConfigs) == 0 {
merr = multierror.Append(merr, resource.ErrInvalidField{
Name: "port_configs",
Wrapped: resource.ErrEmpty,
})
}

for port, pc := range policy.PortConfigs {
wrapErr := func(err error) error {
return resource.ErrInvalidMapValue{
Map: "port_configs",
Key: port,
Wrapped: err,
}
}

if dur := pc.ConnectTimeout.AsDuration(); dur < 0 {
merr = multierror.Append(merr, wrapErr(resource.ErrInvalidField{
Name: "connect_timeout",
Wrapped: fmt.Errorf("'%v', must be >= 0", dur),
}))
}
if dur := pc.RequestTimeout.AsDuration(); dur < 0 {
merr = multierror.Append(merr, wrapErr(resource.ErrInvalidField{
Name: "request_timeout",
Wrapped: fmt.Errorf("'%v', must be >= 0", dur),
}))
}

if pc.LoadBalancer != nil {
lb := pc.LoadBalancer
wrapLBErr := func(err error) error {
return wrapErr(resource.ErrInvalidMapValue{
Map: "load_balancer",
Key: port,
Wrapped: err,
})
}

if lb.Policy != pbmesh.LoadBalancerPolicy_LOAD_BALANCER_POLICY_RING_HASH && lb.Config != nil {
if _, ok := lb.Config.(*pbmesh.LoadBalancer_RingHashConfig); ok {
merr = multierror.Append(merr, wrapLBErr(resource.ErrInvalidField{
Name: "config",
Wrapped: fmt.Errorf("RingHashConfig specified for incompatible load balancing policy %q", lb.Policy),
}))
}
}

if lb.Policy != pbmesh.LoadBalancerPolicy_LOAD_BALANCER_POLICY_LEAST_REQUEST && lb.Config != nil {
if _, ok := lb.Config.(*pbmesh.LoadBalancer_LeastRequestConfig); ok {
merr = multierror.Append(merr, wrapLBErr(resource.ErrInvalidField{
Name: "config",
Wrapped: fmt.Errorf("LeastRequestConfig specified for incompatible load balancing policy %q", lb.Policy),
}))
}
}

if !lb.Policy.IsHashBased() && len(lb.HashPolicies) > 0 {
merr = multierror.Append(merr, wrapLBErr(resource.ErrInvalidField{
Name: "hash_policies",
Wrapped: fmt.Errorf("hash_policies specified for non-hash-based policy %q", lb.Policy),
}))
}

for i, hp := range lb.HashPolicies {
wrapHPErr := func(err error) error {
return wrapLBErr(resource.ErrInvalidListElement{
Name: "hash_policies",
Index: i,
Wrapped: err,
})
}

hasField := (hp.Field != pbmesh.HashPolicyField_HASH_POLICY_FIELD_UNSPECIFIED)

if hp.SourceIp {
if hasField {
merr = multierror.Append(merr, wrapHPErr(resource.ErrInvalidField{
Name: "field",
Wrapped: fmt.Errorf("a single hash policy cannot hash both a source address and a %q", hp.Field),
}))
}
if hp.FieldValue != "" {
merr = multierror.Append(merr, wrapHPErr(resource.ErrInvalidField{
Name: "field_value",
Wrapped: errors.New("cannot be specified when hashing source_ip"),
}))
}
}

if hasField && hp.FieldValue == "" {
merr = multierror.Append(merr, wrapHPErr(resource.ErrInvalidField{
Name: "field_value",
Wrapped: fmt.Errorf("field %q was specified without a field_value", hp.Field),
}))
}
if hp.FieldValue != "" && !hasField {
merr = multierror.Append(merr, wrapHPErr(resource.ErrInvalidField{
Name: "field_value",
Wrapped: errors.New("requires a field to apply to"),
}))
}
if hp.CookieConfig != nil {
if hp.Field != pbmesh.HashPolicyField_HASH_POLICY_FIELD_COOKIE {
merr = multierror.Append(merr, wrapHPErr(resource.ErrInvalidField{
Name: "cookie_config",
Wrapped: fmt.Errorf("incompatible with field %q", hp.Field),
}))
}
if hp.CookieConfig.Session && hp.CookieConfig.Ttl.AsDuration() != 0 {
merr = multierror.Append(merr, wrapHPErr(resource.ErrInvalidField{
Name: "cookie_config",
Wrapped: resource.ErrInvalidField{
Name: "ttl",
Wrapped: fmt.Errorf("a session cookie cannot have an associated TTL"),
},
}))
}
}
}
}
}

return merr
}
Loading

0 comments on commit c4ed456

Please sign in to comment.