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

mesh: normalize/default/validate tenancy components of mesh internal References #18827

Merged
merged 5 commits into from
Sep 18, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,46 +25,59 @@ import (
func TestMapDestinationsToProxyStateTemplate(t *testing.T) {
client := svctest.RunResourceService(t, types.Register, catalog.RegisterTypes)
webWorkload1 := resourcetest.Resource(catalog.WorkloadType, "web-abc").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, &pbcatalog.Workload{
Addresses: []*pbcatalog.WorkloadAddress{{Host: "10.0.0.1"}},
Ports: map[string]*pbcatalog.WorkloadPort{"tcp": {Port: 8081, Protocol: pbcatalog.Protocol_PROTOCOL_TCP}},
}).
Write(t, client)
webWorkload2 := resourcetest.Resource(catalog.WorkloadType, "web-def").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, &pbcatalog.Workload{
Addresses: []*pbcatalog.WorkloadAddress{{Host: "10.0.0.2"}},
Ports: map[string]*pbcatalog.WorkloadPort{"tcp": {Port: 8081, Protocol: pbcatalog.Protocol_PROTOCOL_TCP}},
}).
Write(t, client)
webWorkload3 := resourcetest.Resource(catalog.WorkloadType, "non-prefix-web").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, &pbcatalog.Workload{
Addresses: []*pbcatalog.WorkloadAddress{{Host: "10.0.0.3"}},
Ports: map[string]*pbcatalog.WorkloadPort{"tcp": {Port: 8081, Protocol: pbcatalog.Protocol_PROTOCOL_TCP}},
}).
Write(t, client)

var (
api1ServiceRef = resourcetest.Resource(catalog.ServiceType, "api-1").
WithTenancy(resource.DefaultNamespacedTenancy()).
ReferenceNoSection()
api2ServiceRef = resourcetest.Resource(catalog.ServiceType, "api-2").
WithTenancy(resource.DefaultNamespacedTenancy()).
ReferenceNoSection()
)

webDestinationsData := &pbmesh.Upstreams{
Workloads: &pbcatalog.WorkloadSelector{
Names: []string{"non-prefix-web"},
Prefixes: []string{"web"},
},
Upstreams: []*pbmesh.Upstream{
{
DestinationRef: resourcetest.Resource(catalog.ServiceType, "api-1").ReferenceNoSection(),
DestinationRef: api1ServiceRef,
DestinationPort: "tcp",
},
{
DestinationRef: resourcetest.Resource(catalog.ServiceType, "api-2").ReferenceNoSection(),
DestinationRef: api2ServiceRef,
DestinationPort: "tcp1",
},
{
DestinationRef: resourcetest.Resource(catalog.ServiceType, "api-2").ReferenceNoSection(),
DestinationRef: api2ServiceRef,
DestinationPort: "tcp2",
},
},
}

webDestinations := resourcetest.Resource(types.UpstreamsType, "web-destinations").
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, webDestinationsData).
Write(t, client)

