From 2c0e7b5254bc5e6a3d898141b233ebd89c32a258 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 15 Jan 2025 14:36:53 -0500 Subject: [PATCH] Add plural IP fields to EndpointSpec ...that mirror the singular HealthCheckIP, PrivateIP and PublicIP fields to support dual-stack addresses. The singular fields are deprecated and remain to support backwards compatibility and migration with prior versions. Going forward only the plural fields will be used. Get*IP/Add*IP methods were added to EndpointSpec that handle the singular fields. On Get for IPv4, if the plural field doesn't contain an IPv4 address then retrieve the singular field. On Set for IPv4 also set the corresponding singular field. Signed-off-by: Tom Pantelis --- pkg/apis/submariner.io/v1/endpoint.go | 58 +++++ pkg/apis/submariner.io/v1/endpoint_test.go | 229 ++++++++++++++++++ pkg/apis/submariner.io/v1/string_test.go | 36 ++- pkg/apis/submariner.io/v1/types.go | 21 +- .../submariner.io/v1/zz_generated.deepcopy.go | 15 ++ .../submariner.io/v1/endpointspec.go | 53 +++- 6 files changed, 378 insertions(+), 34 deletions(-) diff --git a/pkg/apis/submariner.io/v1/endpoint.go b/pkg/apis/submariner.io/v1/endpoint.go index ff50c18f8..95ebabd8c 100644 --- a/pkg/apis/submariner.io/v1/endpoint.go +++ b/pkg/apis/submariner.io/v1/endpoint.go @@ -25,6 +25,7 @@ import ( "github.com/pkg/errors" "github.com/submariner-io/admiral/pkg/resource" "k8s.io/apimachinery/pkg/api/equality" + k8snet "k8s.io/utils/net" ) func (ep *EndpointSpec) GetBackendPort(configName string, defaultValue int32) (int32, error) { @@ -102,3 +103,60 @@ func (ep *EndpointSpec) hasSameBackendConfig(other *EndpointSpec) bool { return equality.Semantic.DeepEqual(ep.BackendConfig, other.BackendConfig) } + +func getIPFrom(family k8snet.IPFamily, ips []string, ipv4Fallback string) string { + for _, ip := range ips { + if k8snet.IPFamilyOfString(ip) == family { + return ip + } + } + + if family == k8snet.IPv4 { + return ipv4Fallback + } + + return "" +} + +func setIP(ips []string, ipv4Fallback, newIP string) ([]string, string) { + family := k8snet.IPFamilyOfString(newIP) + + if family == k8snet.IPv4 { + ipv4Fallback = newIP + } + + for i := range ips { + if k8snet.IPFamilyOfString(ips[i]) == family { + ips[i] = newIP + return ips, ipv4Fallback + } + } + + ips = append(ips, newIP) + + return ips, ipv4Fallback +} + +func (ep *EndpointSpec) GetHealthCheckIP(family k8snet.IPFamily) string { + return getIPFrom(family, ep.HealthCheckIPs, ep.HealthCheckIP) +} + +func (ep *EndpointSpec) SetHealthCheckIP(ip string) { + ep.HealthCheckIPs, ep.HealthCheckIP = setIP(ep.HealthCheckIPs, ep.HealthCheckIP, ip) +} + +func (ep *EndpointSpec) GetPublicIP(family k8snet.IPFamily) string { + return getIPFrom(family, ep.PublicIPs, ep.PublicIP) +} + +func (ep *EndpointSpec) SetPublicIP(ip string) { + ep.PublicIPs, ep.PublicIP = setIP(ep.PublicIPs, ep.PublicIP, ip) +} + +func (ep *EndpointSpec) GetPrivateIP(family k8snet.IPFamily) string { + return getIPFrom(family, ep.PrivateIPs, ep.PrivateIP) +} + +func (ep *EndpointSpec) SetPrivateIP(ip string) { + ep.PrivateIPs, ep.PrivateIP = setIP(ep.PrivateIPs, ep.PrivateIP, ip) +} diff --git a/pkg/apis/submariner.io/v1/endpoint_test.go b/pkg/apis/submariner.io/v1/endpoint_test.go index 37c4554da..13e197486 100644 --- a/pkg/apis/submariner.io/v1/endpoint_test.go +++ b/pkg/apis/submariner.io/v1/endpoint_test.go @@ -22,11 +22,23 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" v1 "github.com/submariner-io/submariner/pkg/apis/submariner.io/v1" + k8snet "k8s.io/utils/net" +) + +const ( + ipV4Addr = "1.2.3.4" + ipV6Addr = "2001:db8:3333:4444:5555:6666:7777:8888" ) var _ = Describe("EndpointSpec", func() { Context("GenerateName", testGenerateName) Context("Equals", testEquals) + Context("GetHealthCheckIP", testGetHealthCheckIP) + Context("SetHealthCheckIP", testSetHealthCheckIP) + Context("GetPublicIP", testGetPublicIP) + Context("SetPublicIP", testSetPublicIP) + Context("GetPrivateIP", testGetPrivateIP) + Context("SetPrivateIP", testSetPrivateIP) }) func testGenerateName() { @@ -138,3 +150,220 @@ func testEquals() { }) }) } + +func testGetIP(ipsSetter func(*v1.EndpointSpec, []string, string), ipsGetter func(*v1.EndpointSpec, k8snet.IPFamily) string) { + var ( + spec *v1.EndpointSpec + legacyIPv4IP string + ips []string + ) + + BeforeEach(func() { + legacyIPv4IP = "" + ips = []string{} + }) + + JustBeforeEach(func() { + spec = &v1.EndpointSpec{} + ipsSetter(spec, ips, legacyIPv4IP) + }) + + Context("IPv4", func() { + When("an IPv4 address is present", func() { + BeforeEach(func() { + ips = []string{ipV6Addr, ipV4Addr} + }) + + It("should return the address", func() { + Expect(ipsGetter(spec, k8snet.IPv4)).To(Equal(ipV4Addr)) + }) + }) + + When("an IPv4 address is not present and the legacy IPv4 address is set", func() { + BeforeEach(func() { + ips = []string{ipV6Addr} + legacyIPv4IP = ipV4Addr + }) + + It("should return the legacy address", func() { + Expect(ipsGetter(spec, k8snet.IPv4)).To(Equal(ipV4Addr)) + }) + }) + + When("an IPv4 address is not present and the legacy IPv4 address is not set", func() { + It("should return empty string", func() { + Expect(ipsGetter(spec, k8snet.IPv4)).To(BeEmpty()) + }) + }) + }) + + Context("IPv6", func() { + When("an IPv6 address is present", func() { + BeforeEach(func() { + ips = []string{ipV4Addr, ipV6Addr} + }) + + It("should return the address", func() { + Expect(ipsGetter(spec, k8snet.IPv6)).To(Equal(ipV6Addr)) + }) + }) + + When("an IPv6 address is not present", func() { + BeforeEach(func() { + ips = []string{ipV4Addr} + }) + + It("should return empty string", func() { + Expect(ipsGetter(spec, k8snet.IPv6)).To(BeEmpty()) + }) + }) + }) +} + +func testSetIP(initIPs func(*v1.EndpointSpec, []string), ipsSetter func(*v1.EndpointSpec, string), + ipsGetter func(*v1.EndpointSpec) ([]string, string), +) { + var ( + spec *v1.EndpointSpec + ipToSet string + initialIPs []string + ) + + BeforeEach(func() { + spec = &v1.EndpointSpec{} + initialIPs = []string{} + ipToSet = "" + }) + + JustBeforeEach(func() { + initIPs(spec, initialIPs) + ipsSetter(spec, ipToSet) + }) + + verifyIPs := func(ips []string, legacyV4 string) { + actualIPs, actualLegacy := ipsGetter(spec) + Expect(actualIPs).To(Equal(ips)) + Expect(actualLegacy).To(Equal(legacyV4)) + } + + Context("IPv4", func() { + BeforeEach(func() { + ipToSet = ipV4Addr + }) + + When("no addresses are present", func() { + It("should add the new address", func() { + verifyIPs([]string{ipToSet}, ipToSet) + }) + }) + + When("no IPv4 address is present", func() { + BeforeEach(func() { + initialIPs = []string{ipV6Addr} + }) + + It("should add the new address", func() { + verifyIPs([]string{ipV6Addr, ipToSet}, ipToSet) + }) + }) + + When("an IPv4 address is already present", func() { + BeforeEach(func() { + initialIPs = []string{"11.22.33.44"} + }) + + It("should update address", func() { + verifyIPs([]string{ipToSet}, ipToSet) + }) + }) + }) + + Context("IPv6", func() { + BeforeEach(func() { + ipToSet = ipV6Addr + }) + + When("no addresses are present", func() { + It("should add the new address", func() { + verifyIPs([]string{ipToSet}, "") + }) + }) + + When("no IPv6 address is present", func() { + BeforeEach(func() { + initialIPs = []string{ipV4Addr} + }) + + It("should add the new address", func() { + verifyIPs([]string{ipV4Addr, ipToSet}, "") + }) + }) + + When("an IPv6 address is already present", func() { + BeforeEach(func() { + initialIPs = []string{"1234:cb9:3333:4444:5555:6666:7777:8888"} + }) + + It("should update address", func() { + verifyIPs([]string{ipToSet}, "") + }) + }) + }) +} + +func testGetHealthCheckIP() { + testGetIP(func(s *v1.EndpointSpec, ips []string, ipv4IP string) { + s.HealthCheckIPs = ips + s.HealthCheckIP = ipv4IP + }, func(s *v1.EndpointSpec, family k8snet.IPFamily) string { + return s.GetHealthCheckIP(family) + }) +} + +func testSetHealthCheckIP() { + testSetIP(func(s *v1.EndpointSpec, ips []string) { + s.HealthCheckIPs = ips + }, func(s *v1.EndpointSpec, ip string) { + s.SetHealthCheckIP(ip) + }, func(s *v1.EndpointSpec) ([]string, string) { + return s.HealthCheckIPs, s.HealthCheckIP + }) +} + +func testGetPublicIP() { + testGetIP(func(s *v1.EndpointSpec, ips []string, ipv4IP string) { + s.PublicIPs = ips + s.PublicIP = ipv4IP + }, func(s *v1.EndpointSpec, family k8snet.IPFamily) string { + return s.GetPublicIP(family) + }) +} + +func testSetPublicIP() { + testSetIP(func(s *v1.EndpointSpec, ips []string) { + s.PublicIPs = ips + }, func(s *v1.EndpointSpec, ip string) { + s.SetPublicIP(ip) + }, func(s *v1.EndpointSpec) ([]string, string) { + return s.PublicIPs, s.PublicIP + }) +} + +func testGetPrivateIP() { + testGetIP(func(s *v1.EndpointSpec, ips []string, ipv4IP string) { + s.PrivateIPs = ips + s.PrivateIP = ipv4IP + }, func(s *v1.EndpointSpec, family k8snet.IPFamily) string { + return s.GetPrivateIP(family) + }) +} + +func testSetPrivateIP() { + testSetIP(func(s *v1.EndpointSpec, ips []string) { + s.PrivateIPs = ips + }, func(s *v1.EndpointSpec, ip string) { + s.SetPrivateIP(ip) + }, func(s *v1.EndpointSpec) ([]string, string) { + return s.PrivateIPs, s.PrivateIP + }) +} diff --git a/pkg/apis/submariner.io/v1/string_test.go b/pkg/apis/submariner.io/v1/string_test.go index 18647a29d..6a22b4126 100644 --- a/pkg/apis/submariner.io/v1/string_test.go +++ b/pkg/apis/submariner.io/v1/string_test.go @@ -26,25 +26,23 @@ import ( v1 "github.com/submariner-io/submariner/pkg/apis/submariner.io/v1" ) -const expectedString = `{"metadata":{"creationTimestamp":null},"spec":{"cluster_id":"cluster-id","cable_name":` + - `"cable-1","hostname":"","subnets":["10.0.0.0/24","172.0.0.0/24"],"private_ip":"1.1.1.1",` + - `"public_ip":"","nat_enabled":false,"backend":""}}` - -var _ = Describe("API v1", func() { - When("Endpoint String representation called", func() { - It("Should return a human readable string", func() { - endpoint := v1.Endpoint{ - Spec: v1.EndpointSpec{ - ClusterID: "cluster-id", - Subnets: []string{"10.0.0.0/24", "172.0.0.0/24"}, - CableName: "cable-1", - PublicIP: "", - PrivateIP: "1.1.1.1", - }, - } - - Expect(endpoint.String()).To(Equal(expectedString)) - }) +var _ = Describe("Endpoint String", func() { + It("should return a human readable string", func() { + str := (&v1.Endpoint{ + Spec: v1.EndpointSpec{ + ClusterID: "east", + Subnets: []string{"10.0.0.0/24"}, + CableName: "cable-1", + PublicIPs: []string{"1.1.1.1"}, + PrivateIPs: []string{"2.2.2.2"}, + }, + }).String() + + Expect(str).To(ContainSubstring("east")) + Expect(str).To(ContainSubstring("10.0.0.0/24")) + Expect(str).To(ContainSubstring("cable-1")) + Expect(str).To(ContainSubstring("1.1.1.1")) + Expect(str).To(ContainSubstring("2.2.2.2")) }) }) diff --git a/pkg/apis/submariner.io/v1/types.go b/pkg/apis/submariner.io/v1/types.go index a62c2d1df..2ae49310e 100644 --- a/pkg/apis/submariner.io/v1/types.go +++ b/pkg/apis/submariner.io/v1/types.go @@ -79,11 +79,22 @@ type EndpointSpec struct { ClusterID string `json:"cluster_id"` CableName string `json:"cable_name"` // +optional - HealthCheckIP string `json:"healthCheckIP,omitempty"` - Hostname string `json:"hostname"` - Subnets []string `json:"subnets"` - PrivateIP string `json:"private_ip"` - PublicIP string `json:"public_ip"` + HealthCheckIP string `json:"healthCheckIP,omitempty"` + // +kubebuilder:validation:MaxItems:=2 + // +optional + HealthCheckIPs []string `json:"healthCheckIPs,omitempty"` + Hostname string `json:"hostname"` + Subnets []string `json:"subnets"` + // +optional + PrivateIP string `json:"private_ip,omitempty"` + // +kubebuilder:validation:MaxItems:=2 + // +optional + PrivateIPs []string `json:"privateIPs,omitempty"` + // +optional + PublicIP string `json:"public_ip,omitempty"` + // +kubebuilder:validation:MaxItems:=2 + // +optional + PublicIPs []string `json:"publicIPs,omitempty"` NATEnabled bool `json:"nat_enabled"` Backend string `json:"backend"` BackendConfig map[string]string `json:"backend_config,omitempty"` diff --git a/pkg/apis/submariner.io/v1/zz_generated.deepcopy.go b/pkg/apis/submariner.io/v1/zz_generated.deepcopy.go index 8d7627979..bcab1db27 100644 --- a/pkg/apis/submariner.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/submariner.io/v1/zz_generated.deepcopy.go @@ -292,11 +292,26 @@ func (in *EndpointList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EndpointSpec) DeepCopyInto(out *EndpointSpec) { *out = *in + if in.HealthCheckIPs != nil { + in, out := &in.HealthCheckIPs, &out.HealthCheckIPs + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Subnets != nil { in, out := &in.Subnets, &out.Subnets *out = make([]string, len(*in)) copy(*out, *in) } + if in.PrivateIPs != nil { + in, out := &in.PrivateIPs, &out.PrivateIPs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.PublicIPs != nil { + in, out := &in.PublicIPs, &out.PublicIPs + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.BackendConfig != nil { in, out := &in.BackendConfig, &out.BackendConfig *out = make(map[string]string, len(*in)) diff --git a/pkg/client/applyconfiguration/submariner.io/v1/endpointspec.go b/pkg/client/applyconfiguration/submariner.io/v1/endpointspec.go index 60abf564d..8339f3e5b 100644 --- a/pkg/client/applyconfiguration/submariner.io/v1/endpointspec.go +++ b/pkg/client/applyconfiguration/submariner.io/v1/endpointspec.go @@ -23,16 +23,19 @@ package v1 // EndpointSpecApplyConfiguration represents a declarative configuration of the EndpointSpec type for use // with apply. type EndpointSpecApplyConfiguration struct { - ClusterID *string `json:"cluster_id,omitempty"` - CableName *string `json:"cable_name,omitempty"` - HealthCheckIP *string `json:"healthCheckIP,omitempty"` - Hostname *string `json:"hostname,omitempty"` - Subnets []string `json:"subnets,omitempty"` - PrivateIP *string `json:"private_ip,omitempty"` - PublicIP *string `json:"public_ip,omitempty"` - NATEnabled *bool `json:"nat_enabled,omitempty"` - Backend *string `json:"backend,omitempty"` - BackendConfig map[string]string `json:"backend_config,omitempty"` + ClusterID *string `json:"cluster_id,omitempty"` + CableName *string `json:"cable_name,omitempty"` + HealthCheckIP *string `json:"healthCheckIP,omitempty"` + HealthCheckIPs []string `json:"healthCheckIPs,omitempty"` + Hostname *string `json:"hostname,omitempty"` + Subnets []string `json:"subnets,omitempty"` + PrivateIP *string `json:"private_ip,omitempty"` + PrivateIPs []string `json:"privateIPs,omitempty"` + PublicIP *string `json:"public_ip,omitempty"` + PublicIPs []string `json:"publicIPs,omitempty"` + NATEnabled *bool `json:"nat_enabled,omitempty"` + Backend *string `json:"backend,omitempty"` + BackendConfig map[string]string `json:"backend_config,omitempty"` } // EndpointSpecApplyConfiguration constructs a declarative configuration of the EndpointSpec type for use with @@ -65,6 +68,16 @@ func (b *EndpointSpecApplyConfiguration) WithHealthCheckIP(value string) *Endpoi return b } +// WithHealthCheckIPs adds the given value to the HealthCheckIPs field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the HealthCheckIPs field. +func (b *EndpointSpecApplyConfiguration) WithHealthCheckIPs(values ...string) *EndpointSpecApplyConfiguration { + for i := range values { + b.HealthCheckIPs = append(b.HealthCheckIPs, values[i]) + } + return b +} + // WithHostname sets the Hostname field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the Hostname field is set to the value of the last call. @@ -91,6 +104,16 @@ func (b *EndpointSpecApplyConfiguration) WithPrivateIP(value string) *EndpointSp return b } +// WithPrivateIPs adds the given value to the PrivateIPs field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the PrivateIPs field. +func (b *EndpointSpecApplyConfiguration) WithPrivateIPs(values ...string) *EndpointSpecApplyConfiguration { + for i := range values { + b.PrivateIPs = append(b.PrivateIPs, values[i]) + } + return b +} + // WithPublicIP sets the PublicIP field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the PublicIP field is set to the value of the last call. @@ -99,6 +122,16 @@ func (b *EndpointSpecApplyConfiguration) WithPublicIP(value string) *EndpointSpe return b } +// WithPublicIPs adds the given value to the PublicIPs field in the declarative configuration +// and returns the receiver, so that objects can be build by chaining "With" function invocations. +// If called multiple times, values provided by each call will be appended to the PublicIPs field. +func (b *EndpointSpecApplyConfiguration) WithPublicIPs(values ...string) *EndpointSpecApplyConfiguration { + for i := range values { + b.PublicIPs = append(b.PublicIPs, values[i]) + } + return b +} + // WithNATEnabled sets the NATEnabled field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the NATEnabled field is set to the value of the last call.