Add `Targets` field to service resolver failovers. (#14162)

This field will be used for cluster peering failover.
This commit is contained in:
Eric Haberkorn 2022-08-15 09:20:25 -04:00 committed by GitHub
parent c20d016f62
commit 40ce1c8288
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 1088 additions and 467 deletions

5
.changelog/14162.txt Normal file
View File

@ -0,0 +1,5 @@
```release-note:improvement
config-entry: Validate that service-resolver `Failover`s and `Redirect`s only
specify `Partition` and `Namespace` on Consul Enterprise. This prevents scenarios
where OSS Consul would save service-resolvers that require Consul Enterprise.
```

View File

@ -954,6 +954,10 @@ func (e *ServiceResolverConfigEntry) Validate() error {
r := e.Redirect
if err := r.ValidateEnterprise(); err != nil {
return fmt.Errorf("Redirect: %s", err.Error())
}
if len(e.Failover) > 0 {
return fmt.Errorf("Redirect and Failover cannot both be set")
}
@ -988,18 +992,59 @@ func (e *ServiceResolverConfigEntry) Validate() error {
return fmt.Errorf("Cross-datacenter failover is only supported in the default partition")
}
if subset != "*" && !isSubset(subset) {
return fmt.Errorf("Bad Failover[%q]: not a valid subset", subset)
errorPrefix := fmt.Sprintf("Bad Failover[%q]: ", subset)
if err := f.ValidateEnterprise(); err != nil {
return fmt.Errorf(errorPrefix + err.Error())
}
if f.Service == "" && f.ServiceSubset == "" && f.Namespace == "" && len(f.Datacenters) == 0 {
return fmt.Errorf("Bad Failover[%q] one of Service, ServiceSubset, Namespace, or Datacenters is required", subset)
if subset != "*" && !isSubset(subset) {
return fmt.Errorf(errorPrefix + "not a valid subset subset")
}
if f.isEmpty() {
return fmt.Errorf(errorPrefix + "one of Service, ServiceSubset, Namespace, Targets, or Datacenters is required")
}
if f.ServiceSubset != "" {
if f.Service == "" || f.Service == e.Name {
if !isSubset(f.ServiceSubset) {
return fmt.Errorf("Bad Failover[%q].ServiceSubset %q is not a valid subset of %q", subset, f.ServiceSubset, f.Service)
return fmt.Errorf("%sServiceSubset %q is not a valid subset of %q", errorPrefix, f.ServiceSubset, f.Service)
}
}
}
if len(f.Datacenters) != 0 && len(f.Targets) != 0 {
return fmt.Errorf("Bad Failover[%q]: Targets cannot be set with Datacenters", subset)
}
if f.ServiceSubset != "" && len(f.Targets) != 0 {
return fmt.Errorf("Bad Failover[%q]: Targets cannot be set with ServiceSubset", subset)
}
if f.Service != "" && len(f.Targets) != 0 {
return fmt.Errorf("Bad Failover[%q]: Targets cannot be set with Service", subset)
}
for i, target := range f.Targets {
errorPrefix := fmt.Sprintf("Bad Failover[%q].Targets[%d]: ", subset, i)
if err := target.ValidateEnterprise(); err != nil {
return fmt.Errorf(errorPrefix + err.Error())
}
switch {
case target.Peer != "" && target.ServiceSubset != "":
return fmt.Errorf(errorPrefix + "Peer cannot be set with ServiceSubset")
case target.Peer != "" && target.Partition != "":
return fmt.Errorf(errorPrefix + "Partition cannot be set with Peer")
case target.Peer != "" && target.Datacenter != "":
return fmt.Errorf(errorPrefix + "Peer cannot be set with Datacenter")
case target.Partition != "" && target.Datacenter != "":
return fmt.Errorf(errorPrefix + "Partition cannot be set with Datacenter")
case target.ServiceSubset != "" && (target.Service == "" || target.Service == e.Name):
if !isSubset(target.ServiceSubset) {
return fmt.Errorf("%sServiceSubset %q is not a valid subset of %q", errorPrefix, target.ServiceSubset, e.Name)
}
}
}
@ -1107,10 +1152,25 @@ func (e *ServiceResolverConfigEntry) ListRelatedServices() []ServiceID {
if len(e.Failover) > 0 {
for _, failover := range e.Failover {
if len(failover.Targets) == 0 {
failoverID := NewServiceID(defaultIfEmpty(failover.Service, e.Name), failover.GetEnterpriseMeta(&e.EnterpriseMeta))
if failoverID != svcID {
found[failoverID] = struct{}{}
}
continue
}
for _, target := range failover.Targets {
// We can't know about related services on cluster peers.
if target.Peer != "" {
continue
}
failoverID := NewServiceID(defaultIfEmpty(target.Service, e.Name), target.GetEnterpriseMeta(failover.GetEnterpriseMeta(&e.EnterpriseMeta)))
if failoverID != svcID {
found[failoverID] = struct{}{}
}
}
}
}
@ -1175,8 +1235,9 @@ type ServiceResolverRedirect struct {
// There are some restrictions on what is allowed in here:
//
// - Service, ServiceSubset, Namespace, and Datacenters cannot all be
// empty at once.
// - Service, ServiceSubset, Namespace, Datacenters, and Targets cannot all be
// empty at once. When Targets is defined, the other fields should not be
// populated.
//
type ServiceResolverFailover struct {
// Service is the service to resolve instead of the default as the failover
@ -1205,6 +1266,37 @@ type ServiceResolverFailover struct {
//
// This is a DESTINATION during failover.
Datacenters []string `json:",omitempty"`
// Targets specifies a fixed list of failover targets to try. We never try a
// target multiple times, so those are subtracted from this list before
// proceeding.
//
// This is a DESTINATION during failover.
Targets []ServiceResolverFailoverTarget `json:",omitempty"`
}
func (f *ServiceResolverFailover) isEmpty() bool {
return f.Service == "" && f.ServiceSubset == "" && f.Namespace == "" && len(f.Datacenters) == 0 && len(f.Targets) == 0
}
type ServiceResolverFailoverTarget struct {
// Service specifies the name of the service to try during failover.
Service string `json:",omitempty"`
// ServiceSubset specifies the service subset to try during failover.
ServiceSubset string `json:",omitempty" alias:"service_subset"`
// Partition specifies the partition to try during failover.
Partition string `json:",omitempty"`
// Namespace specifies the namespace to try during failover.
Namespace string `json:",omitempty"`
// Datacenter specifies the datacenter to try during failover.
Datacenter string `json:",omitempty"`
// Peer specifies the name of the cluster peer to try during failover.
Peer string `json:",omitempty"`
}
// LoadBalancer determines the load balancing policy and configuration for services

View File

@ -4,6 +4,8 @@
package structs
import (
"fmt"
"github.com/hashicorp/consul/acl"
)
@ -25,12 +27,56 @@ func (redir *ServiceResolverRedirect) GetEnterpriseMeta(_ *acl.EnterpriseMeta) *
return DefaultEnterpriseMetaInDefaultPartition()
}
// ValidateEnterprise validates that enterprise fields are only set
// with enterprise binaries.
func (redir *ServiceResolverRedirect) ValidateEnterprise() error {
if redir.Partition != "" {
return fmt.Errorf("Setting Partition requires Consul Enterprise")
}
if redir.Namespace != "" {
return fmt.Errorf("Setting Namespace requires Consul Enterprise")
}
return nil
}
// GetEnterpriseMeta is used to synthesize the EnterpriseMeta struct from
// fields in the ServiceResolverFailover
func (failover *ServiceResolverFailover) GetEnterpriseMeta(_ *acl.EnterpriseMeta) *acl.EnterpriseMeta {
return DefaultEnterpriseMetaInDefaultPartition()
}
// ValidateEnterprise validates that enterprise fields are only set
// with enterprise binaries.
func (failover *ServiceResolverFailover) ValidateEnterprise() error {
if failover.Namespace != "" {
return fmt.Errorf("Setting Namespace requires Consul Enterprise")
}
return nil
}
// GetEnterpriseMeta is used to synthesize the EnterpriseMeta struct from
// fields in the ServiceResolverFailoverTarget
func (target *ServiceResolverFailoverTarget) GetEnterpriseMeta(_ *acl.EnterpriseMeta) *acl.EnterpriseMeta {
return DefaultEnterpriseMetaInDefaultPartition()
}
// ValidateEnterprise validates that enterprise fields are only set
// with enterprise binaries.
func (redir *ServiceResolverFailoverTarget) ValidateEnterprise() error {
if redir.Partition != "" {
return fmt.Errorf("Setting Partition requires Consul Enterprise")
}
if redir.Namespace != "" {
return fmt.Errorf("Setting Namespace requires Consul Enterprise")
}
return nil
}
// GetEnterpriseMeta is used to synthesize the EnterpriseMeta struct from
// fields in the DiscoveryChainRequest
func (req *DiscoveryChainRequest) GetEnterpriseMeta() *acl.EnterpriseMeta {

View File

@ -0,0 +1,131 @@
//go:build !consulent
// +build !consulent
package structs
import (
"fmt"
"testing"
"github.com/stretchr/testify/require"
)
func TestServiceResolverConfigEntry_OSS(t *testing.T) {
type testcase struct {
name string
entry *ServiceResolverConfigEntry
normalizeErr string
validateErr string
// check is called between normalize and validate
check func(t *testing.T, entry *ServiceResolverConfigEntry)
}
cases := []testcase{
{
name: "failover with a namespace on OSS",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Service: "backup",
Namespace: "ns1",
},
},
},
validateErr: `Bad Failover["*"]: Setting Namespace requires Consul Enterprise`,
},
{
name: "failover Targets cannot set Namespace on OSS",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{{Namespace: "ns1"}},
},
},
},
validateErr: `Bad Failover["*"].Targets[0]: Setting Namespace requires Consul Enterprise`,
},
{
name: "failover Targets cannot set Partition on OSS",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{{Partition: "ap1"}},
},
},
},
validateErr: `Bad Failover["*"].Targets[0]: Setting Partition requires Consul Enterprise`,
},
{
name: "setting failover Namespace on OSS",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {Namespace: "ns1"},
},
},
validateErr: `Bad Failover["*"]: Setting Namespace requires Consul Enterprise`,
},
}
// Bulk add a bunch of similar validation cases.
for _, invalidSubset := range invalidSubsetNames {
tc := testcase{
name: "invalid subset name: " + invalidSubset,
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Subsets: map[string]ServiceResolverSubset{
invalidSubset: {OnlyPassing: true},
},
},
validateErr: fmt.Sprintf("Subset %q is invalid", invalidSubset),
}
cases = append(cases, tc)
}
for _, goodSubset := range validSubsetNames {
tc := testcase{
name: "valid subset name: " + goodSubset,
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Subsets: map[string]ServiceResolverSubset{
goodSubset: {OnlyPassing: true},
},
},
}
cases = append(cases, tc)
}
for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
err := tc.entry.Normalize()
if tc.normalizeErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.normalizeErr)
return
}
require.NoError(t, err)
if tc.check != nil {
tc.check(t, tc.entry)
}
err = tc.entry.Validate()
if tc.validateErr != "" {
require.Error(t, err)
require.Contains(t, err.Error(), tc.validateErr)
return
}
require.NoError(t, err)
})
}
}