Expand All @@ -81,10 +94,14 @@ func TestMapDestinationsToProxyStateTemplate(t *testing.T) {
require.NoError(t, err)
prototest.AssertElementsMatch(t, expRequests, requests)

//var expDestinations []*intermediate.CombinedDestinationRef
proxy1ID := resourcetest.Resource(types.ProxyStateTemplateType, webWorkload1.Id.Name).ID()
proxy2ID := resourcetest.Resource(types.ProxyStateTemplateType, webWorkload2.Id.Name).ID()
proxy3ID := resourcetest.Resource(types.ProxyStateTemplateType, webWorkload3.Id.Name).ID()
var (
proxy1ID = resourcetest.Resource(types.ProxyStateTemplateType, webWorkload1.Id.Name).
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
proxy2ID = resourcetest.Resource(types.ProxyStateTemplateType, webWorkload2.Id.Name).
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
proxy3ID = resourcetest.Resource(types.ProxyStateTemplateType, webWorkload3.Id.Name).
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
)
for _, u := range webDestinationsData.Upstreams {
expDestination := intermediate.CombinedDestinationRef{
ServiceRef: u.DestinationRef,
Expand Down
63 changes: 44 additions & 19 deletions internal/mesh/internal/types/grpc_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,45 @@ var (

func RegisterGRPCRoute(r resource.Registry) {
r.Register(resource.Registration{
Type: GRPCRouteV1Alpha1Type,
Proto: &pbmesh.GRPCRoute{},
Scope: resource.ScopeNamespace,
// TODO(rb): normalize parent/backend ref tenancies in a Mutate hook
Type: GRPCRouteV1Alpha1Type,
Proto: &pbmesh.GRPCRoute{},
Scope: resource.ScopeNamespace,
Mutate: MutateGRPCRoute,
Validate: ValidateGRPCRoute,
})
}

func MutateGRPCRoute(res *pbresource.Resource) error {
var route pbmesh.GRPCRoute

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

changed := false

if mutateParentRefs(res.Id.Tenancy, route.ParentRefs) {
changed = true
}

for _, rule := range route.Rules {
for _, backend := range rule.BackendRefs {
if backend.BackendRef == nil || backend.BackendRef.Ref == nil {
continue
}
if mutateXRouteRef(res.Id.Tenancy, backend.BackendRef.Ref) {
changed = true
}
}
}

if !changed {
return nil
}

return res.Data.MarshalFrom(&route)
}

func ValidateGRPCRoute(res *pbresource.Resource) error {
var route pbmesh.GRPCRoute

Expand Down Expand Up @@ -170,14 +201,6 @@ func ValidateGRPCRoute(res *pbresource.Resource) error {
}

if len(rule.BackendRefs) == 0 {
/*
BackendRefs (optional)¶

BackendRefs defines API objects where matching requests should be
sent. If unspecified, the rule performs no forwarding. If
unspecified and no filters are specified that would result in a
response being sent, a 404 error code is returned.
*/
merr = multierror.Append(merr, wrapRuleErr(
resource.ErrInvalidField{
Name: "backend_refs",
Expand All @@ -193,13 +216,15 @@ func ValidateGRPCRoute(res *pbresource.Resource) error {
Wrapped: err,
})
}
for _, err := range validateBackendRef(hbref.BackendRef) {
merr = multierror.Append(merr, wrapBackendRefErr(
resource.ErrInvalidField{
Name: "backend_ref",
Wrapped: err,
},
))

wrapBackendRefFieldErr := func(err error) error {
return wrapBackendRefErr(resource.ErrInvalidField{
Name: "backend_ref",
Wrapped: err,
})
}
if err := validateBackendRef(hbref.BackendRef, wrapBackendRefFieldErr); err != nil {
merr = multierror.Append(merr, err)
}

if len(hbref.Filters) > 0 {
Expand Down
95 changes: 94 additions & 1 deletion internal/mesh/internal/types/grpc_route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,99 @@ import (
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"

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

func TestMutateGRPCRoute(t *testing.T) {
type testcase struct {
routeTenancy *pbresource.Tenancy
route *pbmesh.GRPCRoute
expect *pbmesh.GRPCRoute
}

cases := map[string]testcase{}

// Add common parent refs test cases.
for name, parentTC := range getXRouteParentRefMutateTestCases() {
cases["parent-ref: "+name] = testcase{
routeTenancy: parentTC.routeTenancy,
route: &pbmesh.GRPCRoute{
ParentRefs: parentTC.refs,
},
expect: &pbmesh.GRPCRoute{
ParentRefs: parentTC.expect,
},
}
}
// add common backend ref test cases.
for name, backendTC := range getXRouteBackendRefMutateTestCases() {
var (
refs []*pbmesh.GRPCBackendRef
expect []*pbmesh.GRPCBackendRef
)
for _, br := range backendTC.refs {
refs = append(refs, &pbmesh.GRPCBackendRef{
BackendRef: br,
})
}
for _, br := range backendTC.expect {
expect = append(expect, &pbmesh.GRPCBackendRef{
BackendRef: br,
})
}
cases["backend-ref: "+name] = testcase{
routeTenancy: backendTC.routeTenancy,
route: &pbmesh.GRPCRoute{
ParentRefs: []*pbmesh.ParentReference{
newParentRef(catalog.ServiceType, "web", ""),
},
Rules: []*pbmesh.GRPCRouteRule{
{BackendRefs: refs},
},
},
expect: &pbmesh.GRPCRoute{
ParentRefs: []*pbmesh.ParentReference{
newParentRef(catalog.ServiceType, "web", ""),
},
Rules: []*pbmesh.GRPCRouteRule{
{BackendRefs: expect},
},
},
}
}

run := func(t *testing.T, tc testcase) {
res := resourcetest.Resource(GRPCRouteType, "api").
WithTenancy(tc.routeTenancy).
WithData(t, tc.route).
Build()

err := MutateGRPCRoute(res)
require.NoError(t, err)

got := resourcetest.MustDecode[*pbmesh.GRPCRoute](t, res)

if tc.expect == nil {
tc.expect = proto.Clone(tc.route).(*pbmesh.GRPCRoute)
}

prototest.AssertDeepEqual(t, tc.expect, got.Data)
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
run(t, tc)
})
}
}

func TestValidateGRPCRoute(t *testing.T) {
type testcase struct {
route *pbmesh.GRPCRoute
Expand All @@ -26,7 +111,15 @@ func TestValidateGRPCRoute(t *testing.T) {
WithData(t, tc.route).
Build()

err := ValidateGRPCRoute(res)
// Ensure things are properly mutated and updated in the inputs.
err := MutateGRPCRoute(res)
require.NoError(t, err)
{
mutated := resourcetest.MustDecode[*pbmesh.GRPCRoute](t, res)
tc.route = mutated.Data
}

err = ValidateGRPCRoute(res)

// Verify that validate didn't actually change the object.
got := resourcetest.MustDecode[*pbmesh.GRPCRoute](t, res)
Expand Down
37 changes: 20 additions & 17 deletions internal/mesh/internal/types/http_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ func MutateHTTPRoute(res *pbresource.Resource) error {

changed := false

if mutateParentRefs(res.Id.Tenancy, route.ParentRefs) {
changed = true
}

for _, rule := range route.Rules {
for _, match := range rule.Matches {
if match.Method != "" {
Expand All @@ -59,10 +63,16 @@ func MutateHTTPRoute(res *pbresource.Resource) error {
}
}
}
for _, backend := range rule.BackendRefs {
if backend.BackendRef == nil || backend.BackendRef.Ref == nil {
continue
}
if mutateXRouteRef(res.Id.Tenancy, backend.BackendRef.Ref) {
changed = true
}
}
}

// TODO(rb): normalize parent/backend ref tenancies

if !changed {
return nil
}
Expand Down Expand Up @@ -264,14 +274,6 @@ func ValidateHTTPRoute(res *pbresource.Resource) error {
}

if len(rule.BackendRefs) == 0 {
/*
BackendRefs (optional)¶

BackendRefs defines API objects where matching requests should be
sent. If unspecified, the rule performs no forwarding. If
unspecified and no filters are specified that would result in a
response being sent, a 404 error code is returned.
*/
merr = multierror.Append(merr, wrapRuleErr(
resource.ErrInvalidField{
Name: "backend_refs",
Expand All @@ -288,13 +290,14 @@ func ValidateHTTPRoute(res *pbresource.Resource) error {
})
}

for _, err := range validateBackendRef(hbref.BackendRef) {
merr = multierror.Append(merr, wrapBackendRefErr(
resource.ErrInvalidField{
Name: "backend_ref",
Wrapped: err,
},
))
wrapBackendRefFieldErr := func(err error) error {
return wrapBackendRefErr(resource.ErrInvalidField{
Name: "backend_ref",
Wrapped: err,
})
}
if err := validateBackendRef(hbref.BackendRef, wrapBackendRefFieldErr); err != nil {
merr = multierror.Append(merr, err)
}

if len(hbref.Filters) > 0 {
Expand Down
Loading