Skip to content

Commit

Permalink
Add Targets field to service resolver failovers. (#14162)
Browse files Browse the repository at this point in the history
This field will be used for cluster peering failover.
  • Loading branch information
erichaberkorn committed Aug 15, 2022
1 parent d46b515 commit 1a73b0c
Show file tree
Hide file tree
Showing 11 changed files with 1,080 additions and 459 deletions.
5 changes: 5 additions & 0 deletions .changelog/14162.txt
Original file line number Diff line number Diff line change
@@ -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.
```
110 changes: 101 additions & 9 deletions agent/structs/config_entry_discoverychain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -988,18 +992,59 @@ func (e *ServiceResolverConfigEntry) Validate() error {
return fmt.Errorf("Cross-datacenter failover is only supported in the default partition")
}

errorPrefix := fmt.Sprintf("Bad Failover[%q]: ", subset)

if err := f.ValidateEnterprise(); err != nil {
return fmt.Errorf(errorPrefix + err.Error())
}

if subset != "*" && !isSubset(subset) {
return fmt.Errorf("Bad Failover[%q]: not a valid subset", subset)
return fmt.Errorf(errorPrefix + "not a valid subset subset")
}

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 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)
}
}
}
Expand Down Expand Up @@ -1107,9 +1152,24 @@ func (e *ServiceResolverConfigEntry) ListRelatedServices() []ServiceID {

if len(e.Failover) > 0 {
for _, failover := range e.Failover {
failoverID := NewServiceID(defaultIfEmpty(failover.Service, e.Name), failover.GetEnterpriseMeta(&e.EnterpriseMeta))
if failoverID != svcID {
found[failoverID] = struct{}{}
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{}{}
}
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions agent/structs/config_entry_discoverychain_oss.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package structs

import (
"fmt"

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

Expand All @@ -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 {
Expand Down
131 changes: 131 additions & 0 deletions agent/structs/config_entry_discoverychain_oss_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}

0 comments on commit 1a73b0c

Please sign in to comment.