Skip to content

[1.15.x] peering: ensure that merged central configs of peered upstreams for partitioned downstreams work #17181

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.

Sign up for GitHub

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

Merged
rboyer merged 2 commits into from
Apr 28, 2023
Merged

[1.15.x] peering: ensure that merged central configs of peered upstreams for partitioned downstreams work #17181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
870e8e0
[1.15.x] peering: ensure that merged central configs of peered upstre…
rboyer Apr 28, 2023
4a9a01c
changelog
rboyer Apr 28, 2023
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
3 changes: 3 additions & 0 deletions .changelog/17179.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
peering: ensure that merged central configs of peered upstreams for partitioned downstreams work
```
28 changes: 24 additions & 4 deletions agent/configentry/merge_service_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ func MergeServiceConfig(defaults *structs.ServiceConfigResponse, service *struct
// remoteUpstreams contains synthetic Upstreams generated from central config (service-defaults.UpstreamConfigs).
remoteUpstreams := make(map[structs.PeeredServiceName]structs.Upstream)

// If the arguments did not fully normalize tenancy stuff, take care of that now.
entMeta := ns.EnterpriseMeta
entMeta.Normalize()

if len(defaults.UpstreamIDConfigs) > 0 {
// Handle legacy upstreams. This should be removed in Consul 1.16.
for _, us := range defaults.UpstreamIDConfigs {
Expand Down Expand Up @@ -183,11 +187,21 @@ func MergeServiceConfig(defaults *structs.ServiceConfigResponse, service *struct
return nil, fmt.Errorf("failed to parse upstream config map for %s: %v", us.Upstream.String(), err)
}

// If the defaults did not fully normalize tenancy stuff, take care of
// that now too.
psn := us.Upstream // only normalize the copy
psn.ServiceName.EnterpriseMeta.Normalize()

// Normalize the partition field specially.
if psn.Peer != "" {
psn.ServiceName.OverridePartition(entMeta.PartitionOrDefault())
}

remoteUpstreams[us.Upstream] = structs.Upstream{
DestinationNamespace: us.Upstream.ServiceName.NamespaceOrDefault(),
DestinationPartition: us.Upstream.ServiceName.PartitionOrDefault(),
DestinationName: us.Upstream.ServiceName.Name,
DestinationPeer: us.Upstream.Peer,
DestinationNamespace: psn.ServiceName.NamespaceOrDefault(),
DestinationPartition: psn.ServiceName.PartitionOrDefault(),
DestinationName: psn.ServiceName.Name,
DestinationPeer: psn.Peer,
Config: us.Config,
MeshGateway: parsed.MeshGateway,
CentrallyConfigured: true,
Expand All @@ -209,6 +223,12 @@ func MergeServiceConfig(defaults *structs.ServiceConfigResponse, service *struct
}

uid := us.DestinationID()

// Normalize the partition field specially.
if uid.Peer != "" {
uid.ServiceName.OverridePartition(entMeta.PartitionOrDefault())
}

localUpstreams[uid] = struct{}{}
remoteCfg, ok := remoteUpstreams[uid]
if !ok {
Expand Down
209 changes: 209 additions & 0 deletions agent/configentry/merge_service_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/structs"
)

Expand Down Expand Up @@ -270,6 +271,214 @@ func Test_MergeServiceConfig_Extensions(t *testing.T) {
}
}

func isEnterprise() bool {
return acl.PartitionOrDefault("") == "default"
}

func Test_MergeServiceConfig_peeredCentralDefaultsMerging(t *testing.T) {
partitions := []string{"default"}
if isEnterprise() {
partitions = append(partitions, "part1")
}

const peerName = "my-peer"

newDefaults := func(partition string) *structs.ServiceConfigResponse {
// client agents
return &structs.ServiceConfigResponse{
ProxyConfig: map[string]any{
"protocol": "http",
},
UpstreamConfigs: []structs.OpaqueUpstreamConfig{
{
Upstream: structs.PeeredServiceName{
ServiceName: structs.ServiceName{
Name: "*",
EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(partition, "*"),
},
},
Config: map[string]any{
"mesh_gateway": map[string]any{
"Mode": "local",
},
"protocol": "http",
},
},
{
Upstream: structs.PeeredServiceName{
ServiceName: structs.ServiceName{
Name: "static-server",
EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(partition, "default"),
},
Peer: peerName,
},
Config: map[string]any{
"mesh_gateway": map[string]any{
"Mode": "local",
},
"protocol": "http",
},
},
},
MeshGateway: structs.MeshGatewayConfig{
Mode: "local",
},
}
}

for _, partition := range partitions {
t.Run("partition="+partition, func(t *testing.T) {
t.Run("clients", func(t *testing.T) {
defaults := newDefaults(partition)

service := &structs.NodeService{
Kind: "connect-proxy",
ID: "static-client-sidecar-proxy",
Service: "static-client-sidecar-proxy",
Address: "",
Port: 21000,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "static-client",
DestinationServiceID: "static-client",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 8080,
Upstreams: []structs.Upstream{
{
DestinationType: "service",
DestinationNamespace: "default",
DestinationPartition: partition,
DestinationPeer: peerName,
DestinationName: "static-server",
LocalBindAddress: "0.0.0.0",
LocalBindPort: 5000,
},
},
},
EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(partition, "default"),
}

expect := &structs.NodeService{
Kind: "connect-proxy",
ID: "static-client-sidecar-proxy",
Service: "static-client-sidecar-proxy",
Address: "",
Port: 21000,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "static-client",
DestinationServiceID: "static-client",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 8080,
Config: map[string]any{
"protocol": "http",
},
Upstreams: []structs.Upstream{
{
DestinationType: "service",
DestinationNamespace: "default",
DestinationPartition: partition,
DestinationPeer: peerName,
DestinationName: "static-server",
LocalBindAddress: "0.0.0.0",
LocalBindPort: 5000,
MeshGateway: structs.MeshGatewayConfig{
Mode: "local",
},
Config: map[string]any{},
},
},
MeshGateway: structs.MeshGatewayConfig{
Mode: "local",
},
},
EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(partition, "default"),
}

got, err := MergeServiceConfig(defaults, service)
require.NoError(t, err)
require.Equal(t, expect, got)
})

t.Run("dataplanes", func(t *testing.T) {
defaults := newDefaults(partition)

service := &structs.NodeService{
Kind: "connect-proxy",
ID: "static-client-sidecar-proxy",
Service: "static-client-sidecar-proxy",
Address: "10.61.57.9",
TaggedAddresses: map[string]structs.ServiceAddress{
"consul-virtual": {
Address: "240.0.0.2",
Port: 20000,
},
},
Port: 20000,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "static-client",
DestinationServiceID: "static-client",
LocalServicePort: 8080,
Upstreams: []structs.Upstream{
{
DestinationType: "",
DestinationNamespace: "default",
DestinationPeer: peerName,
DestinationName: "static-server",
LocalBindAddress: "0.0.0.0",
LocalBindPort: 5000,
},
},
},
EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(partition, "default"),
}

expect := &structs.NodeService{
Kind: "connect-proxy",
ID: "static-client-sidecar-proxy",
Service: "static-client-sidecar-proxy",
Address: "10.61.57.9",
TaggedAddresses: map[string]structs.ServiceAddress{
"consul-virtual": {
Address: "240.0.0.2",
Port: 20000,
},
},
Port: 20000,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "static-client",
DestinationServiceID: "static-client",
LocalServicePort: 8080,
Config: map[string]any{
"protocol": "http",
},
Upstreams: []structs.Upstream{
{
DestinationType: "",
DestinationNamespace: "default",
DestinationPeer: peerName,
DestinationName: "static-server",
LocalBindAddress: "0.0.0.0",
LocalBindPort: 5000,
MeshGateway: structs.MeshGatewayConfig{
Mode: "local", // This field vanishes if the merging does not work for dataplanes.
},
Config: map[string]any{},
},
},
MeshGateway: structs.MeshGatewayConfig{
Mode: "local",
},
},
EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(partition, "default"),
}

got, err := MergeServiceConfig(defaults, service)
require.NoError(t, err)
require.Equal(t, expect, got)
})
})
}
}

func Test_MergeServiceConfig_UpstreamOverrides(t *testing.T) {
type args struct {
defaults *structs.ServiceConfigResponse
Expand Down