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 1 commit
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
3 changes: 3 additions & 0 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 @@ -79,6 +79,9 @@ spec:
description: '[Experimental] 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
name:
Expand Down
35 changes: 28 additions & 7 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 @@ -73,6 +74,8 @@ type ServiceConsumer struct {
Partition string `json:"partition,omitempty"`
// [Experimental] Peer is the name of the peer to export the service to.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this isn't you, but can you verify if Peering is still experimental for K8s? and if not change this?

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,31 @@ 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 in.Partition == "" && in.Peer == "" {
return field.Invalid(path, *in, "either partition or peer must be specified.")
if count > 1 {
return field.Invalid(path, *in, "Service consumer must define at most one of Peer, Partition, or SamenessGroup")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: errors should start with a lower case

}
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")
}
return nil
}
Expand Down
94 changes: 88 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,56 @@ 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`,
},
},
"multiple errors": {
input: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -423,6 +500,10 @@ func TestExportedServices_Validate(t *testing.T) {
Peer: "second-peer",
},
{},
{
SamenessGroup: "sg2",
Partition: "partition2",
},
},
},
},
Expand All @@ -431,8 +512,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
3 changes: 3 additions & 0 deletions control-plane/config/crd/bases/consul.hashicorp.com_exportedservices.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ spec:
description: '[Experimental] 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
name:
Expand Down
2 changes: 1 addition & 1 deletion control-plane/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20220831174802-b8af65262de8
github.com/hashicorp/consul-server-connection-manager v0.1.0
github.com/hashicorp/consul/api v1.10.1-0.20230331190547-fc64a702f43c
wilkermichael marked this conversation as resolved.
Show resolved Hide resolved
github.com/hashicorp/consul/api v1.10.1-0.20230418163148-eb9f671eafae
github.com/hashicorp/consul/sdk v0.13.1
github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f
github.com/hashicorp/go-hclog v1.2.2
Expand Down
2 changes: 2 additions & 0 deletions control-plane/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,8 @@ github.com/hashicorp/consul-server-connection-manager v0.1.0/go.mod h1:XVVlO+Yk7
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
github.com/hashicorp/consul/api v1.10.1-0.20230331190547-fc64a702f43c h1:MfDuWW38RozPodXT2aGc5jakoYo9EjgYpZl+vR3//Wg=
github.com/hashicorp/consul/api v1.10.1-0.20230331190547-fc64a702f43c/go.mod h1:f8zVJwBcLdr1IQnfdfszjUM0xzp31Zl3bpws3pL9uFM=
github.com/hashicorp/consul/api v1.10.1-0.20230418163148-eb9f671eafae h1:lYnO52QxlfATRZ1Vo8tQV+lFns7rZ4iAbbi3JN4ZAQw=
github.com/hashicorp/consul/api v1.10.1-0.20230418163148-eb9f671eafae/go.mod h1:f8zVJwBcLdr1IQnfdfszjUM0xzp31Zl3bpws3pL9uFM=
github.com/hashicorp/consul/proto-public v0.1.0 h1:O0LSmCqydZi363hsqc6n2v5sMz3usQMXZF6ziK3SzXU=
github.com/hashicorp/consul/proto-public v0.1.0/go.mod h1:vs2KkuWwtjkIgA5ezp4YKPzQp4GitV+q/+PvksrA92k=
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
Expand Down