View File

@ -165,6 +165,34 @@ func TestConfigEntries_ListRelatedServices_AndACLs(t *testing.T) {
},
},
},
{
name: "resolver: failover with targets",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{
{Service: "other1"},
{Datacenter: "dc2"},
{Peer: "cluster-01"},
},
},
},
},
expectServices: []ServiceID{NewServiceID("other1", nil)},
expectACLs: []testACL{
defaultDenyCase,
readTestCase,
writeTestCaseDenied,
{
name: "can write test (with other1:read)",
authorizer: newServiceACL(t, []string{"other1"}, []string{"test"}),
canRead: true,
canWrite: true,
},
},
},
{
name: "splitter: self",
entry: &ServiceSplitterConfigEntry{
@ -595,6 +623,15 @@ func TestServiceResolverConfigEntry(t *testing.T) {
},
validateErr: "Redirect is empty",
},
{
name: "empty redirect",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Redirect: &ServiceResolverRedirect{},
},
validateErr: "Redirect is empty",
},
{
name: "redirect subset with no service",
entry: &ServiceResolverConfigEntry{
@ -606,17 +643,6 @@ func TestServiceResolverConfigEntry(t *testing.T) {
},
validateErr: "Redirect.ServiceSubset defined without Redirect.Service",
},
{
name: "redirect namespace with no service",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Redirect: &ServiceResolverRedirect{
Namespace: "alternate",
},
},
validateErr: "Redirect.Namespace defined without Redirect.Service",
},
{
name: "self redirect with invalid subset",
entry: &ServiceResolverConfigEntry{
@ -695,7 +721,7 @@ func TestServiceResolverConfigEntry(t *testing.T) {
"v1": {},
},
},
validateErr: `Bad Failover["v1"] one of Service, ServiceSubset, Namespace, or Datacenters is required`,
validateErr: `Bad Failover["v1"]: one of Service, ServiceSubset, Namespace, Targets, or Datacenters is required`,
},
{
name: "failover to self using invalid subset",
@ -712,7 +738,7 @@ func TestServiceResolverConfigEntry(t *testing.T) {
},
},
},
validateErr: `Bad Failover["v1"].ServiceSubset "gone" is not a valid subset of "test"`,
validateErr: `Bad Failover["v1"]: ServiceSubset "gone" is not a valid subset of "test"`,
},
{
name: "failover to self using valid subset",
@ -745,6 +771,109 @@ func TestServiceResolverConfigEntry(t *testing.T) {
},
validateErr: `Bad Failover["*"].Datacenters: found empty datacenter`,
},
{
name: "failover target with an invalid subset",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{{ServiceSubset: "subset"}},
},
},
},
validateErr: `Bad Failover["*"].Targets[0]: ServiceSubset "subset" is not a valid subset of "test"`,
},
{
name: "failover targets can't have Peer and ServiceSubset",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{{Peer: "cluster-01", ServiceSubset: "subset"}},
},
},
},
validateErr: `Bad Failover["*"].Targets[0]: Peer cannot be set with ServiceSubset`,
},
{
name: "failover targets can't have Peer and Datacenter",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{{Peer: "cluster-01", Datacenter: "dc1"}},
},
},
},
validateErr: `Bad Failover["*"].Targets[0]: Peer cannot be set with Datacenter`,
},
{
name: "failover Targets cannot be set with Datacenters",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Datacenters: []string{"a"},
Targets: []ServiceResolverFailoverTarget{{Peer: "cluster-01"}},
},
},
},
validateErr: `Bad Failover["*"]: Targets cannot be set with Datacenters`,
},
{
name: "failover Targets cannot be set with ServiceSubset",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
ServiceSubset: "v2",
Targets: []ServiceResolverFailoverTarget{{Peer: "cluster-01"}},
},
},
Subsets: map[string]ServiceResolverSubset{
"v2": {Filter: "Service.Meta.version == v2"},
},
},
validateErr: `Bad Failover["*"]: Targets cannot be set with ServiceSubset`,
},
{
name: "failover Targets cannot be set with Service",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Service: "another-service",
Targets: []ServiceResolverFailoverTarget{{Peer: "cluster-01"}},
},
},
Subsets: map[string]ServiceResolverSubset{
"v2": {Filter: "Service.Meta.version == v2"},
},
},
validateErr: `Bad Failover["*"]: Targets cannot be set with Service`,
},
{
name: "complicated failover targets",
entry: &ServiceResolverConfigEntry{
Kind: ServiceResolver,
Name: "test",
Failover: map[string]ServiceResolverFailover{
"*": {
Targets: []ServiceResolverFailoverTarget{
{Peer: "cluster-01", Service: "test-v2"},
{Service: "test-v2", ServiceSubset: "test"},
{Datacenter: "dc2"},
},
},
},
},
},
{
name: "bad connect timeout",
entry: &ServiceResolverConfigEntry{

View File

@ -227,6 +227,16 @@ type ServiceResolverFailover struct {
// Referencing other partitions is not supported.
Namespace string `json:",omitempty"`
Datacenters []string `json:",omitempty"`
Targets []ServiceResolverFailoverTarget `json:",omitempty"`
}
type ServiceResolverFailoverTarget struct {
Service string `json:",omitempty"`
ServiceSubset string `json:",omitempty" alias:"service_subset"`
Partition string `json:",omitempty"`
Namespace string `json:",omitempty"`
Datacenter string `json:",omitempty"`
Peer string `json:",omitempty"`
}
// LoadBalancer determines the load balancing policy and configuration for services

View File

@ -149,6 +149,9 @@ func TestAPI_ConfigEntry_DiscoveryChain(t *testing.T) {
"v2": {
Filter: "Service.Meta.version == v2",
},
"v3": {
Filter: "Service.Meta.version == v3",
},
},
Failover: map[string]ServiceResolverFailover{
"*": {
@ -158,6 +161,13 @@ func TestAPI_ConfigEntry_DiscoveryChain(t *testing.T) {
Service: "alternate",
Namespace: splitDefaultNamespace,
},
"v3": {
Targets: []ServiceResolverFailoverTarget{
{Peer: "cluster-01"},
{Datacenter: "dc1"},
{Service: "another-service", ServiceSubset: "v1"},
},
},
},
ConnectTimeout: 5 * time.Second,
Meta: map[string]string{

View File

@ -630,6 +630,14 @@ func ServiceResolverFailoverToStructs(s *ServiceResolverFailover, t *structs.Ser
t.ServiceSubset = s.ServiceSubset
t.Namespace = s.Namespace
t.Datacenters = s.Datacenters
{
t.Targets = make([]structs.ServiceResolverFailoverTarget, len(s.Targets))
for i := range s.Targets {
if s.Targets[i] != nil {
ServiceResolverFailoverTargetToStructs(s.Targets[i], &t.Targets[i])
}
}
}
}
func ServiceResolverFailoverFromStructs(t *structs.ServiceResolverFailover, s *ServiceResolverFailover) {
if s == nil {
@ -639,6 +647,38 @@ func ServiceResolverFailoverFromStructs(t *structs.ServiceResolverFailover, s *S
s.ServiceSubset = t.ServiceSubset
s.Namespace = t.Namespace
s.Datacenters = t.Datacenters
{
s.Targets = make([]*ServiceResolverFailoverTarget, len(t.Targets))
for i := range t.Targets {
{
var x ServiceResolverFailoverTarget
ServiceResolverFailoverTargetFromStructs(&t.Targets[i], &x)
s.Targets[i] = &x
}
}
}
}
func ServiceResolverFailoverTargetToStructs(s *ServiceResolverFailoverTarget, t *structs.ServiceResolverFailoverTarget) {
if s == nil {
return
}
t.Service = s.Service
t.ServiceSubset = s.ServiceSubset
t.Partition = s.Partition
t.Namespace = s.Namespace
t.Datacenter = s.Datacenter
t.Peer = s.Peer
}
func ServiceResolverFailoverTargetFromStructs(t *structs.ServiceResolverFailoverTarget, s *ServiceResolverFailoverTarget) {
if s == nil {
return
}
s.Service = t.Service
s.ServiceSubset = t.ServiceSubset
s.Partition = t.Partition
s.Namespace = t.Namespace
s.Datacenter = t.Datacenter
s.Peer = t.Peer
}
func ServiceResolverRedirectToStructs(s *ServiceResolverRedirect, t *structs.ServiceResolverRedirect) {
if s == nil {

View File

@ -107,6 +107,16 @@ func (msg *ServiceResolverFailover) UnmarshalBinary(b []byte) error {
return proto.Unmarshal(b, msg)
}
// MarshalBinary implements encoding.BinaryMarshaler
func (msg *ServiceResolverFailoverTarget) MarshalBinary() ([]byte, error) {
return proto.Marshal(msg)
}
// UnmarshalBinary implements encoding.BinaryUnmarshaler
func (msg *ServiceResolverFailoverTarget) UnmarshalBinary(b []byte) error {
return proto.Unmarshal(b, msg)
}
// MarshalBinary implements encoding.BinaryMarshaler
func (msg *LoadBalancer) MarshalBinary() ([]byte, error) {
return proto.Marshal(msg)

File diff suppressed because it is too large Load Diff

View File

@ -134,6 +134,21 @@ message ServiceResolverFailover {
string ServiceSubset = 2;
string Namespace = 3;
repeated string Datacenters = 4;
repeated ServiceResolverFailoverTarget Targets = 5;
}
// mog annotation:
//
// target=github.com/hashicorp/consul/agent/structs.ServiceResolverFailoverTarget
// output=config_entry.gen.go
// name=Structs
message ServiceResolverFailoverTarget {
string Service = 1;
string ServiceSubset = 2;
string Partition = 3;
string Namespace = 4;
string Datacenter = 5;
string Peer = 6;
}
// mog annotation: