Add Targets field to service resolver failovers.

.changelog/14162.txt

@@ -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.
+```

agent/structs/config_entry_discoverychain.go

@@ -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")
 			}
 
+			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)
 					}
 				}
 			}
@@ -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{}{}
+				}
 			}
 		}
 	}
@@ -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

agent/structs/config_entry_discoverychain_oss.go

@@ -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 {

agent/structs/config_entry_discoverychain_oss_test.go

@@ -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)
+		})
+	}
+}