Skip to content

Commit

Permalink
PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ishustava committed Sep 8, 2023
1 parent a281bdc commit 9a260b1
Show file tree
Hide file tree
Showing 23 changed files with 1,013 additions and 303 deletions.
4 changes: 4 additions & 0 deletions internal/mesh/exports.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
package mesh

import (
"github.com/hashicorp/consul/internal/controller"
"github.com/hashicorp/consul/internal/mesh/internal/controllers"
"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecarproxy"
"github.com/hashicorp/consul/internal/mesh/internal/controllers/sidecarproxy/status"
"github.com/hashicorp/consul/internal/mesh/internal/types"
"github.com/hashicorp/consul/internal/resource"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,21 @@ package sidecarproxycache
import (
"testing"

"github.com/stretchr/testify/require"

"github.com/hashicorp/consul/internal/catalog"
"github.com/hashicorp/consul/internal/mesh/internal/types"
"github.com/hashicorp/consul/internal/mesh/internal/types/intermediate"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/resourcetest"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/stretchr/testify/require"
)

func TestWrite_Create(t *testing.T) {
cache := NewDestinationsCache()

proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").ID()
proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
destination := testDestination(proxyID)
cache.WriteDestination(destination)

Expand All @@ -36,7 +38,8 @@ func TestWrite_Create(t *testing.T) {
func TestWrite_Update(t *testing.T) {
cache := NewDestinationsCache()

proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").ID()
proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
destination1 := testDestination(proxyID)
cache.WriteDestination(destination1)

Expand All @@ -58,7 +61,8 @@ func TestWrite_Update(t *testing.T) {
// Add another destination for a different proxy.
anotherProxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-def").ID()
destination3 := testDestination(anotherProxyID)
destination3.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-3").ReferenceNoSection()
destination3.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-3").
WithTenancy(resource.DefaultNamespacedTenancy()).ReferenceNoSection()
cache.WriteDestination(destination3)

actualSourceProxies = cache.sourceProxiesIndex
Expand Down Expand Up @@ -93,13 +97,15 @@ func TestWrite_Update(t *testing.T) {
func TestWrite_Delete(t *testing.T) {
cache := NewDestinationsCache()

proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").ID()
proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
destination1 := testDestination(proxyID)
cache.WriteDestination(destination1)

// Add another destination for the same proxy ID.
destination2 := testDestination(proxyID)
destination2.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-2").ReferenceNoSection()
destination2.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-2").
WithTenancy(resource.DefaultNamespacedTenancy()).ReferenceNoSection()
cache.WriteDestination(destination2)

cache.DeleteDestination(destination1.ServiceRef, destination1.Port)
Expand All @@ -117,7 +123,8 @@ func TestWrite_Delete(t *testing.T) {

// Try to delete non-existing destination and check that nothing has changed..
cache.DeleteDestination(
resourcetest.Resource(catalog.ServiceType, "does-not-exist").ReferenceNoSection(),
resourcetest.Resource(catalog.ServiceType, "does-not-exist").
WithTenancy(resource.DefaultNamespacedTenancy()).ReferenceNoSection(),
"doesn't-matter")

require.Contains(t, cache.store, KeyFromRefAndPort(destination2.ServiceRef, destination2.Port))
Expand All @@ -127,13 +134,15 @@ func TestWrite_Delete(t *testing.T) {
func TestDeleteSourceProxy(t *testing.T) {
cache := NewDestinationsCache()

proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").ID()
proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
destination1 := testDestination(proxyID)
cache.WriteDestination(destination1)

// Add another destination for the same proxy ID.
destination2 := testDestination(proxyID)
destination2.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-2").ReferenceNoSection()
destination2.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-2").
WithTenancy(resource.DefaultNamespacedTenancy()).ReferenceNoSection()
cache.WriteDestination(destination2)

cache.DeleteSourceProxy(proxyID)
Expand Down Expand Up @@ -162,13 +171,15 @@ func TestDeleteSourceProxy(t *testing.T) {
func TestDestinationsBySourceProxy(t *testing.T) {
cache := NewDestinationsCache()

proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").ID()
proxyID := resourcetest.Resource(types.ProxyStateTemplateType, "service-workload-abc").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
destination1 := testDestination(proxyID)
cache.WriteDestination(destination1)

// Add another destination for the same proxy ID.
destination2 := testDestination(proxyID)
destination2.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-2").ReferenceNoSection()
destination2.ServiceRef = resourcetest.Resource(catalog.ServiceType, "test-service-2").
WithTenancy(resource.DefaultNamespacedTenancy()).ReferenceNoSection()
cache.WriteDestination(destination2)

actualDestinations := cache.DestinationsBySourceProxy(proxyID)
Expand All @@ -178,9 +189,11 @@ func TestDestinationsBySourceProxy(t *testing.T) {

func testDestination(proxyID *pbresource.ID) intermediate.CombinedDestinationRef {
return intermediate.CombinedDestinationRef{
ServiceRef: resourcetest.Resource(catalog.ServiceType, "test-service").ReferenceNoSection(),
Port: "tcp",
ExplicitDestinationsID: resourcetest.Resource(types.UpstreamsType, "test-servicedestinations").ID(),
ServiceRef: resourcetest.Resource(catalog.ServiceType, "test-service").
WithTenancy(resource.DefaultNamespacedTenancy()).ReferenceNoSection(),
Port: "tcp",
ExplicitDestinationsID: resourcetest.Resource(types.UpstreamsType, "test-servicedestinations").
WithTenancy(resource.DefaultNamespacedTenancy()).ID(),
SourceProxies: map[resource.ReferenceKey]struct{}{
resource.NewReferenceKey(proxyID): {},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,37 @@ package sidecarproxycache
import (
"testing"

"github.com/stretchr/testify/require"

"github.com/hashicorp/consul/internal/mesh/internal/types"
"github.com/hashicorp/consul/internal/resource"
"github.com/hashicorp/consul/internal/resource/resourcetest"
"github.com/hashicorp/consul/proto-public/pbresource"
"github.com/hashicorp/consul/proto/private/prototest"
"github.com/stretchr/testify/require"
)

func TestProxyConfigurationCache(t *testing.T) {
cache := NewProxyConfigurationCache()

// Create some proxy configurations.
proxyCfg1 := resourcetest.Resource(types.ProxyConfigurationType, "test-cfg-1").ID()
proxyCfg2 := resourcetest.Resource(types.ProxyConfigurationType, "test-cfg-2").ID()
proxyCfg3 := resourcetest.Resource(types.ProxyConfigurationType, "test-cfg-3").ID()
proxyCfg1 := resourcetest.Resource(types.ProxyConfigurationType, "test-cfg-1").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
proxyCfg2 := resourcetest.Resource(types.ProxyConfigurationType, "test-cfg-2").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
proxyCfg3 := resourcetest.Resource(types.ProxyConfigurationType, "test-cfg-3").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()

// Create some proxy state templates.
p1 := resourcetest.Resource(types.ProxyStateTemplateType, "w-111").ID()
p2 := resourcetest.Resource(types.ProxyStateTemplateType, "w-222").ID()
p3 := resourcetest.Resource(types.ProxyStateTemplateType, "w-333").ID()
p4 := resourcetest.Resource(types.ProxyStateTemplateType, "w-444").ID()
p5 := resourcetest.Resource(types.ProxyStateTemplateType, "w-555").ID()
p1 := resourcetest.Resource(types.ProxyStateTemplateType, "w-111").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
p2 := resourcetest.Resource(types.ProxyStateTemplateType, "w-222").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
p3 := resourcetest.Resource(types.ProxyStateTemplateType, "w-333").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
p4 := resourcetest.Resource(types.ProxyStateTemplateType, "w-444").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()
p5 := resourcetest.Resource(types.ProxyStateTemplateType, "w-555").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()

// Track these and make sure there's some overlap.
cache.TrackProxyConfiguration(proxyCfg1, []resource.ReferenceOrID{p1, p2, p4})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ type Builder struct {
proxyCfg *pbmesh.ProxyConfiguration
trustDomain string
localDatacenter string

outboundListenerBuilder *ListenerBuilder
}

func New(id *pbresource.ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package builder

import (
"google.golang.org/protobuf/types/known/wrapperspb"

"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/envoyextensions/xdscommon"
"github.com/hashicorp/consul/internal/mesh/internal/types/intermediate"
Expand All @@ -12,7 +14,6 @@ import (
pbmesh "github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1"
"github.com/hashicorp/consul/proto-public/pbmesh/v1alpha1/pbproxystate"
"github.com/hashicorp/consul/proto-public/pbresource"
"google.golang.org/protobuf/types/known/wrapperspb"
)

func (b *Builder) BuildDestinations(destinations []*intermediate.Destination) *Builder {
Expand Down Expand Up @@ -56,11 +57,9 @@ func (b *Builder) buildExplicitDestination(destination *intermediate.Destination
addEndpointsRef(clusterName, destination.ServiceEndpoints.Resource.Id, meshPortName)
}
}

return b
}

func (b *Builder) buildImplicitDestination(destination *intermediate.Destination) *Builder {
func (b *Builder) buildImplicitDestination(destination *intermediate.Destination) {
serviceRef := resource.Reference(destination.ServiceEndpoints.Resource.Owner, "")
clusterName := DestinationClusterName(serviceRef, b.localDatacenter, b.trustDomain)
statPrefix := DestinationStatPrefix(serviceRef, b.localDatacenter)
Expand All @@ -74,13 +73,13 @@ func (b *Builder) buildImplicitDestination(destination *intermediate.Destination
meshPortName := findMeshPort(destination.ServiceEndpoints.Endpoints.Endpoints[0].Ports)

for _, port := range destination.ServiceEndpoints.Endpoints.Endpoints[0].Ports {
b.addRouterWithIPMatch(clusterName, statPrefix, port.Protocol, destination.VirtualIPs).
b.outboundListenerBuilder.
addRouterWithIPMatch(clusterName, statPrefix, port.Protocol, destination.VirtualIPs).
buildListener().
addCluster(clusterName, destination.Identities).
addEndpointsRef(clusterName, destination.ServiceEndpoints.Resource.Id, meshPortName)
}
}

return b
}

func (b *Builder) addOutboundDestinationListener(explicit *pbmesh.Upstream) *ListenerBuilder {
Expand Down Expand Up @@ -113,7 +112,7 @@ func (b *Builder) addOutboundDestinationListener(explicit *pbmesh.Upstream) *Lis
return b.NewListenerBuilder(listener)
}

func (b *Builder) addOutboundListener(port uint32) *Builder {
func (b *Builder) addOutboundListener(port uint32) *ListenerBuilder {
listener := &pbproxystate.Listener{
Name: xdscommon.OutboundListenerName,
Direction: pbproxystate.Direction_DIRECTION_OUTBOUND,
Expand All @@ -126,16 +125,19 @@ func (b *Builder) addOutboundListener(port uint32) *Builder {
Capabilities: []pbproxystate.Capability{pbproxystate.Capability_CAPABILITY_TRANSPARENT},
}

return b.addListener(listener)
lb := b.NewListenerBuilder(listener)

// Save outbound listener builder so we can use it in the future.
b.outboundListenerBuilder = lb

return lb
}

func (l *ListenerBuilder) addRouter(clusterName, statPrefix string, protocol pbcatalog.Protocol) *ListenerBuilder {
return b.addRouterWithIPMatch(clusterName, statPrefix, protocol, nil)
return l.addRouterWithIPMatch(clusterName, statPrefix, protocol, nil)
}

func (b *Builder) addRouterWithIPMatch(clusterName, statPrefix string, protocol pbcatalog.Protocol, vips []string) *Builder {
listener := b.getLastBuiltListener()

func (l *ListenerBuilder) addRouterWithIPMatch(clusterName, statPrefix string, protocol pbcatalog.Protocol, vips []string) *ListenerBuilder {
// For explicit destinations, we have no filter chain match, and filters are based on port protocol.
router := &pbproxystate.Router{}
switch protocol {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,24 @@ func TestBuildExplicitDestinations(t *testing.T) {
Build()

actual := protoToJSON(t, proxyTmpl)
expected := goldenValue(t, name, actual, *update)
expected := golden.Get(t, actual, name)

require.JSONEq(t, expected, actual)
}
}

func TestBuildImplicitDestinations(t *testing.T) {
api1Endpoints := resourcetest.Resource(catalog.ServiceEndpointsType, "api-1").
WithOwner(resourcetest.Resource(catalog.ServiceType, "api-1").ID()).
WithOwner(
resourcetest.Resource(catalog.ServiceType, "api-1").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()).
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, endpointsData).Build()

api2Endpoints := resourcetest.Resource(catalog.ServiceEndpointsType, "api-2").
WithOwner(resourcetest.Resource(catalog.ServiceType, "api-2").ID()).
WithOwner(resourcetest.Resource(catalog.ServiceType, "api-2").
WithTenancy(resource.DefaultNamespacedTenancy()).ID()).
WithTenancy(resource.DefaultNamespacedTenancy()).
WithData(t, endpointsData).Build()

api1Identity := &pbresource.Reference{
Expand Down
Loading

0 comments on commit 9a260b1

Please sign in to comment.