Skip to content
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

add sameness group to exported services #2075

Merged
merged 8 commits into from
Apr 24, 2023
Merged

add sameness group to exported services #2075

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
03c7303
add sameness group to exported services
Apr 20, 2023
02664c0
update CRDs
Apr 21, 2023
dcd69cc
update deep copy
Apr 21, 2023
7a34ad0
re add license line
Apr 21, 2023
39a0d45
check if sameness group is wildcard
Apr 21, 2023
71886f1
remove experimental tag on peering fields
Apr 21, 2023
e748820
update error message case
Apr 21, 2023
d465ae0
update error message case in webhook test
Apr 21, 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
7 changes: 5 additions & 2 deletions charts/consul/templates/crd-exportedservices.yaml
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double checking that you ran make ctrl-manifests because I see changes when I run it locally

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I ran both make ctrl-manifests ctrl-generate and i saw multiple local changes as well. Maybe something needs changing here Mali?

Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,11 @@ spec:
the service to.
type: string
peer:
description: '[Experimental] Peer is the name of the peer
to export the service to.'
description: Peer is the name of the peer to export the service to.
type: string
samenessGroup:
description: SamenessGroup is the name of the sameness
group to export the service to.
type: string
type: object
type: array
Expand Down
5 changes: 3 additions & 2 deletions charts/consul/templates/crd-proxydefaults.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,9 @@ spec:
"sequential") and "order-by-locality".
type: string
regions:
description: The ordered list of the regions of the failover targets.
Valid values can be "us-west-1", "us-west-2", and so on.
description: Regions is the ordered list of the regions of the
failover targets. Valid values can be "us-west-1", "us-west-2",
and so on.
items:
type: string
type: array
Expand Down
17 changes: 14 additions & 3 deletions charts/consul/templates/crd-samenessgroups.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,27 @@ spec:
description: SamenessGroupSpec defines the desired state of SamenessGroup.
properties:
defaultForFailover:
description: 'DefaultForFailover indicates that upstream requests to members of the given sameness group will implicitly failover between members of this sameness group.'
description: DefaultForFailover indicates that upstream requests to
members of the given sameness group will implicitly failover between
members of this sameness group. When DefaultForFailover is true,
the local partition must be a member of the sameness group or IncludeLocal
must be set to true.
type: boolean
includeLocal:
description: 'IncludeLocal is used to include the local partition as the first member of the sameness group.'
description: IncludeLocal is used to include the local partition as
the first member of the sameness group. The local partition can
only be a member of a single sameness group.
type: boolean
members:
description: 'Members are the partitions and peers that are part of the sameness group.'
description: Members are the partitions and peers that are part of
the sameness group. If a member of a sameness group does not exist,
it will be ignored.
items:
properties:
partition:
description: The partitions and peers that are part of the sameness
group. A sameness group member cannot define both peer and
partition at the same time.
type: string
peer:
type: string
Expand Down
3 changes: 1 addition & 2 deletions charts/consul/templates/crd-serviceintentions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ spec:
description: Partition is the Admin Partition for the Name parameter.
type: string
peer:
description: '[Experimental] Peer is the peer name for the Name
parameter.'
description: Peer is the peer name for the Name parameter.
type: string
permissions:
description: Permissions is the list of all additional L7 attributes
Expand Down
5 changes: 3 additions & 2 deletions charts/consul/templates/crd-serviceresolvers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@ spec:
to "sequential") and "order-by-locality".
type: string
regions:
description: The ordered list of the regions of the failover targets.
Valid values can be "us-west-1", "us-west-2", and so on.
description: Regions is the ordered list of the regions
of the failover targets. Valid values can be "us-west-1",
"us-west-2", and so on.
items:
type: string
type: array
Expand Down
40 changes: 32 additions & 8 deletions control-plane/api/v1alpha1/exportedservices_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
)

const ExportedServicesKubeKind = "exportedservices"
const WildcardSpecifier = "*"

func init() {
SchemeBuilder.Register(&ExportedServices{}, &ExportedServicesList{})
Expand Down Expand Up @@ -71,8 +72,10 @@ type ExportedService struct {
type ServiceConsumer struct {
// Partition is the admin partition to export the service to.
Partition string `json:"partition,omitempty"`
// [Experimental] Peer is the name of the peer to export the service to.
// Peer is the name of the peer to export the service to.
Peer string `json:"peer,omitempty"`
// SamenessGroup is the name of the sameness group to export the service to.
SamenessGroup string `json:"samenessGroup,omitempty"`
}

func (in *ExportedServices) GetObjectMeta() metav1.ObjectMeta {
Expand Down Expand Up @@ -169,8 +172,9 @@ func (in *ExportedService) toConsul() capi.ExportedService {
var consumers []capi.ServiceConsumer
for _, consumer := range in.Consumers {
consumers = append(consumers, capi.ServiceConsumer{
Partition: consumer.Partition,
Peer: consumer.Peer,
Partition: consumer.Partition,
Peer: consumer.Peer,
SamenessGroup: consumer.SamenessGroup,
})
}
return capi.ExportedService{
Expand Down Expand Up @@ -230,14 +234,34 @@ func (in *ExportedService) validate(path *field.Path, consulMeta common.ConsulMe
}

func (in *ServiceConsumer) validate(path *field.Path, consulMeta common.ConsulMeta) *field.Error {
if in.Partition != "" && in.Peer != "" {
return field.Invalid(path, *in, "both partition and peer cannot be specified.")
count := 0

if in.Partition != "" {
count++
}
if in.Peer != "" {
count++
}
if in.SamenessGroup != "" {
count++
}
if count > 1 {
return field.Invalid(path, *in, "service consumer must define at most one of Peer, Partition, or SamenessGroup")
}
if in.Partition == "" && in.Peer == "" {
return field.Invalid(path, *in, "either partition or peer must be specified.")
if count == 0 {
return field.Invalid(path, *in, "service consumer must define at least one of Peer, Partition, or SamenessGroup")
}
if !consulMeta.PartitionsEnabled && in.Partition != "" {
return field.Invalid(path.Child("partitions"), in.Partition, "Consul Admin Partitions need to be enabled to specify partition.")
return field.Invalid(path.Child("partition"), in.Partition, "Consul Admin Partitions need to be enabled to specify partition.")
}
if in.Partition == WildcardSpecifier {
return field.Invalid(path.Child("partition"), "", "exporting to all partitions (wildcard) is not supported")
}
if in.Peer == WildcardSpecifier {
wilkermichael marked this conversation as resolved.
Show resolved Hide resolved
return field.Invalid(path.Child("peer"), "", "exporting to all peers (wildcard) is not supported")
}
if in.SamenessGroup == WildcardSpecifier {
return field.Invalid(path.Child("samenessgroup"), "", "exporting to all sameness groups (wildcard) is not supported")
}
return nil
}
Expand Down
119 changes: 113 additions & 6 deletions control-plane/api/v1alpha1/exportedservices_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ func TestExportedServices_MatchesConsul(t *testing.T) {
{
Peer: "second-peer",
},
{
SamenessGroup: "sg1",
},
},
},
{
Expand All @@ -74,6 +77,9 @@ func TestExportedServices_MatchesConsul(t *testing.T) {
{
Peer: "third-peer",
},
{
SamenessGroup: "sg2",
},
},
},
},
Expand All @@ -95,6 +101,9 @@ func TestExportedServices_MatchesConsul(t *testing.T) {
{
Peer: "second-peer",
},
{
SamenessGroup: "sg1",
},
},
},
{
Expand All @@ -110,6 +119,9 @@ func TestExportedServices_MatchesConsul(t *testing.T) {
{
Peer: "third-peer",
},
{
SamenessGroup: "sg2",
},
},
},
},
Expand Down Expand Up @@ -183,6 +195,9 @@ func TestExportedServices_ToConsul(t *testing.T) {
{
Peer: "second-peer",
},
{
SamenessGroup: "sg2",
},
},
},
{
Expand All @@ -198,6 +213,9 @@ func TestExportedServices_ToConsul(t *testing.T) {
{
Peer: "third-peer",
},
{
SamenessGroup: "sg3",
},
},
},
},
Expand All @@ -219,6 +237,9 @@ func TestExportedServices_ToConsul(t *testing.T) {
{
Peer: "second-peer",
},
{
SamenessGroup: "sg2",
},
},
},
{
Expand All @@ -234,6 +255,9 @@ func TestExportedServices_ToConsul(t *testing.T) {
{
Peer: "third-peer",
},
{
SamenessGroup: "sg3",
},
},
},
},
Expand Down Expand Up @@ -278,6 +302,9 @@ func TestExportedServices_Validate(t *testing.T) {
{
Peer: "second-peer",
},
{
SamenessGroup: "sg2",
},
},
},
},
Expand Down Expand Up @@ -331,10 +358,10 @@ func TestExportedServices_Validate(t *testing.T) {
namespaceEnabled: true,
partitionsEnabled: true,
expectedErrMsgs: []string{
`spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", Peer:"second-peer"}: both partition and peer cannot be specified.`,
`service consumer must define at most one of Peer, Partition, or SamenessGroup`,
},
},
"neither partition nor peer name specified": {
"none of peer, partition, or sameness group defined": {
input: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Name: common.DefaultConsulPartition,
Expand All @@ -354,7 +381,7 @@ func TestExportedServices_Validate(t *testing.T) {
namespaceEnabled: true,
partitionsEnabled: true,
expectedErrMsgs: []string{
`spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", Peer:""}: either partition or peer must be specified.`,
`service consumer must define at least one of Peer, Partition, or SamenessGroup`,
},
},
"partition provided when partitions are disabled": {
Expand All @@ -379,7 +406,7 @@ func TestExportedServices_Validate(t *testing.T) {
namespaceEnabled: true,
partitionsEnabled: false,
expectedErrMsgs: []string{
`spec.services[0].consumers[0].partitions: Invalid value: "test-partition": Consul Admin Partitions need to be enabled to specify partition.`,
`spec.services[0].consumers[0].partition: Invalid value: "test-partition": Consul Admin Partitions need to be enabled to specify partition.`,
},
},
"namespace provided when namespaces are disabled": {
Expand Down Expand Up @@ -407,6 +434,81 @@ func TestExportedServices_Validate(t *testing.T) {
`spec.services[0]: Invalid value: "frontend": Consul Namespaces must be enabled to specify service namespace.`,
},
},
"exporting to all partitions is not supported": {
input: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Name: common.DefaultConsulPartition,
},
Spec: ExportedServicesSpec{
Services: []ExportedService{
{
Name: "service-frontend",
Namespace: "frontend",
Consumers: []ServiceConsumer{
{
Partition: "*",
},
},
},
},
},
},
namespaceEnabled: true,
partitionsEnabled: true,
expectedErrMsgs: []string{
`exporting to all partitions (wildcard) is not supported`,
},
},
"exporting to all peers (wildcard) is not supported": {
input: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Name: common.DefaultConsulPartition,
},
Spec: ExportedServicesSpec{
Services: []ExportedService{
{
Name: "service-frontend",
Namespace: "frontend",
Consumers: []ServiceConsumer{
{
Peer: "*",
},
},
},
},
},
},
namespaceEnabled: true,
partitionsEnabled: true,
expectedErrMsgs: []string{
`exporting to all peers (wildcard) is not supported`,
},
},
"exporting to all sameness groups (wildcard) is not supported": {
input: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Name: common.DefaultConsulPartition,
},
Spec: ExportedServicesSpec{
Services: []ExportedService{
{
Name: "service-frontend",
Namespace: "frontend",
Consumers: []ServiceConsumer{
{
SamenessGroup: "*",
},
},
},
},
},
},
namespaceEnabled: true,
partitionsEnabled: true,
expectedErrMsgs: []string{
`exporting to all sameness groups (wildcard) is not supported`,
},
},
"multiple errors": {
input: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -423,6 +525,10 @@ func TestExportedServices_Validate(t *testing.T) {
Peer: "second-peer",
},
{},
{
SamenessGroup: "sg2",
Partition: "partition2",
},
},
},
},
Expand All @@ -431,8 +537,9 @@ func TestExportedServices_Validate(t *testing.T) {
namespaceEnabled: true,
partitionsEnabled: true,
expectedErrMsgs: []string{
`spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", Peer:"second-peer"}: both partition and peer cannot be specified.`,
`spec.services[0].consumers[1]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", Peer:""}: either partition or peer must be specified.`,
`spec.services[0].consumers[0]: Invalid value: v1alpha1.ServiceConsumer{Partition:"second", Peer:"second-peer", SamenessGroup:""}: service consumer must define at most one of Peer, Partition, or SamenessGroup`,
`spec.services[0].consumers[1]: Invalid value: v1alpha1.ServiceConsumer{Partition:"", Peer:"", SamenessGroup:""}: service consumer must define at least one of Peer, Partition, or SamenessGroup`,
`spec.services[0].consumers[2]: Invalid value: v1alpha1.ServiceConsumer{Partition:"partition2", Peer:"", SamenessGroup:"sg2"}: service consumer must define at most one of Peer, Partition, or SamenessGroup`,
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion control-plane/api/v1alpha1/serviceintentions_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ type SourceIntention struct {
Name string `json:"name,omitempty"`
// Namespace is the namespace for the Name parameter.
Namespace string `json:"namespace,omitempty"`
// [Experimental] Peer is the peer name for the Name parameter.
// Peer is the peer name for the Name parameter.
Peer string `json:"peer,omitempty"`
// Partition is the Admin Partition for the Name parameter.
Partition string `json:"partition,omitempty"`
Expand Down
Loading